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

Invalid URLs in static imports do not produce a helpful error message #49571

Closed
renhiyama opened this issue Sep 9, 2023 · 4 comments · Fixed by #49736
Closed

Invalid URLs in static imports do not produce a helpful error message #49571

renhiyama opened this issue Sep 9, 2023 · 4 comments · Fixed by #49736
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@renhiyama
Copy link

Version

v18.17.1

Platform

Linux desktop 6.1.51 #1-NixOS SMP PREEMPT_DYNAMIC Sat Sep 2 07:16:20 UTC 2023 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

test.js file:

import "file://hmm.js";

Run this test.js file. DO NOT CREATE hmm.js file.

How often does it reproduce? Is there a required condition?

Always. The required condition for the importing file to not exist.

What is the expected behavior? Why is that the expected behavior?

$ node test.js
node:internal/errors:496
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be "localhost" or empty on linux
    // error stack should point to my file, here in this case, "test.js:1:7" or something similar.
   // it would be helpful in debugging and find out where the problem is in my code...
}

What do you see instead?

$ node test.js
node:internal/errors:496
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be "localhost" or empty on linux
    at new NodeError (node:internal/errors:405:5)
    at getPathFromURLPosix (node:internal/url:1381:11)
    at fileURLToPath (node:internal/url:1404:50)
    at finalizeResolution (node:internal/modules/esm/resolve:294:14)
    at moduleResolve (node:internal/modules/esm/resolve:943:10)
    at defaultResolve (node:internal/modules/esm/resolve:1129:11)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:835:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:77:40) {
  code: 'ERR_INVALID_FILE_URL_HOST'
}

As seen above, nodejs logs internals, instead of linking to my source code. Makes it harder for debug.

Additional information

No response

@renhiyama
Copy link
Author

For reference, Deno seems to correctly link the file too.

deno run -A ./test.js 
error: Invalid file path.
  Specifier: file://hmm.js/
    at file:///home/ren/coding/oo/test.js:1:8

@bnoordhuis bnoordhuis added the esm Issues and PRs related to the ECMAScript Modules implementation. label Sep 9, 2023
@aduh95 aduh95 changed the title TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be "localhost" or empty on (linux/darwin/windows) Invalid URLs in static imports do not produce a helpful error message Sep 10, 2023
@aduh95
Copy link
Contributor

aduh95 commented Sep 10, 2023

@nodejs/loaders

@GeoffreyBooth GeoffreyBooth added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Sep 11, 2023
@Manish4044
Copy link

@renhiyama Adding JsFileName, ImportFilePath (wrong or non-existent), and its LineNumber will be helpful enough in the logs or some other things also be taken into consideration.

@JakobJingleheimer
Copy link
Member

This is coming from esm/resolve finalizeResolution()fileURLToPath(), which doesn't have the parent module's info. ERR_INVALID_FILE_URL_HOST is used only by fileURLToPath()getPathFromURLPosix(), so we could supply it, but fileURLToPath() is used a quite a lot of places.

Option 1: I think it should be safe though to add a 2nd, optional argument to fileURLToPath() to supply parentURL, but that will involve a bit of daisy-chaining.

Option 2: Duplicate the check getPathFromURLPosix() is doing, putting it in finalizeResolution().

I think option 1 is less bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants