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

Errors downloading files from s3 are propagated to incorrect thread #2946

Closed
DanielHeath opened this issue Nov 20, 2023 · 8 comments · Fixed by #2954
Closed

Errors downloading files from s3 are propagated to incorrect thread #2946

DanielHeath opened this issue Nov 20, 2023 · 8 comments · Fixed by #2954
Labels
bug This issue is a bug. investigating Issue is being investigated

Comments

@DanielHeath
Copy link

Describe the bug

While downloading files from S3, any exceptions are propagated to the main thread instead of the calling thread.

This appears to be due to the use of Thread.abort_on_exception (see

thread.abort_on_exception = true
).

Expected Behavior

Exceptions propagate to the calling context

Current Behavior

Exceptions are propagated to the main thread, interrupting unrelated code.

Reproduction Steps

workers = []
workers << Thread.new do
  begin
    Aws::S3::Object.new(bucket_name: "test", key: "fails_to_download").download_file("outputfile")
  rescue
    puts "We should get here if the download failed, but do not."
  end
end
workers.each &:join

puts "We should get here regardless of the download failing or succeeding, but do not"

Because the error is propagated to the main thread instead of the calling thread, and the main thread has no exception handler, the process aborts unexpectedly.

Possible Solution

Instead of using abort_on_exception, store the calling thread at the start of download_in_threads and use calling_thread.raise(exception) to propagate errors.

Additional Information/Context

No response

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-s3

Environment details (Version of Ruby, OS environment)

3.2.2 / linux

@DanielHeath DanielHeath added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 20, 2023
@DanielHeath DanielHeath changed the title Errors downloading files from s3 are propagated to incorrect rhead Errors downloading files from s3 are propagated to incorrect thread Nov 20, 2023
@jterapin
Copy link
Contributor

Thanks for reporting this, we will be taking a look.

@jterapin jterapin added investigating Issue is being investigated and removed needs-triage This issue or PR still needs to be triaged. labels Nov 20, 2023
@jterapin
Copy link
Contributor

Hi! Which version are you using for the aws-sdk-s3? Is it the latest, which is 1.137.0?

@DanielHeath
Copy link
Author

aws-sdk-s3 (1.136.0) - will update and confirm it affects 137

@DanielHeath
Copy link
Author

Yep, still an issue.

@mullermp
Copy link
Contributor

Instead of using abort_on_exception, store the calling thread at the start of download_in_threads and use calling_thread.raise(exception) to propagate errors.

Is removing abort_on_exception sufficient here? Thread's value method will raise the exception that terminated the thread, presumably bubbling up to the calling thread automatically.

@DanielHeath
Copy link
Author

Ahh that's a good point - simply removing abort_on_exception does appear to fix it.

Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@jterapin
Copy link
Contributor

Thank you for your patience! The fix has been merged into main but a new version of S3 gem will not be released until around Dec 4 (due to reInvent). Apologizes for the inconvenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. investigating Issue is being investigated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants