cavaliergopher / grab

A download manager package for Go
BSD 3-Clause "New" or "Revised" License
1.38k stars 151 forks source link

DeleteOnError does not seem to work (at least on Windows) #45

Closed 030 closed 5 years ago

030 commented 5 years ago
// create download request
req, err := NewRequest("", "http://example.com/example.zip")
if err != nil {
    panic(err)
}

// set request checksum
sum, err := hex.DecodeString("33daf4c03f86120fdfdc66bddf6bfff4661c7ca11c5da473e537f4d69b470e57")
if err != nil {
    panic(err)
}
req.SetChecksum(sha256.New(), sum, true)

// download and validate file
resp := DefaultClient.Do(req)
if err := resp.Err(); err != nil {
    panic(err)
}

Although there is a checksum mismatch, the file will not be removed:

panic: checksum mismatch

goroutine 1 [running]:
main.downloadOpenjdk()
        C:/Users/path/to/main.go:88 +0x1b6
main.main()
        C:/Users/path/to/main.go:94 +0x2c
exit status 2

Apart from this, how to prevent that the file gets downloaded anyway if there is checksum mismatch?

I think that one of the first improvements could be adding error handling to the os.remove snippet https://github.com/cavaliercoder/grab/blob/925bcfe56bc16868f1a398af4231cd4ffa07276f/client.go#L294 to ensure that at least an error message is returned. In my opinion error message should not be omitted.

cavaliercoder commented 5 years ago

I agree, this error should be returned. Fixed this in c709a70. Please let me know if you can now see why your file was not deleted.

030 commented 5 years ago

@cavaliercoder An error message is returned now!

2018/11/08 07:25:08 cannot remove downloaded file with checksum mismatch: remove 
C:\Program Files\someFolder\someZip.zip: 
The process cannot access the file because it is being used by another process.
exit status 1

Apart from this, the file can be removed manually. Is the code preventing this or is this a Windows issue? I have ran the same command from an Administrator prompt, but then the same issue occurs.

When rm "/c/Program Files/someFolder\someZip.zip" is issued the file gets removed immediately.

cavaliercoder commented 5 years ago

This may be because grab tries to delete the file before closing it (which does not manifest as an issue on Linux). I've refactored to ensure the file is first closed (see: 2c8601d).

Please test and advise.

cavaliercoder commented 5 years ago

@030 Any luck with this?

030 commented 5 years ago

@cavaliercoder Verified that it works! Thank you! 👍

$ go run main_windows_amd64.go
2018/11/13 07:20:20 checksum mismatch
exit status 1

Steps to reproduce

cavaliercoder commented 5 years ago

Thanks for the feedback :)