You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
@addaleax makes a good point on twitter regarding one issue with our current errors.
I would ... fix it first? The point of introducing it was to prevent users from parsing error message, but it's actually really not designed in a way that actually removes the need for that
like, now we have things like
E('ERR_NETWORK_IMPORT_DISALLOWED',
"import of '%s' by %s is not supported: %s", Error);
where if you want to get the value of any of the %s ... you need to *parse the error message*
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 process.emitWarning was really nice, I see this as a natural evolution from that.
Reacted by Colin Ihrig, Matteo Collina and Moshe Atlow
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.
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?
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.
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.
Activity
targos commentedon Apr 12, 2023
Are there any downsides to doing it?
cjihrig commentedon Apr 12, 2023
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.
anonrig commentedon Apr 12, 2023
One downside is performance, especially node errors are very slow to create. nodejs/performance#40 written by @ronag
ronag commentedon Apr 12, 2023
I think the Node error performance was significantly improved though. Don't remember which PR.
anonrig commentedon Apr 12, 2023
@ronag It's still open: #46648
cjihrig commentedon Apr 12, 2023
@addaleax makes a good point on twitter regarding one issue with our current errors.
Source: https://twitter.com/addaleax/status/1646155989261987840
isaacs commentedon Apr 12, 2023
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
process.emitWarningwas really nice, I see this as a natural evolution from that.mcollina commentedon Apr 13, 2023
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.
ronag commentedon Apr 13, 2023
I'm +1.
isaacs commentedon Apr 13, 2023
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?
mcollina commentedon Apr 13, 2023
The underlining mechanisms to provide a good DX are quite similar.
ronag commentedon Apr 14, 2023
Why was this closed?
cjihrig commentedon Apr 14, 2023
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.