Repository navigation
Should Node.js standard library throw custom Error subclasses? #8342
Description
Activity
A couple of immediate thoughts:
- Node.js does not implement those *Error objects, V8 does
- -1 to exporting random non-core-related Error objects like
DatabaseError, IMHO that is out of scope for node
DatabaseErrorprobably does not makes sense. Most database errors would beNetworkError. There may be query-related errors, but those should probably be more specific to the particular database module, so I'm also -1 on that.The other stuff sounds good to me though. Being able to type-check errors would be fantastic.
@jasnell I like the error code idea too. I'd actually like to see both though. Error types let us group errors by category to handle them similarly. For example, there's multiple ways in which network access can error, so you may want the detail to see if retrying is a valid approach. However, in a higher level API, you may receive network errors alongside other non-network errors, so branching logic on that error type would be quite helpful. 😸
-1 to defining new error types.
I just want to point out you can use stuff like
.catchin bluebird without error subclasses - by using something like https://github.com/petkaantonov/core-error-predicates .I'm also -1 on this at this point since it seems like a lot of effort to replace a system that already works (the error codes).
I would prefer a non-native Error subtype for Node's own errors, if anything for easier differentiation at a glance and a better ability to use them in testing (no need to reimplement Node's magic for a mock FS error).
I am 👎 for a complex error hierarchy (that's just not very idiomatic, and it's unnecessarily complex), but something like just
FSError,NetworkError,CryptoError, etc. (just the high level) would suffice.Reacted by Sindre Sorhus and Steven VachonThis issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.
@Trott Can this be re-opened please? At the very least node should have a SystemError class. With typescript becoming more and more popular the lack of types is becoming more troublesome.
I'm going to re-open, but I suspect this will get closed pretty quickly. I could certainly be wrong about that, though. I think there will be a lot of resistance among Node.js core devs to add this stuff and support it. But let's see....
@mscdex @benjamingr @cjihrig You're all still -1 on adding something like
SystemError?Reacted by Almenon14 remaining items
That’s a disadvantage since
instanceofis both unreliable and doesn’t work cross-realm.Any types node adds should offer a robust cross-realm brand checking mechanism.
Reacted by Ben Noordhuis- Potentially OT, but I'd like a way to create synthetic system errors, especially common FS errors like ENOENT, from Node's API. All I really need is the constructor part, not the subclassing part. (Please point me to the right issue if this isn't the correct place to request this.)…On Tue, Feb 19, 2019 at 15:37 Jordan Harband ***@***.***> wrote: That’s a disadvantage since instanceof is both unreliable and doesn’t work cross-realm. Any types node adds should offer a robust cross-realm brand checking mechanism. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#8342 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AERrBOfCyt71DQdcqIFj4FdwW_3Q0fzSks5vPGCagaJpZM4JwyAA> .
That’s a disadvantage since
instanceofis both unreliable and doesn’t work cross-realm.Do you think there would be any chance to add a new language feature that could replace
instanceofin favor of one which works cross-realm?Any types node adds should offer a robust cross-realm brand checking mechanism.
I agree about that and to do so we could use another property similar to the
codeproperty which groups different Node.js errors together.That is not ideal either but it's likely the best we can do at the moment.
@BridgeAR I would dearly love one, but https://github.com/jasnell/proposal-istypes was the last attempt.
A
.codeproperty isn't sufficient; it'd need to be likeArray.isArray- a static brand-checking method.There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.
As we know, ECMAScript define only few
Errorsubclasses. W3C specifications adds to this listDOMExceptionandURIError. Right now, Node.js implements sevenErrors:On other side we have Java standard library that defines 74 classes extending java.lang.Exception and numerous other extending these subclasses. Most of them doesn't make sense in Node.js context, as our standard library is much smaller, but I'm convinced that 7 errors defined in ECMAScript are too general to provide meaningful information on types of error that can happen.
Therefore, I propose:
SystemError(class SystemError extends Error)FileSystemErrorandNetworkErrorto inherit fromSystemErrorDeprecationErrorinheriting fromErrrorfor deprecated featuresErrorin standard libraryDisadvantages:
err.constructor === TypeErrorcan breakAdvantages:
.catch(klass, handler)easier to useLoose ideas:
DatabaseErrorto be subclassed by database modulesI'm working on pull request to replace
new Errorwithnew TypeErrorornew RangeErrorwherever it makes sense.