-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Catch and log unhandled exceptions stemming from closed file descriptor #596
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
konradpabjan
approved these changes
May 27, 2021
This was referenced May 27, 2021
This was referenced May 27, 2021
1 task
This was referenced May 27, 2021
This was referenced May 27, 2021
1 task
This was referenced Jun 12, 2021
This was referenced Jun 20, 2021
This was referenced Jul 1, 2021
This was referenced Jul 9, 2021
This was referenced Jul 19, 2021
bors
added a commit
to rust-lang/crates.io
that referenced
this pull request
Jul 26, 2021
Update actions/cache action to v2.1.6 [](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [actions/cache](https://togithub.com/actions/cache) | action | patch | `v2.1.4` -> `v2.1.6` | --- ### Release Notes <details> <summary>actions/cache</summary> ### [`v2.1.6`](https://togithub.com/actions/cache/releases/v2.1.6) [Compare Source](https://togithub.com/actions/cache/compare/v2.1.5...v2.1.6) - Catch unhandled "bad file descriptor" errors that sometimes occurs when the cache server returns non-successful response (actions/cache#596) ### [`v2.1.5`](https://togithub.com/actions/cache/releases/v2.1.5) [Compare Source](https://togithub.com/actions/cache/compare/v2.1.4...v2.1.5) - Fix permissions error seen when extracting caches with GNU tar that were previously created using BSD tar (actions/cache#527) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box. --- This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/rust-lang/crates.io).
1 task
1 task
777666007
reviewed
Nov 14, 2021
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.
etiennebarrie
added a commit
to Shopify/maintenance_tasks
that referenced
this pull request
Nov 7, 2022
The error seems similar to actions/cache#596 and ruby/setup-ruby has actions/cache locked to a version not including that fix.
lawrencewong
pushed a commit
to lawrencewong/maintenance_tasks
that referenced
this pull request
Apr 29, 2023
The error seems similar to actions/cache#596 and ruby/setup-ruby has actions/cache locked to a version not including that fix.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to how
uploadChunk
is implemented, an error while uploading a chunk causesPromise.all
to reject with that error. This then closes the file descriptor inside thefinally
block. However, if there are any async uploads still running, closing the file descriptor may cause I/O operations to fail withbad file descriptor
or similar errors. But most importantly, these errors are uncaught, even though this is all run inside try-catch blocks.Fixes #154, #441, #467, and actions/toolkit#612
This solution is a bit heavy handed as it just converts all unhandled exceptions to warnings. I also thought about fixing
uploadChunk
so we don't close the file descriptor until all upload promises complete, but this may cause slowness. Closing the file descriptor kills all uploads and lets the action terminate quickly.Repro
The following script can reliably reproduce this issue. It essentially create a large file that will take some time to read, then creates two promises: one reads the file and the second causes an error (rejects the promise). The error closes the file descriptor which interrupts the read, producing a "bad file descriptor" error.
It doesn't matter if we have try-catch blocks or using
run().catch(...)
, the bad file descriptor error still makes its way out and is only caught by theunhandledException
event: