Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Set the 'errno' and 'code' of a module when not found. #2358

Closed
wants to merge 7 commits into from
Closed

Set the 'errno' and 'code' of a module when not found. #2358

wants to merge 7 commits into from

Conversation

TooTallNate
Copy link

I'm not sure if you guys will take this one, but basically I'm currently sniffing "not find" from the error object's message, which is rather hacky. So checking this property is a little cleaner approach.

Example of what I'm talking about:
https://github.com/TooTallNate/node-weak/blob/1781429d8590eaf70c7fe64d87dbc642af6d1e1d/lib/bindings.js#L13

@felixge
Copy link

felixge commented Dec 18, 2011

-1 on this particular solution.

Setting err.code to ENOENT does not allow to differentiate between the module not being found, vs. the module doing something like this:

fs.readFileSync('does-not-exist.json');

I would propose setting err.code to 'MODULE_NOT_FOUND' and making it part of the official module loading API (by that I mean documenting it).

@TooTallNate
Copy link
Author

Good point @felixge, I hadn't considered the case where the module itself throws a similar error. Then in the MODULE_NOT_FOUND case, what should we set the errno to?

@felixge
Copy link

felixge commented Dec 18, 2011

errno is deprecated. See #2211 (comment)

I'll submit a patch for the docs mentioning that, as well as a patch for the master branch to remove the remaining errno properties.

@TooTallNate
Copy link
Author

Ok nice sounds good, I'll update this pull with the 'MODULE_NOT_FOUND' code, and tests and docs.

@TooTallNate
Copy link
Author

@felixge Ok fixed up with your suggestions. Thanks!
@ry @bnoordhuis @isaacs or any of the other committers have any thoughts?

@TooTallNate
Copy link
Author

@koichik Thanks for the suggestions; they've been implemented. Test passes. This pull should be ready.

@koichik koichik closed this in 3f987cd Dec 19, 2011
@koichik
Copy link

koichik commented Dec 19, 2011

@TooTallNate Thanks, merged in 3f987cd, ec11525, and 855f466.

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

Successfully merging this pull request may close these issues.

None yet

3 participants