Skip to content
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.

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

Don't silently consume Exceptions when uploading assets #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

soundasleep
Copy link
Contributor

This should resolve #34

@BreadMoirai
Copy link
Owner

BreadMoirai commented Oct 17, 2022

So the previous implementation philosophy is that because there's no current implementation to "cancel the release" in this plugin, we just attempt to power through and even if the release is incomplete, keep it as complete as possible.

What's the new intended behavior here?

the function uploadAssetsToUrl does not currently throw any exceptions if github api rejects the upload...

@soundasleep
Copy link
Contributor Author

Every now and again the upload fails (in my case, because there's already an asset released with the same filename), but the build says it's successful.

This can cause issues down the line, e.g. devs getting confused, later actions failing or working with an inconsistent state, all of which are really expensive to debug!

If there's an error during release, the dev/build should be notified, or at least have the option to be notified, rather than assuming everything is OK.

If you still want to keep this behaviour, what if we could provide a ignoreExceptions = false parameter to the config so that devs could choose to enable early failure?

@BreadMoirai
Copy link
Owner

BreadMoirai commented Oct 17, 2022

You can add an exception in the uploadAssetsToUrl at the end if any of the assets failed. That way we can attempt all the files and mark the task as failed if any of the uploads didn't succeed,

@runningcode
Copy link

Hey there! Any movement on this PR? We are also facing the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release succeeds even though asset upload fails
3 participants