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

Gem rescues Exception, causing OS signals (SIGTERM) to not work #854

Closed
theckman opened this issue Jun 26, 2015 · 4 comments
Closed

Gem rescues Exception, causing OS signals (SIGTERM) to not work #854

theckman opened this issue Jun 26, 2015 · 4 comments

Comments

@theckman
Copy link

I have a utility using this Gem, and noticed that it wasn't respecting a SIGTERM signal being sent (i.e., the process kept running). Upon further investigation, I found that this gem rescues Exception which can cause this behavior.

In Ruby, rescuing Exception includes things beyond just errors. It'll also include signals sent by the operating system (SIGTERM, SIGQUIT, etc). The better form is to use rescue StandardError and to make sure all of your exception classes inherit from StandardError.

@awood45
Copy link
Member

awood45 commented Jun 26, 2015

I found at least one example of this in Version 2 code, but for reference what version of the Gem are you using?

@awood45
Copy link
Member

awood45 commented Jun 26, 2015

So, the one case in V2 where we rescue Exception is a valid case - we are cleaning up our connection pool, then re-raising the error. There shouldn't be any trapping of signals.

Can you let me know the version of the Gem you're using, or any other details about what you are doing when this issue occurs?

@awood45
Copy link
Member

awood45 commented Jun 26, 2015

On an initial pass the same appears to be true in V1 of the Gem as well.

@trevorrowe
Copy link
Member

I did some initial digging. The SDK will rescue exceptions inside the Net::HTTP connection pool. Thisis intentional in the version 1 Ruby SDK because it needs to rescue Timeout::Errors from raised by Net::HTTP. In Ruby 1.8, which is supported by the version 1 SDK, Timeout::Error is not a subclass of StandardError, but rather Exception. This was changed in Ruby 1.9.

So while I am unable to change the rescue in version 1, it is reasonable now to remove it from the version 2 SDK as it no longer supports Ruby 1.8.

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

No branches or pull requests

4 participants