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

standard way to ignore an error #620

Closed
timoxley opened this issue Sep 13, 2016 · 6 comments

Comments

@timoxley
Copy link
Contributor

commented Sep 13, 2016

This is mostly just a discussion item rather than a serious issue.

Currently standard uses handle-callback-err which complains if you do not "handle" callback errors.

Sometimes you do want to not handle an error e.g. ignore mkdirp errors.

The current rule is good as it encourages you to explicitly flag intentionally ignored errors rather than just leaving the future reader wondering whether the error was left ignored accidentally or for some purpose.

Even prior to using standard, I try to always flag explicitly ignored errors with a comment like so

mkdirp('/dir/might/exist', err => {
  // ignore error
  doStuff()
})

This only changed slightly upon starting to use standard:

mkdirp('/dir/might/exist', err => {
  err // ignore error 
})

try {
  mkdirp.sync('/dir/might/exist')
} catch (err) {
  err // ignore err
}

Another option could be to use a different variable name for ignored errors:

mkdirp('/dir/might/exist', ignoreErr => {
  doStuff()
})

Does anyone else have a standard pattern they use for flagging "ignored" errors?

It's a possibility that even with the comment this is still a crappy workaround and I really should be explicitly whitelisting the error.code === 'EEXIST' case and throw err/return callback(err) otherwise.

Note that it's also still valid to not add any comment, which makes it again unclear whether the error is being ignored intentionally or not:

mkdirp('/dir/might/exist', err => {
  err
  doStuff()
})

This should perhaps be disallowed.

In fact, I'm not sure there's any valid reason for a statement to solely contain a reference to a variable like that since it's a noop so perhaps I'm really requesting a rule for something like "no useless statements".

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

so perhaps I'm really requesting a rule for something like "no useless statements".

👍 , I wonder if we can apply to standard to politics.

In any case, agreed. My go-to for ignoring errors is:

mkdirp('/dir/might/exist', () => {
  doStuff()
})

Quite literally... waves hand "there is no error".

@timoxley

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2016

@dcousens what if you need to access the other arguments?

Yeah I personally wish that particular case were detectable to and required some explicit signal that you hadn't inadvertently invited chaos into your program by forgetting to handle the err. It'd be possible to detect for node core functions I guess.

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

Yeah, I suppose I just use _ in that case.
You're right ignoredErr would be better or some such maybe.

@LinusU

This comment has been minimized.

Copy link
Member

commented Sep 14, 2016

I think _ is good enough, most people knows that it means ignored variable and I believe that standard doesn't even complain of unused variables if it's named _...

@feross

This comment has been minimized.

Copy link
Member

commented Sep 14, 2016

Yeah, _ is a nice solution. That's what I use. But this is a pretty rare occurrence.

@feross feross added the question label Sep 14, 2016

@feross

This comment has been minimized.

Copy link
Member

commented Feb 9, 2017

Closing, since discussion has concluded 😄

@feross feross closed this Feb 9, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
4 participants
You can’t perform that action at this time.