Conversation
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Code Review
Assessment: APPROVE
This PR contains clean refactoring improvements and a bug fix:
✅ cmd/root/api.go: Refactors error handling to remove else-after-return anti-pattern (Go style improvement). The cleanup defer pattern is safe - setupRecordingProxy always returns a valid cleanup function when err == nil, and the defer is only reached after confirming no error occurred.
✅ cmd/root/sandbox.go: Fixes a bug where os.Exit() prevented deferred cleanup functions from running. Now returns cli.StatusError instead, allowing proper resource cleanup.
✅ main.go: Adds proper handling for cli.StatusError to exit with correct status codes, completing the sandbox.go fix.
All changes improve code quality and correctness. No bugs detected in the changed code.
Replace the direct os.Exit() call with a cli.StatusError return so that all deferred cleanup functions (including stopTokenWriter) run normally. The exit code is propagated via cli.StatusError and handled in main.go. This removes the need to manually call stopTokenWriter() before os.Exit() and ensures all defers in the call chain execute. Fixes docker#1981 Fixes docker#1986 Assisted-By: docker-agent
The defer for recordCleanup was inside an else block, which meant it was scoped to that block only. Restructure to declare variables first, check the error, then defer cleanup unconditionally - matching the pattern already used in run.go. Fixes docker#1980 Fixes docker#1987 Assisted-By: docker-agent
2 participants
No description provided.