Skip to content
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

support the new error system from core #297

Closed
mcollina opened this issue Jun 21, 2017 · 4 comments
Closed

support the new error system from core #297

mcollina opened this issue Jun 21, 2017 · 4 comments

Comments

@mcollina
Copy link
Member

mcollina commented Jun 21, 2017

As we talked about it in May, core is implementing error codes, and 'stream' should conform to this rule. The big question is how we port those to readable-stream, as I think it might be worthy to keep consistency.

I do not want a regexp for each error, so we might have to parse the error definition file and/or do some form or processing of that one as well.

cc @jasnell

Example: nodejs/node@d50a802

@jasnell
Copy link
Member

jasnell commented Jun 21, 2017

What I have in mind is creating a new standalone npm module that replicates the functionality of internal/errors, including the various error codes. The readable-stream module can use it as a dependency. All that should have to happen is changing require('internal/errors') to something like require('internal-errors'), and it should just work.

@mcollina
Copy link
Member Author

how big will that module be?

As long as it works down to Node 0.8 (and maybe 0.6) we are good. I'm a bit worried about Error.captureStackTrace().

@jasnell
Copy link
Member

jasnell commented Jun 21, 2017

the Error.captureStackTrace() is minor and can be omitted. It's there largely as a convenience when viewing stack traces. The real concern will be on browsers, which have not had the best track record when it comes to custom errors. The module itself should be rather small (generally limited to a single js file)

@calvinmetcalf
Copy link
Contributor

so last time i tried getting custom errors in firefox and other browsers was not really worth it and you end up getting worse stack traces so we may want to investigate what the case is currently becuase if that still the case just having for readable-streams the case where we just add a few properties to the errors and call it a day may end being the best way forward

mcollina added a commit that referenced this issue Aug 10, 2018
Fixes #283
Fixes #284
Fixes #212
Fixes #297
Fixes #346
Fixes #339
Fixed #335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants