-
Notifications
You must be signed in to change notification settings - Fork 30k
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
esm loader error not catchable #24175
Comments
nvm i see what you mean now, i'm looking into it. |
PR with the test case ready to land as soon this issue is resolved: #24183 |
@MylesBorins who is the best person to ask to take a look at this to confirm if our understanding that this is a bug is correct? |
@mhdawson i'm looking into it now. i can confirm that not being able to catch() this is a bug. |
@devsnek thanks! |
weee okay so the problem is the resolve hook. the actual error being generated is (as i said earlier by coincidence) an early error. it is trying to resolve the test file, but since the resolve hook just returns an empty string, it won't work. if the loader is changed to this it works cc @franher export async function resolve(specifier, parent, defaultResolve) {
if (specifier === 'does_not_exist') {
return { url: 'does_not_exist', format: 'builtin' };
}
return defaultResolve(specifier, parent);
} |
ohh I see @devsnek Thank you for your support. I've just pushed the changes to the PR with the proper resolve hook. I close this issue. |
For code-learn at NodeCONF EU we wrote a new test case to cover https://coverage.nodejs.org/coverage-d32b5bd567544f5b/root/internal/bootstrap/loaders.js.html#L152 . After that, the test case generates the error but we can't catch it.
PR with the test case ready to land as soon this issue is resolved: #24183
The text was updated successfully, but these errors were encountered: