FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: source maps filter null prefix by fabiancook · Pull Request #42522 · nodejs/node · GitHub

/ node Public

lib: source maps filter null prefix - #42522

Merged
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
fabiancook:fix/issue-42417
Apr 3, 2022
Merged

lib: source maps filter null prefix#42522
nodejs-github-bot merged 1 commit into
nodejs:masterfrom
fabiancook:fix/issue-42417

Conversation

Copy link
Copy Markdown
Contributor

Fixes: #42417

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 30, 2022
fabiancook added a commit to virtualstate/focus that referenced this pull request Mar 30, 2022

benjamingr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good, can you please add a test?

Copy link
Copy Markdown
Contributor Author

Added tests.

Before

AssertionError [ERR_ASSERTION]: The input did not match the regular expression /at Throw \([^)]+throw-async\.ts:4:9\)/. Input:

'/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:4\n' +
  '  throw new Error(message)\n' +
  '        ^\n' +
  '\n' +
  'Error: Message 0.3702386924909997\n' +
  '    at null.Throw (/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:4:9)\n' +
  '    at null.<anonymous> (/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:7:7)\n' +
  '    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)\n' +
  '    at async Promise.all (index 0)\n' +
  '    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)\n' +
  '    at async loadESM (node:internal/process/esm_loader:85:5)\n' +
  '    at async handleMainPromise (node:internal/modules/run_main:61:12)\n' +
  '\n' +
  'Node.js v18.0.0-pre\n'

    at Object.<anonymous> (/Users/fabiancook/src/virtualstate/node/test/parallel/test-source-map-enable.js:359:10)
    at Module._compile (node:internal/modules/cjs/loader:1106:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1160:10)
    at Module.load (node:internal/modules/cjs/loader:982:32)
    at Function.Module._load (node:internal/modules/cjs/loader:829:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: '/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:4\n' +
    '  throw new Error(message)\n' +
    '        ^\n' +
    '\n' +
    'Error: Message 0.3702386924909997\n' +
    '    at null.Throw (/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:4:9)\n' +
    '    at null.<anonymous> (/Users/fabiancook/src/virtualstate/node/test/fixtures/source-map/throw-async.ts:7:7)\n' +
    '    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)\n' +
    '    at async Promise.all (index 0)\n' +
    '    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)\n' +
    '    at async loadESM (node:internal/process/esm_loader:85:5)\n' +
    '    at async handleMainPromise (node:internal/modules/run_main:61:12)\n' +
    '\n' +
    'Node.js v18.0.0-pre\n',
  expected: /at Throw \([^)]+throw-async\.ts:4:9\)/,
  operator: 'match'
}

After

 ✗ tools/test.py test/parallel/test-source-map-enable.js
[00:01|% 100|+   1|-   0]: Done

Copy link
Copy Markdown
Contributor Author

I see a test failed, is there any way we could... re run it?

=== release test-vm-timeout-escape-promise-module ===
Path: parallel/test-vm-timeout-escape-promise-module
Error: --- stderr ---
(node:122116) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)


#
# Fatal error in , line 0
# Check failed: module->status() == kErrored.
#
#
#
#FailureMessage Object: 0x7ffec359bf00
 1: 0xbb6664 node::DumpBacktrace(_IO_FILE*) [out/Release/node]
 2: 0xf16753  [out/Release/node]
 3: 0x3f14201 V8_Fatal(char const*, ...) [out/Release/node]
 4: 0x205d43e v8::internal::SourceTextModule::Evaluate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>) [out/Release/node]
 5: 0x1f99860 v8::internal::Module::Evaluate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Module>) [out/Release/node]
 6: 0x1326c83 v8::Module::Evaluate(v8::Local<v8::Context>) [out/Release/node]
 7: 0xcb7b05 node::loader::ModuleWrap::Evaluate(v8::FunctionCallbackInfo<v8::Value> const&) [out/Release/node]
 8: 0x1466372 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [out/Release/node]
 9: 0x146518d  [out/Release/node]
10: 0x1462fd0 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [out/Release/node]
11: 0x2ee7df9  [out/Release/node]
Command: out/Release/node --experimental-vm-modules /home/runner/work/node/node/test/parallel/test-vm-timeout-escape-promise-module.js
--- CRASHED (Signal: 5) ---

===
=== 1 tests failed
=== 1 tests CRASHED
===
make[1]: *** [Makefile:528: test-ci] Error 1
make: *** [Makefile:557: run-ci] Error 2
Error: Process completed with exit code 2.

Copy link
Copy Markdown
Member

I see a test failed, is there any way we could... re run it?

That looks flaky, I'll re-run and trigger a full CI

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 31, 2022
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 31, 2022

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor Author

The same tests are failing

=== release test-vm-timeout-escape-promise-module ===
Path: parallel/test-vm-timeout-escape-promise-module
Error: --- stderr ---
(node:121410) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)


#
# Fatal error in , line 0
# Check failed: module->status() == kErrored.
#
#
#
#FailureMessage Object: 0x7fff5fc799a0
 1: 0xbb6664 node::DumpBacktrace(_IO_FILE*) [out/Release/node]
 2: 0xf16753  [out/Release/node]
 3: 0x3f14201 V8_Fatal(char const*, ...) [out/Release/node]
 4: 0x205d43e v8::internal::SourceTextModule::Evaluate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::SourceTextModule>) [out/Release/node]
 5: 0x1f99860 v8::internal::Module::Evaluate(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Module>) [out/Release/node]
 6: 0x1326c83 v8::Module::Evaluate(v8::Local<v8::Context>) [out/Release/node]
 7: 0xcb7b05 node::loader::ModuleWrap::Evaluate(v8::FunctionCallbackInfo<v8::Value> const&) [out/Release/node]
 8: 0x1466372 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [out/Release/node]
 9: 0x146518d  [out/Release/node]
10: 0x1462fd0 v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [out/Release/node]
11: 0x2ee7df9  [out/Release/node]
Command: out/Release/node --experimental-vm-modules /home/runner/work/node/node/test/parallel/test-vm-timeout-escape-promise-module.js
--- CRASHED (Signal: 5) ---

===
=== 1 tests failed
=== 1 tests CRASHED
===
make[1]: *** [Makefile:528: test-ci] Error 1
make: *** [Makefile:557: run-ci] Error 2
Error: Process completed with exit code 2.

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Contributor Author

I can see the tests say passed! Hopefully it gets marked as passed too

Copy link
Copy Markdown
Member

Hopefully it gets marked as passed too

It did, sorry for the CI experience Node.js has a lot of tests and some of them break in some environments in hard-to-debug cases.

Copy link
Copy Markdown
Member

@bcoe any chance this could get a review from you too?

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2022
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2022
nodejs-github-bot merged commit 154fa41 into nodejs:master Apr 3, 2022

Copy link
Copy Markdown
Collaborator

Landed in 154fa41

Copy link
Copy Markdown
Member

Congrats on your first commit in core @fabiancook and thanks for the fix 🙏 🎉


// To recreate:
//
// npx tsc --module esnext --target es2017 --outDir test/fixtures/source-map --sourceMap test/fixtures/source-map/throw-async.ts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If the target is es2018 or higher tsc will not down-level transpile async functions - so that'd how you'd test the thing

juanarbol pushed a commit to juanarbol/node that referenced this pull request Apr 5, 2022
Fixes: nodejs#42417

PR-URL: nodejs#42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Apr 5, 2022
juanarbol pushed a commit that referenced this pull request Apr 6, 2022
Fixes: #42417

PR-URL: #42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
Fixes: nodejs#42417

PR-URL: nodejs#42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this pull request May 31, 2022
Fixes: #42417

PR-URL: #42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
Fixes: #42417

PR-URL: #42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 11, 2022
Fixes: #42417

PR-URL: #42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
Fixes: #42417

PR-URL: #42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Fixes: nodejs/node#42417

PR-URL: nodejs/node#42522
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error stacks in log includes null.thing instead of just thing

4 participants


Back | FazBrowse Home | New Git URL