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

ERR_MODULE_NOT_FOUND: add a property to exception object to hold failing import's argument #37581

Closed
rulatir opened this issue Mar 3, 2021 · 20 comments
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.

Comments

@rulatir
Copy link

rulatir commented Mar 3, 2021

Is your feature request related to a problem? Please describe.

I am developing a framework that uses node as a host for user scripts. The framework loads them dynamically with the async import() function, and the userscripts can also import other modules with import statement.

When I call that import() function, I basically want to rethrow all errors thrown by it, except for one very specific condition: when the import() function fails with ERR_MODULE_NOT_FOUND because the userscript file itself wasn't present in the location just tried. The userscript might still be found in some other search path, which is why its absence in any given candidate location is not an error. If however the import() function failed with ERR_MODULE_NOT_FOUND because some other module imported by the userscript was not found, then I want to rethrow that error.

Describe the solution you'd like

Add a property to the exception object thrown for ERR_MODULE_NOT_FOUND that will hold the exact value of the argument that was given to the failing import function or statement.

Describe alternatives you've considered

I considered checking for existence first and then attempting to import(), but that is a known antipattern.

I considered parsing the message or stack property, but that would be a hack. These properties are probably locale dependent and their format is not set in stone.

I looked into custom loaders and found the subject difficult, scary, apparently indefinitely experimental, and an overkill for this particular purpose. I need a simple and reliable way.

@DerekNonGeneric
Copy link
Contributor

Thanks for raising this — the request sounds reasonable.

Hope to take a deeper look a bit later. Curious what others may think on the matter.

/cc @nodejs/modules

@ljharb
Copy link
Member

ljharb commented Mar 4, 2021

A custom loader doesn't sound like overkill at all to me - it's precisely what you're trying to achieve. The other adjectives, however, are likely true at this time.

@rulatir rulatir changed the title ERR_MODULE_NOT_FOUND: add property to exception object to store the argument to failing import ERR_MODULE_NOT_FOUND: add a property to exception object to store the argument to failing import Mar 5, 2021
@rulatir rulatir changed the title ERR_MODULE_NOT_FOUND: add a property to exception object to store the argument to failing import ERR_MODULE_NOT_FOUND: add a property to exception object to hold the argument to failing import Mar 5, 2021
@rulatir rulatir changed the title ERR_MODULE_NOT_FOUND: add a property to exception object to hold the argument to failing import ERR_MODULE_NOT_FOUND: add a property to exception object to hold failing import's argument Mar 8, 2021
@PoojaDurgad PoojaDurgad added module Issues and PRs related to the module subsystem. feature request Issues that request new features to be added to Node.js. labels Mar 10, 2021
@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 22, 2022
@rulatir
Copy link
Author

rulatir commented Mar 27, 2022

@DerekNonGeneric is it "a bit later" yet? ;)

@GeoffreyBooth
Copy link
Member

Doesn’t the error message included with ERR_MODULE_NOT_FOUND have the file path you’re asking for? Yes it’s inside an English sentence that you’d need to parse out, but even if the message itself changes in the future, code like error.description.includes(fullPathToUserScript) would work as long as the path to the missing module never gets removed (which is very unlikely). Seeing as your use case is very specific, I don’t know if it’s likely that Node will add this path as its own property on the error object.

@github-actions github-actions bot removed the stale label Mar 29, 2022
@rulatir
Copy link
Author

rulatir commented Apr 22, 2022

Yes it’s inside an English sentence

... when node is running with English locale.

@GeoffreyBooth
Copy link
Member

… when node is running with English locale.

Does Node have non-English error messages when it’s running in other locales?

@rulatir
Copy link
Author

rulatir commented Apr 25, 2022

Does Node have non-English error messages when it’s running in other locales?

It either has, or might have at some point in the future, which is still a sufficient reason not to rely on parsing natural language.

@ljharb
Copy link
Member

ljharb commented Apr 25, 2022

Even if it wouldn’t ever be anything but English, i would think it’d be a horrifying suggestion to tell people to parse natural language. The message is for humans, only - the stuff for programs is “other properties”.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Oct 23, 2022
@rulatir
Copy link
Author

rulatir commented Nov 5, 2022

No bot, no. I'm still rooting for this.

@github-actions github-actions bot removed the stale label Nov 6, 2022
@GeoffreyBooth
Copy link
Member

Node.js recently added support for Error.cause: https://nodejs.org/api/errors.html#errorcause, e54ee80. I would assume that the ESM loader’s ERR_MODULE_NOT_FOUND error is thrown as a result of some file system “file not found”-type error; the earlier exception could be included in the cause property of the ERR_MODULE_NOT_FOUND exception. I would think that this should satisfy your use case.

@rulatir
Copy link
Author

rulatir commented Nov 14, 2022

It wouldn't. I need the exact byte-for-byte string that was passed as the argument to import(), rather than the filesystem path it resolved to via a possibly unknown mapping function that may not even be invertible. I never said that what I pass to import is a fully resolved filesystem path. Those can well be module references to be resolved by the default loader, but still my code wants to be able to try several candidates in order, and if import() fails, I need to be able to distinguish whether it failed directly (try another candidate) or indirectly (fail).

EDIT: it might work if the entire chain down to the failed dependency is recorded using the cause references, i.e. ERR_MODULE_NOT_FOUND because ENOENT means a direct failure and ERR_MODULE_NOT_FOUND (because ERR_MODULE_NOT_FOUND)+ because ENOENT means an indirect failure.

@rulatir

This comment was marked as abuse.

@bnoordhuis
Copy link
Member

Is the resolution here to add a custom loader with --experimental-loader? In the two years since this issue was raised loaders have been documented quite thoroughly.

My intuition is that adding more properties to the ERR_MODULE_NOT_FOUND exception object complicates writing third-party loaders but maybe I'm mistaken here.

@GeoffreyBooth
Copy link
Member

Is the resolution here to add a custom loader

I think that's one way to achieve the request today, yes, but it's a bit much to write a custom loader to improve a Node error message. I think a reasonable solution would be to add the specifier of the failing import to the cause property of the error object. Pull requests are welcome.

@rulatir
Copy link
Author

rulatir commented Mar 18, 2023

The rationale for my original request is not to improve error messages, though that would be an extra benefit. I need the feature to be able to implement special-purpose loading logic correctly in the first place, without resorting to the check-existence-and-load antipattern. As it is, the error doesn't supply necessary information to make the key decision in the algorithm: whether to try another candidate specifier (because this one didn't resolve to an existing file) or to rethrow (because the file was found, but contained errors).

I insist that the first question one asks when being presented with an error object is, "What failed?", and answering this question is the error object's primary job. In this case the specifier is the what.

Unless someone is doing something really exotic with ERR_MODULE_NOT_FOUND objects, like validating their structure against an inextensible schema, I can't imagine a scenario where adding a super-relevant property under already standardized cause subobject would "complicate writing third-party loaders".

@GeoffreyBooth
Copy link
Member

my original request

@rulatir This request has been open for two years without anyone expressing interest in implementing this enhancement. If you want to see it happen you might need to make a PR yourself. A good place to start is https://github.com/nodejs/node/blob/main/CONTRIBUTING.md.

@rulatir
Copy link
Author

rulatir commented Mar 18, 2023

@rulatir This request has been open for two years

... and custom loaders are still under a flag.

without anyone expressing interest in implementing this enhancement. If you want to see it happen you might need to make a PR yourself. A good place to start is https://github.com/nodejs/node/blob/main/CONTRIBUTING.md.

If I must in fact sit down and write the code myself, what provisions in the LICENSE compel or incentivize me to subject myself to new contributor hazing invest additional time and effort to conform to the contributing process? Can't I just happily use my fork in-house without ever distributing it to third parties? Why should I invest a difficult-to-estimate amount of time to study all the documents linked to from CONTRIBUTING.md to learn how to be a good nodejs contributor boy, all this striving and becoming just to be allowed to contribute what is likely to amount to ~10 LOC plus a paragraph of markdown?

@bnoordhuis
Copy link
Member

Okay, close it is. I don't see this discussion taking a productive turn. @rulatir don't let the door hit you on the way out.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants