Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Object oriented error handling #1454

Closed
medikoo opened this Issue · 7 comments

4 participants

@medikoo

There are some cases when we want to check type of Error that thrown.
Node codebase just uses Error object widely (with AssertionError as the only exception), this makes error handling difficult.

Example use case:

try {
  require('/any/path');
} catch (e) {
  // Currently following is not possible
  if (e instanceof ModuleNotFoundError) {
    // alternative approach
  } else {
    throw e;
  }
}

It's just one of the examples, it'll be great to see different error types in all node modules, wherever it makes sense.

@felixge

I don't think we will go for this. Otherwise we'd have to either make all of those error classes global (not gonna happen), or put them all inside a single module. While the later would be doable, it would be a PITA.

The right approach here seems to follow the convention established by other parts of the core, and attach a property on the error.

In this case I could imagine:

if (err.type == 'module_not_found') {

} else if (err.type == 'parse_error') {

}

Or something like this. Maybe more thought should go into the convention here, but a property seems to be the right thing to do.

What do others think?

@medikoo

err.type might work for me, I see that V8 is using that property to provide more specific error 'tag'.

New error classes I would expect to be specific per one given module, exactly as it's with AssertionError - it's used internally just by assert module. Then example above would work with following:

var ModuleNotFoundError = require('module').ModuleNotFoundError;
@AvianFlu

If you make a new Error(), it has a type property by default - it just defaults to undefined. I've seen many modules use it for what you describe, and I'd also recommend it.

@kof

is err.name not the thing you want to modify/check?

if (err.name == 'ModuleNotFound')
@medikoo

I think best solution (as @felixge and @AvianFlu suggested) is to use type property for that, extending Error object is indeed troublesome.

So far type property on error thrown by require is undefined, So we need to to read through error message to decide whether it's module not found error. That is not great (redaction of error message may break the code).

@kof

What about to introduce something like ExtError or NodeError, which will accept a message and additional object. This would make it easy to set type and probably other infos. I use it in my project.

new ExtError('Module not found.', {
  type: 'ModuleNotFound',
  level: 'fatal',
  code: 12345
})
@medikoo

I see that code error property was widely adopted (for module it stands as MODULE_NOT_FOUND).

That's great, I take it as expected solution. Closed.

@medikoo medikoo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.