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
JSON.stringify generic errors #31866
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Can you add a test for this change?
49c82ec
to
14a8d1e
Compare
14a8d1e
to
7e958c0
Compare
@timneutkens The test said it failed on the CI but when I run it locally it passes. I've rebased to the top of canary and pushed again, but I don't see any reason the test would have failed. They all reported timeouts on the CI run:
Passes for me:
|
I see the latest merges into canary are also failing the same azure tests. |
c2d3e8e
to
d9eb3d3
Compare
d9eb3d3
to
ed022ed
Compare
@timneutkens I've rebased but I see the tests are still failing the same way as other PRs. Could you help me with the next steps for this PR? |
2d848f0
to
2ca072e
Compare
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
buildDuration | 16.9s | 16.9s | |
buildDurationCached | 3.8s | 3.7s | -111ms |
nodeModulesSize | 355 MB | 355 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.407 | 3.448 | |
/ avg req/sec | 733.85 | 725.11 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.606 | 1.579 | -0.03 |
/error-in-render avg req/sec | 1556.61 | 1583.03 | +26.42 |
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
main-HASH.js gzip | 27.1 kB | 27.1 kB | |
webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
Overall change | 70.9 kB | 70.9 kB |
Legacy Client Bundles (polyfills)
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.37 kB | 1.37 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.37 kB | 2.37 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 4.74 kB | 4.74 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.13 kB | 2.13 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.1 kB | 14.1 kB | ✓ |
Client Build Manifests
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
index.html gzip | 532 B | 532 B | ✓ |
link.html gzip | 546 B | 547 B | |
withRouter.html gzip | 527 B | 527 B | ✓ |
Overall change | 1.6 kB | 1.61 kB |
Diffs
Diff for main-HASH.js
@@ -918,7 +918,7 @@
// This catches errors like throwing in the top level of a module
initialErr = (0, _isError).default(_ctx.t1)
? _ctx.t1
- : new Error(_ctx.t1 + "");
+ : new Error(JSON.stringify(_ctx.t1));
case 33:
if (false) {
}
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
Default Build with SWC (Increase detected ⚠️ )
General Overall increase ⚠️
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
buildDuration | 18.4s | 18.6s | |
buildDurationCached | 3.8s | 3.7s | -97ms |
nodeModulesSize | 355 MB | 355 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.386 | 3.461 | |
/ avg req/sec | 738.41 | 722.33 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.528 | 1.571 | |
/error-in-render avg req/sec | 1636.21 | 1590.9 |
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.3 kB | 42.3 kB | ✓ |
main-HASH.js gzip | 27.2 kB | 27.2 kB | |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.1 kB | 71.1 kB |
Legacy Client Bundles (polyfills)
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.36 kB | 2.36 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 906 B | 906 B | ✓ |
image-HASH.js gzip | 4.76 kB | 4.76 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.19 kB | 2.19 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.1 kB | 14.1 kB | ✓ |
Client Build Manifests
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary | mozeryansky/next.js michael/error | Change | |
---|---|---|---|
index.html gzip | 532 B | 530 B | -2 B |
link.html gzip | 546 B | 544 B | -2 B |
withRouter.html gzip | 526 B | 525 B | -1 B |
Overall change | 1.6 kB | 1.6 kB | -5 B |
Diffs
Diff for main-HASH.js
@@ -918,7 +918,7 @@
// This catches errors like throwing in the top level of a module
initialErr = (0, _isError).default(_ctx.t1)
? _ctx.t1
- : new Error(_ctx.t1 + "");
+ : new Error(JSON.stringify(_ctx.t1));
case 33:
if (false) {
}
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-65c45d624fc47f18.js"
+ src="/_next/static/chunks/main-c192afff05800900.js"
defer=""
></script>
<script
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.
Hi, I opened an alternative/continuation of this PR here which adds a shared util for this and updates all cases to use this so we can change this easier. I copied over your commits so you're marked as a co-contributor on that PR. Thanks for taking a look at this!
Thanks! I had originally done a helper function in the exact same spot, but I reverted to changing each line when I was trying to figure out how the tests worked. Should I close this PR? |
I see, yeah we can close this one and continue in the other PR |
fixes #31670
Use
JSON.stringify
instead of creating a string from an unknown error usingerror + ''
.