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

Fix artifact upload chunk timeout logic + update tests #1774

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

robherley
Copy link
Contributor

While looking at CI output for:

We had an issue/race where we were trying to do stuff after the test completed:

ReferenceError: You are trying to import a file after the Jest environment has been torn down. From packages/artifact/tests/upload-artifact.test.ts.

Turns out we had a few errors in our tests and I also discovered we were not handling our timeout logic correctly:

The problematic area of the timeout logic was that it was throwing an error within the setTimeout block, which is outside the promise area and went unhandled. To remedy this (and avoid other leaky async code) I swapped it with a setInterval wrapped in a Promise that would check the last chunk upload. Then, I gave this function and the blob upload an AbortController so we can quit everything when we want to. Then, the main await is a Promise.race where we always abort after the first promise resolves.

As for the tests, we were incorrectly mocking over the logic we were testing and throwing immediately. I also took the liberty of updating the rest of the tests since there was a lot of duplication that made each test harder to reason about.

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.

3 participants