Skip to content

Fail upgrade tool on download and write errors #1435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattn merged 2 commits into from
Jul 29, 2026
+33 −17
Merged

Fail upgrade tool on download and write errors #1435

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
322330f
Fail upgrade tool on download and write errors
mattn Jul 29, 2026
6ebb665
Close merge destination before removing the source
mattn Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 24 additions & 16 deletions upgrade/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ func download(prefix string) (url string, content []byte, err error) {
return "", nil, err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return "", nil, fmt.Errorf("downloading download.html: %s", resp.Status)
}

b, err := ioutil.ReadAll(resp.Body)
if err != nil {
Expand Down Expand Up @@ -60,10 +63,13 @@ func download(prefix string) (url string, content []byte, err error) {
if err != nil {
return "", nil, err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return "", nil, fmt.Errorf("downloading %s: %s", url, resp.Status)
}

// Ready Body Content
content, err = ioutil.ReadAll(resp.Body)
defer resp.Body.Close()
if err != nil {
return "", nil, err
}
Expand All @@ -72,17 +78,6 @@ func download(prefix string) (url string, content []byte, err error) {
}

func mergeFile(src string, dst string) error {
defer func() error {
fmt.Printf("Removing: %s\n", src)
err := os.Remove(src)

if err != nil {
return err
}

return nil
}()

// Open destination
fdst, err := os.OpenFile(dst, os.O_APPEND|os.O_WRONLY, 0666)
if err != nil {
Expand All @@ -106,7 +101,9 @@ func mergeFile(src string, dst string) error {
return err
}

return nil
// Only remove the source once it has been merged successfully.
fmt.Printf("Removing: %s\n", src)
return os.Remove(src)
Comment on lines +112 to +114

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Close the destination before removing the source.

fdst.Close() remains deferred and its error is ignored, but os.Remove(src) runs first. A destination close/flush failure can therefore delete the only source copy while mergeFile still returns success. Explicitly close fdst, propagate that error, and remove src only after the close succeeds; preserve close errors on earlier return paths as well.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@upgrade/upgrade.go` around lines 104 - 106, Update mergeFile so fdst is
explicitly closed before os.Remove(src), returning any close error and only
deleting the source after a successful close. Replace the deferred-only close
handling with logic that also preserves and propagates fdst.Close errors on all
earlier return paths.

}

func main() {
Expand Down Expand Up @@ -170,6 +167,7 @@ func main() {
log.Fatal(err)
}
scanner := bufio.NewScanner(zr)
var werr error
for scanner.Scan() {
text := scanner.Text()
if text == `#include "sqlite3.h"` {
Expand All @@ -179,11 +177,19 @@ func main() {
#endif
`
}
_, err = fmt.Fprintln(f, text)
if err != nil {
_, werr = fmt.Fprintln(f, text)
if werr != nil {
break
}
}
// A write failure must not be masked by scanner.Err(), which
// reports nil in that case; either way a truncated output file
// must never be reported as successfully extracted.
if werr != nil {
zr.Close()
f.Close()
log.Fatal(werr)
}
err = scanner.Err()
if err != nil {
zr.Close()
Expand All @@ -197,7 +203,9 @@ func main() {
log.Fatal(err)
}
zr.Close()
f.Close()
if err := f.Close(); err != nil {
log.Fatal(err)
}
fmt.Printf("Extracted: %v\n", filepath.Base(f.Name()))
}

Expand Down
Loading