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

HTTP imports not found error #41948

Closed
guybedford opened this issue Feb 12, 2022 · 10 comments
Closed

HTTP imports not found error #41948

guybedford opened this issue Feb 12, 2022 · 10 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders stale

Comments

@guybedford
Copy link
Contributor

What is the problem this feature will solve?

When a 404 not found response is made for an HTTP/s import, the error thrown is currently:

Error [ERR_NETWORK_IMPORT_BAD_RESPONSE]: import 'https://h3manth.com/foo' received a bad response: HTTP response returned status code of 404

it might be useful to convert this into a full ERR_MODULE_NOT_FOUND error to properly match semantics in error handling code paths.

What is the feature you are proposing to solve the problem?

Return an ERR_MODULE_NOT_FOUND for 404 network errors in HTTP/s imports.

What alternatives have you considered?

Keeping it as a custom error or network not found error creates divergence between local and remote resolvers. It can be useful to streamline the error path handling by having a singular code to check.

@guybedford guybedford added the feature request Issues that request new features to be added to Node.js. label Feb 12, 2022
@guybedford
Copy link
Contributor Author

/cc @JakobJingleheimer would be interested to get your thoughts on this one.

@Mesteery Mesteery added the esm Issues and PRs related to the ECMAScript Modules implementation. label Feb 12, 2022
@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Feb 12, 2022

That sounds appropriate to me! Should we also handle other 4XX response statuses like 401 & 403? I took a quick look through existing error codes, and I didn't see anything appropriate for those two (so perhaps a new ERR_NETWORK_AUTH_FAILURE or following fs access failures, ERR_NETWORK_EACCESS?).

@Mesteery Mesteery added the loaders Issues and PRs related to ES module loaders label Feb 12, 2022
@benjamingr
Copy link
Member

That whole function can be simplified significantly by avoiding new Promise I believe

@benjamingr
Copy link
Member

Should I maybe add the 404 error handling to #41950 ?

@guybedford
Copy link
Contributor Author

Would be great to see actually.

@benjamingr
Copy link
Member

There is a second question here of "whether 404s should be cached or should a second import attempt attempt to make the request again" - but I won't change it in that PR

@benjamingr
Copy link
Member

Done + added "fixes" #41950

@benjamingr
Copy link
Member

Would be great to get some reviews on that PR from people interested in the feature request :)

@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 Aug 16, 2022
@benjamingr
Copy link
Member

Was fixed a while ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js. loaders Issues and PRs related to ES module loaders stale
Projects
None yet
Development

No branches or pull requests

4 participants