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

Error APIs should make it easy to supply an error code property #241

Closed
jasongin opened this issue May 1, 2017 · 7 comments
Closed

Error APIs should make it easy to supply an error code property #241

jasongin opened this issue May 1, 2017 · 7 comments

Comments

@jasongin
Copy link
Member

jasongin commented May 1, 2017

From nodejs/node#12729 (comment)

Node.js errors by convention may have a code property with a value that can be tested programmatically, as a more reliable alternative to comparing error message strings. To encourage N-API add-ons to conform to that convention, we could add APIs like napi_create_error_with_code() and napi_throw_error_with_code() that take the code value as another parameter.

@mhdawson
Copy link
Member

mhdawson commented May 1, 2017

I agree encouraging the use of the code is good, would it make any sense to only offer versions that take a code ? Maybe we'd need a code to indicate no code. I'm just thinking it would be better to not have a whole bunch of functions doing the same thing.

@jasongin
Copy link
Member Author

jasongin commented May 1, 2017

Right, I had been thinking we could add the new APIs in a non-breaking way, but since we're still able to make breaking changes I agree it would probably be better to change the existing APIs now. I'm not sure the code should be required, but at least it would be more clear that something is missing if nullptr is passed as the code parameter.

@jasongin
Copy link
Member Author

jasongin commented May 2, 2017

Node core also sets some other Error object properties: errno, path, syscall, address, port. These are all documented at https://nodejs.org/api/errors.html#errors_system_errors

Does it make sense to add support for a code property without also supporting all those other properties?

@mhdawson
Copy link
Member

The main motivation is around this nodejs/node#11273 We want to decouple error checking from the error string. While errors generated by N-API don't necessarily fall into the same category as those generated by the Node.js itself it is probably good to encourage the use of an error code in addition to the string for the same reason. If errors require other specific values then they can probably be handled by additional n-api calls as we can't allow for all error specific values. (For example port will not apply to most error messages).

@mhdawson
Copy link
Member

So my recommendation is that we add support for an error code in order to encourage good practice and leave it at that.

@mhdawson
Copy link
Member

nodejs/node#13933

@digitalinfinity
Copy link
Contributor

I believe this is addressed with nodejs/node#13988

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants