Skip to content

fix: prevent unhandled promise rejection that caused action to exit and retries not to happen #60

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

Merged
merged 5 commits into from
Dec 22, 2022

Conversation

tjenkinson
Copy link
Contributor

@tjenkinson tjenkinson commented Nov 19, 2022

Previously if an error happened in the promise constructor in startSc (like the error that is rethrown as the result of a timeout) it would never be handled anywhere, causing node to exit with an error as a result of an unhandled promise rejection. The promise constructor does not really expect a function that returns a promise, and if it did that rejection would not be handled, as was the case here.

So this PR updates scartSc to not be wrapped in a new Promise.

It also increases the timeout as I saw 45 seconds was sometimes not enough.

fixes #59

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson
which may happen if it already terminated

fixes saucelabs#59

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson
tjenkinson added a commit to video-dev/hls.js that referenced this pull request Nov 19, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@tjenkinson tjenkinson marked this pull request as draft November 19, 2022 13:27

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson
previously if an error happened in the promise constructor it would never be handled anywhere. The promise constructor does not really expect a function that returns a promise, and if it did that rejection would not be handled, as was the case here

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson
because we were seeing timeouts
@tjenkinson tjenkinson changed the title fix: do not error if sc could not be killed fix: prevent unhandled promise rejection that caused action to exit and retries not to happen Nov 19, 2022
@tjenkinson tjenkinson marked this pull request as ready for review November 19, 2022 18:12
tjenkinson added a commit to video-dev/hls.js that referenced this pull request Nov 19, 2022

Verified

This commit was signed with the committer’s verified signature.
tjenkinson Tom Jenkinson
@waggledans waggledans merged commit 8d026ce into saucelabs:main Dec 22, 2022
@tjenkinson
Copy link
Contributor Author

Thanks for merging! Looks like the build needs updating in the main branch though

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.

Action is quitting before retries
2 participants