dsaltares / fetch-gh-release-asset

Github Action to download an asset from a Github release
MIT License
114 stars 70 forks source link

Check a hash and retry #29

Closed sethgupton-mastery closed 2 years ago

sethgupton-mastery commented 2 years ago

We've been running into a weird error where this action would say it had successfully downloaded the file but it hadn't.

The file it had actually downloaded was 922 bytes instead of the 32MB file it was supposed to have downloaded.

Wanted to give it the ability to retry so I added the option to pass in a hash and, if the hash doesn't match the file downloaded, try again. Even if the retry doesn't work at least it will properly error out instead of failing silently.

I also thought about going off of filesize (since that is included in the github message) but I had problems getting that working and hash seemed safer.

dsaltares commented 2 years ago

Thanks for the PR @sethgupton-mastery!

I wonder if a better API would be:

At the moment, tying the hash option to both checking the hash and retrying seems a bit odd.

tomMoulard commented 2 years ago

I am wondering if the hash could also be gathered from the assets. This way, you are not forced to update the action whenever the hash of you downloaded assets changes.

As an example, goreleaser provide a checksum.txt for each release to check if the downloaded asset is valid https://github.com/goreleaser/goreleaser/releases/tag/v1.3.0

sethgupton-mastery commented 2 years ago

@dsaltares Working on splitting retries out to another parameter now.

@tomMoulard I considered that but worried that adding another download that could have problems causing it to be more likely to fail. (And if added for security or stability reasons would allow for insertion of a different checksum along with a different file)

sethgupton-mastery commented 2 years ago

Done! Let me know if you'd like any other changes.

dsaltares commented 2 years ago

@sethgupton-mastery apologies, but since the last merge, this PR now has a few conflicts. Would you be able to resolve them and take into account the regex option?

sethgupton-mastery commented 2 years ago

Hrm, regex with retries isn't too hard. But how would regex and hash work? regex will return multiple files so that seems kinda awkward. Make them mutually exclusive options?

I really wish github would include the hash in their api call... Although like I said above that may introduce security issues. (Not that security issues were my primary concern with this PR. I just wanted to make sure the file got downloaded.)

dsaltares commented 2 years ago

@sethgupton-mastery we just added support for mac and windows runners, which really changes the implementation (moved to node and TS). Happy to help out move this forward.

sethgupton-mastery commented 2 years ago

¯\(ツ)/¯ at this point you can probably just close this down for a couple reasons. 1) I don't know if the problem I was experiencing would still happen in the node version and 2) I still don't know how it should interact with the regex option that was added.

dsaltares commented 2 years ago

Yes, I think you're right @sethgupton-mastery. Thanks for understanding!