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

Use consistent error formatting in terminal #71909

Merged

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Oct 26, 2024

We stop treating certain errors special. Instead, each error will be treated as if it were logged via console. This way we can just use console.error(error) (instead of bundler.logErrorWithOriginalStack) everywhere and get a consistent stack format. This will also allow us the get sourcemapped stacks in next build and even next start if desired.

Only reason to use bundler.logErrorWithOriginalStack is to get a prefixed log with a colored "x". We might get rid of bundler.logErrorWithOriginalStack in favor of instrumenting.

Copy link
Member Author

eps1lon commented Oct 26, 2024

@ijjk
Copy link
Member

ijjk commented Oct 26, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Oct 26, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
buildDuration 20.8s 17.7s N/A
buildDurationCached 17s 14.7s N/A
nodeModulesSize 409 MB 409 MB N/A
nextStartRea..uration (ms) 471ms 474ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
1187-HASH.js gzip 49.2 kB 49.2 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.3 kB 5.3 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 232 B 235 B N/A
main-HASH.js gzip 33.7 kB 33.7 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 513 B 511 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB N/A
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.44 kB 4.43 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.34 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.75 kB 1.75 kB
Client Build Manifests
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
_buildManifest.js gzip 746 B 747 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
index.html gzip 523 B 524 B N/A
link.html gzip 537 B 537 B
withRouter.html gzip 519 B 521 B N/A
Overall change 537 B 537 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 200 kB 200 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
middleware-b..fest.js gzip 665 B 664 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31.1 kB 31.1 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
797-experime...dev.js gzip 322 B 322 B
797.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 321 kB 321 kB
app-page-exp..prod.js gzip 126 kB 126 kB
app-page-tur..prod.js gzip 139 kB 139 kB
app-page-tur..prod.js gzip 134 kB 134 kB
app-page.run...dev.js gzip 311 kB 311 kB
app-page.run..prod.js gzip 122 kB 122 kB
app-route-ex...dev.js gzip 36.8 kB 36.8 kB
app-route-ex..prod.js gzip 25 kB 25 kB
app-route-tu..prod.js gzip 25 kB 25 kB
app-route-tu..prod.js gzip 24.8 kB 24.8 kB
app-route.ru...dev.js gzip 38.5 kB 38.5 kB
app-route.ru..prod.js gzip 24.8 kB 24.8 kB
pages-api-tu..prod.js gzip 9.56 kB 9.56 kB
pages-api.ru...dev.js gzip 11.4 kB 11.4 kB
pages-api.ru..prod.js gzip 9.56 kB 9.56 kB
pages-turbo...prod.js gzip 21.3 kB 21.3 kB
pages.runtim...dev.js gzip 27 kB 27 kB
pages.runtim..prod.js gzip 21.3 kB 21.3 kB
server.runti..prod.js gzip 916 kB 916 kB N/A
Overall change 1.43 MB 1.43 MB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/10-26-use_consistent_error_formatting_in_terminal Change
0.pack gzip 2.04 MB 2.04 MB ⚠️ +2.08 kB
index.pack gzip 145 kB 145 kB N/A
Overall change 2.04 MB 2.04 MB ⚠️ +2.08 kB
Diff details
Diff for main-HASH.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: cac668a

@eps1lon eps1lon force-pushed the sebbie/10-26-respect_sourcemap_s_ignore_list_when_printing_errors_in_the_terminal branch from d1f7530 to a469f58 Compare October 27, 2024 15:25
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from 84374b1 to 5446d4c Compare October 27, 2024 15:25
@eps1lon eps1lon force-pushed the sebbie/10-26-respect_sourcemap_s_ignore_list_when_printing_errors_in_the_terminal branch from a469f58 to 7fcb95c Compare October 27, 2024 18:46
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from 5446d4c to 88aee34 Compare October 27, 2024 18:46
@eps1lon eps1lon force-pushed the sebbie/10-26-respect_sourcemap_s_ignore_list_when_printing_errors_in_the_terminal branch from 7fcb95c to 10283fa Compare October 27, 2024 19:24
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch 2 times, most recently from 07c176a to 2dddf8c Compare October 27, 2024 21:30
@ijjk ijjk added the tests label Oct 27, 2024
@eps1lon eps1lon changed the base branch from sebbie/10-26-respect_sourcemap_s_ignore_list_when_printing_errors_in_the_terminal to sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors October 27, 2024 21:30
@eps1lon eps1lon force-pushed the sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors branch 2 times, most recently from c7865b0 to 4a62733 Compare October 27, 2024 22:26
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from 2dddf8c to b8d9d50 Compare October 27, 2024 22:26
@eps1lon eps1lon force-pushed the sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors branch from 4a62733 to 4af2e97 Compare October 28, 2024 09:24
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from b8d9d50 to 556771b Compare October 28, 2024 09:25
@eps1lon eps1lon force-pushed the sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors branch from 4af2e97 to 933d739 Compare October 28, 2024 12:30
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from 556771b to a73d9f1 Compare October 28, 2024 12:30
@eps1lon eps1lon force-pushed the sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors branch from 4c97f7f to 9ccdf4b Compare October 28, 2024 12:40
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from a73d9f1 to 9fb236a Compare October 28, 2024 12:41
@eps1lon eps1lon force-pushed the sebbie/10-27-clickable_stack_trace_links_in_logged_terminal_errors branch from 9ccdf4b to e9e1eff Compare October 28, 2024 23:57
@eps1lon eps1lon changed the base branch from canary to sebbie/12-02-ensure_issue_overlay_sourcemaps_externals_in_turbopack December 3, 2024 18:09
@eps1lon eps1lon mentioned this pull request Dec 3, 2024
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch 2 times, most recently from bf7a8e7 to d816edf Compare December 3, 2024 23:57
@@ -947,127 +929,24 @@ async function startWatcher(opts: SetupOpts) {
return { finished: false }
}

async function logErrorWithOriginalStack(
function logErrorWithOriginalStack(
Copy link
Member Author

Choose a reason for hiding this comment

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

At this point it's really just a prefixed log. The only meaningful logic is silencing of TurbopackInternalError but that's a debateable strategy.

Might get rid of the prefixing or replace it with console instrumentation instead.

Comment on lines +119 to +120
// TODO(veil): Use codeframe froma stackframe that's not ignore-listed
expect(stripAnsi(output)).not.toInclude(errorHighlight)
Copy link
Member Author

Choose a reason for hiding this comment

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

Revisit once bundler sourcemap fallback is implemented for webpack

@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from d816edf to c41e998 Compare December 4, 2024 00:12
@eps1lon eps1lon changed the base branch from sebbie/12-02-ensure_issue_overlay_sourcemaps_externals_in_turbopack to graphite-base/71909 December 4, 2024 09:11
@eps1lon eps1lon force-pushed the graphite-base/71909 branch from 8eafe6e to 81f6cb4 Compare December 4, 2024 09:11
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from c41e998 to 68e43f4 Compare December 4, 2024 09:11
@eps1lon eps1lon changed the base branch from graphite-base/71909 to canary December 4, 2024 09:12
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch 3 times, most recently from 3f7f03c to 93c99c4 Compare December 4, 2024 15:45
@eps1lon eps1lon marked this pull request as ready for review December 4, 2024 16:40
@eps1lon eps1lon requested review from unstubbable and huozhi December 4, 2024 16:42
'\n ⚠ middleware.js (3:22) @ __TURBOPACK__default__export__' +
'\n ⨯ middleware.js (4:9) @ eval'
// TODO(veil): Should be sourcemapped
'\n at __TURBOPACK__default__export__ (.next/'
Copy link
Member

Choose a reason for hiding this comment

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

I would love to have these turbopack special identifiers demangled before printing. There is a method to demangle them in a string we need to call

Suggested change
'\n at __TURBOPACK__default__export__ (.next/'
'\n at {default export} (.next/'

Copy link
Member Author

Choose a reason for hiding this comment

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

My current thinking is that these should just be sourcemapped to default. That's what you'd get in the original stack trace.

But we already special case the WEBPACK__default__export so I probably just go with that short term.

Copy link
Member

Choose a reason for hiding this comment

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

packages/next/src/shared/lib/magic-identifier.ts decodeMagicIdentifier

@eps1lon eps1lon enabled auto-merge (squash) December 6, 2024 11:04
@eps1lon eps1lon disabled auto-merge December 6, 2024 11:04
A case where we encounter this is not known.
Only saw this while stepping through Node.js internals.
@eps1lon eps1lon force-pushed the sebbie/10-26-use_consistent_error_formatting_in_terminal branch from 93c99c4 to cac668a Compare December 6, 2024 11:20
@eps1lon eps1lon merged commit 98de5db into canary Dec 6, 2024
107 of 112 checks passed
@eps1lon eps1lon deleted the sebbie/10-26-use_consistent_error_formatting_in_terminal branch December 6, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants