-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
request: expose Node's error system #47521
Comments
Are there any downsides to doing it? |
Nothing immediately stands out to me since the errors are pretty stable at this point. If there are individual errors that don't make sense to expose, we could discuss that as well. |
One downside is performance, especially node errors are very slow to create. nodejs/performance#40 written by @ronag |
I think the Node error performance was significantly improved though. Don't remember which PR. |
@addaleax makes a good point on twitter regarding one issue with our current errors.
Source: https://twitter.com/addaleax/status/1646155989261987840 |
I think @addaleax's comment is extremely valid, and it would be wonderful if this effort provided a motivation to polish up the error internals. That said, whatever comes of this, I'd use the ever loving heck out of it, and I'd be motivated to polyfill in userland support for node versions that don't have it yet. I maintain a lot of fairly low-level utility libraries, and always try to follow Node's patterns wrt error codes and such, but it's always been a bit of a duck-typing mess. Exposing |
I created https://github.com/fastify/fastify-error to basically solve this problem. I can't find any issue, but I recall having some conversations with other folks and the general sentiment was that exposing it would not have been a good idea (e.g. small core). I'm definitely +1, as it would one less module for me to maintain long term :D. We should also take the moment to polishing the internals and create some useful utilities. On that note, we should include also utilities to generate warnings, as we had to write https://github.com/fastify/process-warning to match core behavior. |
I'm +1. |
Yeah, process.emitWarning can be a bit clunky to use properly. But I think that probably ought to be a separate issue? Or is it connected to the error types in some way? |
The underlining mechanisms to provide a good DX are quite similar. |
Why was this closed? |
I'm not going to work on this - there were some additional reservations about exposing this voiced by @joyeecheung on twitter - and I'm cleaning up my open issues list. |
What is the problem this feature will solve?
It would allow userland to leverage core's error system. It would also make discussions around vendoring ecosystem modules into core go more smoothly (if package authors were willing to use it).
I am willing to do the work if people think this is a good idea.
What is the feature you are proposing to solve the problem?
Exposing the constructors for https://nodejs.org/api/errors.html#nodejs-error-codes to userland.
What alternatives have you considered?
Status quo
The text was updated successfully, but these errors were encountered: