fix(update): close response body and file on error paths

[nghiphaam]

utils.Fatal calls os.Exit(1), so deferred closes never run on error paths.

Manually close out and resp2.Body before each Fatal call.
Move defer resp2.Body.Close() to after all error checks pass.

Fixes resource leak in src/cmd/update.go.

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of the update download process by ensuring temporary files and network connections are reliably closed on all error paths, preventing resource leaks and improving stability during failed or partial downloads.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 55173768-101c-466a-be2a-acaa23a7f56f

📥 Commits

Reviewing files that changed from the base of the PR and between 3d78f38 and 05ef30e.

📒 Files selected for processing (1)

• src/cmd/update.go

🚧 Files skipped from review as they are similar to previous changes (1)

• src/cmd/update.go

---

📝 Walkthrough

Walkthrough

The update download flow now ensures explicit closure of the output file and HTTP response body on all error paths: after http.Get failure, non-200 responses, and io.Copy errors. defer resp2.Body.Close() is installed only after confirming StatusCode == http.StatusOK.

Changes

Cohort / File(s)	Summary
Resource cleanup in download flow src/cmd/update.go	Added explicit out.Close() and resp2.Body.Close() calls on error paths (http.Get error, non-200 status, io.Copy error). Moved defer resp2.Body.Close() to run only after confirming StatusCode == http.StatusOK rather than immediately after the request.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through code with nervous cheer,
Closed files and bodies far and near,
Deferred no more before the check,
Now failures tidy every speck,
A cleaner run—huzzah, my dear! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the main change: closing response body and file on error paths to fix resource leaks in the update process.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/cmd/update.go (1)

82-95: ⚠️ Potential issue | 🟡 Minor

Subsequent utils.Fatal calls still leak out and resp2.Body.

The same root cause addressed by this PR (deferred closes don't run because utils.Fatal invokes os.Exit(1)) applies to the error branches at lines 84, 87, and inside permissionError (line 94). On those paths, both the deferred out.Close() (line 56) and defer resp2.Body.Close() (line 72) are skipped, leaving the file and response body open until process termination.

Consider closing both resources before invoking utils.Fatal/permissionError on these remaining branches to fully eliminate the leak class, e.g.:

🛠️ Suggested adjustment

 	exe, err := os.Executable()
 	if err != nil {
+		resp2.Body.Close()
+		out.Close()
 		utils.Fatal(err)
 	}
 	if exe, err = filepath.EvalSymlinks(exe); err != nil {
+		resp2.Body.Close()
+		out.Close()
 		utils.Fatal(err)
 	}

A cleaner alternative is to close both resources explicitly right after io.Copy succeeds (and drop the defer at line 72), so later error paths don't need to repeat the cleanup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cmd/update.go` around lines 82 - 95, The error paths in update.go call
utils.Fatal or permissionError before deferred cleanup runs, leaking out and
resp2.Body; modify the code in the update flow (references: variables out and
resp2.Body, functions utils.Fatal and permissionError, and the io.Copy success
point) to explicitly close both out.Close() and resp2.Body.Close() before any
call to utils.Fatal or permissionError, or move the closes to occur immediately
after the successful io.Copy and remove the deferred
resp2.Body.Close()/out.Close() so later error branches no longer rely on defer
for cleanup.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cmd/update.go`:
- Around line 74-79: The io.Copy error branch leaks the output file handle
because out.Close() isn't called before utils.Fatal exits; update the error path
after io.Copy (where resp2.Body is closed and spinner.Fail is called) to also
explicitly call out.Close() prior to invoking utils.Fatal so the file descriptor
is released (referencing out, resp2.Body, io.Copy, spinner.Fail, and
utils.Fatal).

---

Outside diff comments:
In `@src/cmd/update.go`:
- Around line 82-95: The error paths in update.go call utils.Fatal or
permissionError before deferred cleanup runs, leaking out and resp2.Body; modify
the code in the update flow (references: variables out and resp2.Body, functions
utils.Fatal and permissionError, and the io.Copy success point) to explicitly
close both out.Close() and resp2.Body.Close() before any call to utils.Fatal or
permissionError, or move the closes to occur immediately after the successful
io.Copy and remove the deferred resp2.Body.Close()/out.Close() so later error
branches no longer rely on defer for cleanup.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5ba24610-83b8-456a-ab72-f3b050d09c83

📥 Commits

Reviewing files that changed from the base of the PR and between 035d194 and 3d78f38.

📒 Files selected for processing (1)

• src/cmd/update.go