-
Notifications
You must be signed in to change notification settings - Fork 64
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
Prevent spinnies error on upload if buildPolling fails #562
Conversation
deployedBuildId | ||
); | ||
} catch (err) { | ||
logger.error(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate on why moving this here fixes the problem?
Also, what type of errors can exist here? Are they api-related errors, or internal JS errors? If it's the latter I think we should switch this to be logger.debug()
so we don't fill the console with internal js errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call on error types, I'll address that.
Also, after looking at this again don't think think that this change is relevant here. It is in upload.js, so I'll add a comment there
); | ||
await pollBuildStatus(accountId, projectName, upload.buildId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this fails, the spinnies.fail()
within the catch statement may throw an error because the spinner was already removed above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me 👍
Description and Context
When polling was sharing a catch block with the upload function, which was causing issues where an inactive "spinner" was incorrectly updated.
Screenshots
TODO
Who to Notify