Skip to content

Is there a typo in the Suppressing Exceptions section? #887

Open

Description

@juanpaco

At https://github.com/rubocop/ruby-style-guide#suppressing-exceptions, it says "Don't suppress" exceptions, but then lists do_something rescue nil as a good example.

# good
do_something rescue nil

It could be my ignorance, but that reads like an example of suppressing an exception. Am I misunderstanding? Apologies if so.

Activity

  1. pirj commented on Oct 6, 2021

    @pirj
    Member

    It indeed looks confusing.

    It was originally introduced as a bad example, see 4240682#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R961

    This is how bad turned good 3a20519

    Would you like to add a brief explanation to why it's a good (fair use) example?
    I can suggest digging through real-world-rails/real-world-ruby-apps for inspiration.

    Here's an example of how it can look like:

    # good - it's apparent what's going on
    
  2. juanpaco commented on Oct 6, 2021

    @juanpaco
    Author

    I'm definitely happy to help the project--- I'm struggling with my orthodoxy of not seeing rescue nil as acceptable in any case, heh. I'll need to come to terms with that. I'll browse in those examples.

    I'm fine if you want to close the issue so that it doesn't add noise.

    Thank you for the super fast and detailed response!

  3. gpopescu commented on Jan 9, 2025

    @gpopescu

    Add one more to the disagree pile. inline rescue nil is a code smell and big no-no as it does in fact mask exceptions you haven't even considered yet. What about instead of #good or #bad , something like #questionable or #only_ok_in_some_cases

  4. pirj commented on Jan 9, 2025

    @pirj
    Member

    This is what I meant above with “fair use”.
    A better explanation will be:

    # ok’ish - I fully acknowledge what exception was suppressed 
    File.delete(config_file_name) rescue nil # NOTE: we don’t care if the file is missing, or we lack permissions, or it’s a directiry. We just swallow the exception, as there is no point to recover.
    

    If you agree, a PR is welcome.

    As it was pointed out in prior changes, returning nil can be deliberate choice.
    But we only can consider it “returning” at the end of a method/block.
    Otherwise, this is pure suppression, and should conscious and deliberate for a good reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions