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

Issue #533: rescue StandardError only #535

Merged

Conversation

ColinDKelley
Copy link
Collaborator

Address issue #533 by only rescuing StandardError (the default) rather than Exception.

@ColinDKelley
Copy link
Collaborator Author

@ioquatix can you please review?
This is what you originally recommended: simply rescue StandardError. At the time I argued to rescue more, because Ruby could wind up burying other exceptions. But I'm backing away from that based on

  1. Noise that was getting logged as reported here: Don't log SystemExit as an error #532
  2. The fact that modern Ruby has report_on_exception = true by default, so exceptions that sneak by will still be logged to (to STDERR I believe, which is good enough in k8s). Also, such exceptions get re-raised if you join the thread later. These seem sufficient to address my concerns.

@ColinDKelley ColinDKelley changed the title Issue 533/rescue standard error only Issue #533: rescue StandardError only Mar 20, 2021
@ColinDKelley ColinDKelley self-assigned this Mar 20, 2021
@ColinDKelley ColinDKelley force-pushed the issue-533/rescue-standard_error-only branch 7 times, most recently from f13a358 to 5cf38e4 Compare March 21, 2021 04:50
@ColinDKelley ColinDKelley force-pushed the issue-533/rescue-standard_error-only branch from 5cf38e4 to 57224cc Compare March 21, 2021 04:59
@ioquatix
Copy link
Member

When rescuing StandardError I normally call the variable error rather than exception.

LGTM.

@ColinDKelley ColinDKelley merged commit 133b1bf into guard:master Mar 22, 2021
@ColinDKelley ColinDKelley deleted the issue-533/rescue-standard_error-only branch March 22, 2021 00:59
@ColinDKelley
Copy link
Collaborator Author

@ioquatix Re error vs. exception, I decided to leave it exception since all the surrounding methods call it that too. I don't see any harm in being a bit more precise. (All exceptions are errors, but not all errors are exceptions.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants