Skip to content

request: expose Node's error system #47521

Description

@cjihrig

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

Activity

  1. targos commented on Apr 12, 2023

    @targos
    Member

    Are there any downsides to doing it?

  2. cjihrig commented on Apr 12, 2023

    @cjihrig
    ContributorAuthor

    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.

  3. anonrig commented on Apr 12, 2023

    @anonrig
    Member

    Are there any downsides to doing it?

    One downside is performance, especially node errors are very slow to create. nodejs/performance#40 written by @ronag

  4. ronag commented on Apr 12, 2023

    @ronag
    Member

    I think the Node error performance was significantly improved though. Don't remember which PR.

  5. anonrig commented on Apr 12, 2023

    @anonrig
    Member

    I think the Node error performance was significantly improved though. Don't remember which PR.

    @ronag It's still open: #46648

  6. cjihrig commented on Apr 12, 2023

    @cjihrig
    ContributorAuthor

    @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*
    

    Source: https://twitter.com/addaleax/status/1646155989261987840

  7. isaacs commented on Apr 12, 2023

    @isaacs
    Contributor

    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.

  8. mcollina commented on Apr 13, 2023

    @mcollina
    SponsorMember

    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.

  9. ronag commented on Apr 13, 2023

    @ronag
    Member

    I'm +1.

  10. isaacs commented on Apr 13, 2023

    @isaacs
    Contributor

    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.

    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?

  11. mcollina commented on Apr 13, 2023

    @mcollina
    SponsorMember

    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.

    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.

  12. ronag commented on Apr 14, 2023

    @ronag
    Member

    Why was this closed?

  13. cjihrig commented on Apr 14, 2023

    @cjihrig
    ContributorAuthor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions