Malformed source URLs in source map cache on Windows

[PaperStrike]

Search Terms

v8 coverage, coverage, source map windows

Expected Behavior

The sources field of the generated source map cache consists of file URLs (e.g., file:https:///D:/Example/index.ts).

Actual Behavior

Some generated sources field contain malformed file paths (e.g., d:/Example/index.ts).
I think it "malformed" because the disk letter, along with :, is treated like a protocol. At least, it should be in upper case. It will be better if it follows the file:https:/// prefix like the others.

Steps to reproduce the problem

1. Enable Node.js coverage output by setting NODE_V8_COVERAGE
2. Run any .ts file with ts-node
3. Compare the sources fields in each produced coverage output file with each other.

Minimal reproduction

https://github.com/PaperStrike/ts-node-bug-repro
The workflow does basically the steps above, and prints the malformed URLs.

Specifications

• ts-node version: 10.8.0
• node version: 16.15.0
• TypeScript version: 4.6.4
• Operating system and version: Windows 11

[PaperStrike]

This is very likely to be caused by that

• Node.js uses sourcesToAbsolute to parse the paths in the sources field for the source map cache, and as you can see in L181, it relies on the URL constructor,

      return new URL(source, baseURL).href; // baseURL is guaranteed to be a file URL, file:https:///xxx

• Node.js current implementation of the constructor will parse the Windows disk letter, along with :, as the protocol,

  > new URL('D:/Example').href
  'd:/Example'
  > new URL('D:/Example', 'file:https:///Whatever').href
  'd:/Example'

So I suggest we use either file URLs (PaperStrike@134bef2) or relative paths (PaperStrike@2a933ee).

[cspotcode]

Thanks for this. It'll probably take me a few days before I can properly review, merge, and publish this. The push to prepare and publish 10.8.0 ahead of TS 4.7 was time-consuming, so I've had to switch gears this week and play catch-up on some other stuff.

One thing that I want to make sure we get right:

Node's stack traces from ESM files use file URLs, but from CommonJS files, they use native paths. I remember writing some logic to ensure that we preserve this behavior even after we've source-mapped the stack frames.

Error
  at D:\this\is\a\commonjs\files.cjs:10:10
  at file:https:///d/this/is/a/esm/file.mjs:10:10

I want to be sure that, after we apply sourcemaps to a stack trace, the CJS files still use native paths, since that's the way vanilla node behaves.

[PaperStrike]

@cspotcode Thank you, I hadn't noticed the stack traces of different types of files, and hadn't considered the impact of the PRs on them.

So just now I wrote a test case locally, and luckily found that neither #1770 nor #1771 affects the mapped stack traces. They all start with:

Error: check the output traces
    at createTraces (D:\PR\ts-node-bug-repro\commonjs.cts:2:15)
    at file:https:///D:/PR/ts-node-bug-repro/esm.mts:3:1

Not posting code for this simple test so as not to waste everyone's time.
Let's check for other possible side effects another day.

[cspotcode]

Thanks, I will also check our existing tests. They may already be checking for this in some way, and I see both of your pull requests have passing tests on CI.

[cspotcode]

I attempted the same test you did #1769 (comment)
and got the same result: it seems that paths/URLs in stack traces are correct with #1771.

Since URLs in sourcemaps seems more correct/more sane/more robust than using basename, and since it doesn't seem to be breaking anything, I'm going to merge that one.

[PaperStrike]

@cspotcode Before we can test the impacts, could the file url/base name solution be made into an option? This problem is critical and easily reproducible when we use c8 with --exclude-after-remap on Windows, where c8 excludes every file that was processed by ts-node and shows empty results, which is caused by that the mapped paths do not matches the including paths (the case of the disk letter is different). A new option wouldn't be breaking.