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

TS transforms not working when source file path contains some chars #56296

Closed
aduh95 opened this issue Dec 17, 2024 · 4 comments
Closed

TS transforms not working when source file path contains some chars #56296

aduh95 opened this issue Dec 17, 2024 · 4 comments
Labels
source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.

Comments

@aduh95
Copy link
Contributor

aduh95 commented Dec 17, 2024

Version

main

Platform

N/A

Subsystem

No response

What steps will reproduce the bug?

$ cat test/fixtures/test-runner/matching-patterns/typescript-test.cts                                            
const test = require('node:test');

// 'as string' ensures that type stripping actually occurs
test('this should pass' as string);
$ cp test/fixtures/test-runner/matching-patterns/typescript-test.cts dir%20with\ \$unusual\"chars\?\'åß∂ƒ©∆¬…\`.cts
$ ./node --experimental-transform-types dir%20with\ \$unusual\"chars\?\'åß∂ƒ©∆¬…\`.cts
node:internal/errors:540
      throw error;
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "payload" argument must be of type object. Received null
    at cloneSourceMapV3 (node:internal/source_map/source_map:365:3)
    at new SourceMap (node:internal/source_map/source_map:145:21)
    at findSourceMap (node:internal/source_map/source_map_cache:348:17)
    at lazyFindSourceMap (node:internal/test_runner/test:98:10)
    at new Test (node:internal/test_runner/test:556:21)
    at createTestTree (node:internal/test_runner/harness:82:16)
    at lazyBootstrapRoot (node:internal/test_runner/harness:264:5)
    at run (node:internal/test_runner/harness:302:61)
    at test (node:internal/test_runner/harness:317:12)
    at Object.<anonymous> (…/dir%20with $unusual"chars?'åß∂ƒ©∆¬…`.cts:3:1) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v24.0.0-pre

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

Required condition: source file path contains some special chars (not sure which)

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

It should not throw

What do you see instead?

TypeError [ERR_INVALID_ARG_TYPE]: The "payload" argument must be of type object. Received null
    at cloneSourceMapV3 (node:internal/source_map/source_map:365:3)
    at new SourceMap (node:internal/source_map/source_map:145:21)
    at findSourceMap (node:internal/source_map/source_map_cache:348:17)
    at lazyFindSourceMap (node:internal/test_runner/test:98:10)
    at new Test (node:internal/test_runner/test:556:21)
    at createTestTree (node:internal/test_runner/harness:82:16)
    at lazyBootstrapRoot (node:internal/test_runner/harness:264:5)
    at run (node:internal/test_runner/harness:302:61)
    at test (node:internal/test_runner/harness:317:12)
    at Object.<anonymous> (…/dir%20with $unusual"chars?'åß∂ƒ©∆¬…`.cts:3:1) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Additional information

No response

@marco-ippolito
Copy link
Member

Cc @legendecas

@legendecas legendecas added the source maps Issues and PRs related to source map support. label Dec 17, 2024
@legendecas
Copy link
Member

FWIW, this is not related to this forged path. This is due to the source map generated with --experimental-tranform-types is invalid, like //# sourceMappingURL=data:application/json;base64,-1. Anyway, this should not crash the test runner.

#56299 should fix this.

@legendecas legendecas added the test_runner Issues and PRs related to the test runner subsystem. label Dec 18, 2024
@aduh95
Copy link
Contributor Author

aduh95 commented Dec 18, 2024

I've tried pulling #56299 into #48409, there's still an issue with e.g. test/fixtures/typescript/ts/test-get-callsite.ts, where the ability to generate a correct source map is dependent on the presence of some chars in the path.

$ ./node --experimental-transform-types test/fixtures/typescript/ts/test-get-callsite.ts
getCallSite:  [Object: null prototype] {
  functionName: '',
  scriptName: 'file://…/test/fixtures/typescript/ts/test-get-callsite.ts',
  lineNumber: 8,
  column: 18
}
(node:32692) ExperimentalWarning: Type Stripping is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
$ cp test/fixtures/typescript/ts/test-get-callsite.ts dir%20with\ \$unusual\"chars\?\'åß∂ƒ©∆¬…\`.cts
$ ./node --experimental-transform-types dir%20with\ \$unusual\"chars\?\'åß∂ƒ©∆¬…\`.cts  
getCallSite:  [Object: null prototype] {
  functionName: '',
  scriptName: '…/dir%20with $unusual"chars?\'åß∂ƒ©∆¬…`.cts',
  lineNumber: 2,
  column: 18
}
(node:32814) ExperimentalWarning: Type Stripping is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

Notice the reported lineNumber is not the same, indicating I think the source map is not being generated correctly.

@legendecas
Copy link
Member

This sounds like a separate issue from the original crash issue. Submitted #56315 to fix the generation of the invalid source map url.

aduh95 pushed a commit to legendecas/node that referenced this issue Dec 20, 2024
When the source map data are invalid json strings, skip construct
SourceMap on it. Additionally, suppress exceptions on source map lookups
and fix test runners crash on invalid source maps.

PR-URL: nodejs#56299
Refs: nodejs#56296
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
aduh95 pushed a commit to legendecas/node that referenced this issue Dec 20, 2024
Only invalidates source map lookup cache when a new source map is found.
This improves when user codes interleave with builtin functions, like
`array.map`.

PR-URL: nodejs#56299
Refs: nodejs#56296
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
aduh95 pushed a commit to legendecas/node that referenced this issue Dec 20, 2024
When the source map data are invalid json strings, skip construct
`SourceMap` on it. Additionally, suppress exceptions on source map
lookups and fix test runners crash on invalid source maps.

PR-URL: nodejs#56299
Refs: nodejs#56296
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
aduh95 pushed a commit to legendecas/node that referenced this issue Dec 20, 2024
Only invalidates source map lookup cache when a new source map is found.
This improves when user codes interleave with builtin functions, like
`array.map`.

PR-URL: nodejs#56299
Refs: nodejs#56296
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
nodejs-github-bot pushed a commit that referenced this issue Dec 21, 2024
`btoa` only supports latin-1 charset and produces invalid source
mapping urls.

PR-URL: #56315
Refs: #56296
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants