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

Incorrectly cleans file:// paths #60

Open
jaydenseric opened this issue May 20, 2021 · 4 comments · May be fixed by #72
Open

Incorrectly cleans file:// paths #60

jaydenseric opened this issue May 20, 2021 · 4 comments · May be fixed by #72

Comments

@jaydenseric
Copy link

file:// URL paths can only be absolute:

file: URLs are always absolute paths.
https://nodejs.org/api/fs.html#fs_file_url_paths

Yet the clean method strips out the CWD from the file: URLs, leaving the file:// at the start, making them invalid.

This is a pressing issue, because the error stack from errors thrown in an ESM file starts with the file: URL of the ESM. The ecosystem is currently migrating to vanilla ESM (in .mjs files, or .js files with package "type": "module") and more people will start to notice this bug.

In test.mjs:

import StackUtils from 'stack-utils';

const stackUtils = new StackUtils();

try {
  throw new Error('Message.');
} catch (error) {
  console.log(error.stack);
  console.log(stackUtils.clean(error.stack));
}

Then, run node test.mjs and the console log is ([CWD] substituting the absolute CWD path):

Error: Message.
    at file://[CWD]/test.mjs:6:9
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5)
file://test.mjs:6:9

Note the invalid file://test.mjs:6:9, it should be test.mjs:6:9.

@jaydenseric
Copy link
Author

Also, the cleaning needs to scan for file: URL paths with the CWD converted to the URL format, since a traditional file path and URL file path can differ in encoding of certain characters in the path string.

@conartist6
Copy link

I'd help fix this.

@conartist6
Copy link

conartist6 commented Nov 1, 2021

stack-tools can parse and print URIs now!

mshima added a commit to mshima/jest that referenced this issue Jan 29, 2022
mshima added a commit to mshima/jest that referenced this issue Feb 2, 2022
mshima added a commit to mshima/jest that referenced this issue Feb 2, 2022
mshima added a commit to mshima/jest that referenced this issue Feb 2, 2022
mshima added a commit to mshima/jest that referenced this issue Feb 2, 2022
@mshima mshima linked a pull request Dec 4, 2022 that will close this issue
@mshima
Copy link

mshima commented Dec 4, 2022

Workaround is to set a non existing cwd:

const stackUtils = new StackUtils({ cwd: 'I want absolute paths' });

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

Successfully merging a pull request may close this issue.

3 participants