GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — errors: align source-map stacks with spec by bcoe · Pull Request #37252 · nodejs/node

Conversation

@bcoe

@bcoe bcoe commented Feb 6, 2021

Copy link
Copy Markdown
Contributor

Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks


After discussion with @ljharb, who's working on the Error Stacks TC39 spec, we decided that it would be better to modify the behavior of --enable-source-maps, so that the user gets just the original call site, rather than both the original and transpiled call site.

The reasoning being, this gives us much of the value of the --enable-source-maps feature, without deviating from stack traces on other platforms.

This is a step towards removing the experimental flag from --enable-source-maps.

CC: @nodejs/tooling

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr, bricss, and 7rulnik reacted with thumbs up emoji
All reactions
  • 👍 3 reactions
bcoe
errors: align source-map stacks with spec
25fc170
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks
@bcoe
bcoe requested review from aduh95, devsnek and joyeecheung February 6, 2021 19:16
ljharb
ljharb reviewed Feb 6, 2021
View reviewed changes

@ljharb ljharb left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM, thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Trott
Trott approved these changes Feb 6, 2021
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Feb 6, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35986/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iansu
iansu approved these changes Feb 7, 2021
View reviewed changes
devsnek
devsnek approved these changes Feb 7, 2021
View reviewed changes
himself65
himself65 approved these changes Feb 7, 2021
View reviewed changes
benjamingr
benjamingr approved these changes Feb 7, 2021
View reviewed changes
joyeecheung
joyeecheung approved these changes Feb 8, 2021
View reviewed changes
bcoe pushed a commit that referenced this pull request Feb 12, 2021
bcoe
errors: align source-map stacks with spec
88d9268
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@bcoe

bcoe commented Feb 12, 2021

Copy link
Copy Markdown
Contributor Author

Landed in 88d9268

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bcoe bcoe closed this Feb 12, 2021
@bcoe bcoe mentioned this pull request Feb 13, 2021
errors: remove experimental from --enable-source-maps #37362
Closed
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
bcoe @danielleadams
errors: align source-map stacks with spec
15804e0
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
This was referenced Feb 16, 2021
v15.9.0 proposal #37405
Closed
v15.9.0 proposal #37406
Merged
This was referenced Mar 10, 2021
[v14.x backport] errors: print original exception context #37700
Closed
[v14.x backport] errors: do not call resolve on URLs with schemes #37717
Closed
targos pushed a commit that referenced this pull request May 27, 2021
bcoe @targos
errors: align source-map stacks with spec
de1de70
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request May 30, 2021
bcoe @targos
errors: align source-map stacks with spec
dd28e81
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Jun 5, 2021
bcoe @targos
errors: align source-map stacks with spec
b379933
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos added a commit that referenced this pull request Jun 6, 2021
@targos
2021-06-15, Version 14.17.1 'Fermium' (LTS)
a023ec1
Notable changes:

deps:
  * update ICU to 69.1 (Michaël Zasso) #38178
src:
  * align source-map stacks with spec (Benjamin Coe) #37252

PR-URL: #38948
@targos targos mentioned this pull request Jun 6, 2021
v14.17.1 release proposal #38948
Merged
targos pushed a commit that referenced this pull request Jun 11, 2021
bcoe @targos
errors: align source-map stacks with spec
9417fd0
Reformat stack traces when --enable-source-maps flag is set to format
more likely to fit https://github.com/tc39/proposal-error-stacks

PR-URL: #37252
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ian Sutherland <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
targos added a commit that referenced this pull request Jun 11, 2021
@targos
2021-06-15, Version 14.17.1 'Fermium' (LTS)
431d90c
Notable changes:

deps:
  * update ICU to 69.1 (Michaël Zasso) #38178
src:
  * align source-map stacks with spec (Benjamin Coe) #37252

PR-URL: #38948
targos added a commit that referenced this pull request Jun 14, 2021
@targos
2021-06-15, Version 14.17.1 'Fermium' (LTS)
6c77fd3
Notable changes:

deps:
  * update ICU to 69.1 (Michaël Zasso) #38178
src:
  * align source-map stacks with spec (Benjamin Coe) #37252

PR-URL: #38948
targos added a commit that referenced this pull request Jun 15, 2021
@targos
2021-06-15, Version 14.17.1 'Fermium' (LTS)
c0becbc
Notable changes:

deps:
  * update ICU to 69.1 (Michaël Zasso) #38178
src:
  * align source-map stacks with spec (Benjamin Coe) #37252

PR-URL: #38948
@victorandree victorandree mentioned this pull request May 23, 2022
--enable-source-maps adds error context before stack string #43186
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments
@Trott Trott Trott approved these changes
@benjamingr benjamingr benjamingr approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@himself65 himself65 himself65 approved these changes
@aduh95 aduh95 Awaiting requested review from aduh95
+2 more reviewers
@iansu iansu iansu approved these changes
@devsnek devsnek devsnek approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@bcoe @nodejs-github-bot @ljharb @iansu @Trott @benjamingr @joyeecheung @devsnek @himself65
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.