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.

Sign up for GitHub

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

Jump to bottom

Ensure dev server side errors are correct #28520

Merged
merged 8 commits into from
Aug 27, 2021
Merged

Ensure dev server side errors are correct #28520

merged 8 commits into from
Aug 27, 2021

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Aug 26, 2021 •
edited
Loading

This updates server-side errors shown in the terminal to resolve the stack trace and show accurate line/columns and related source code similar to how we do in the dev overlay. This also updates to trigger the dev overlay when an error occurred in an API route and dismisses the overlay after the error as been fixed.

Examples:

before: `getServerSideProps` error Screen Shot 2021-08-25 at 23 16 40
after: `getServerSideProps` error Screen Shot 2021-08-25 at 21 49 35
before: API route error Screen Shot 2021-08-25 at 23 16 01
after: API route error Screen Shot 2021-08-25 at 21 47 01 Screen Shot 2021-08-25 at 20 46 58
before: uncaught rejection/exception Screen Shot 2021-08-26 at 09 27 58
after: uncaught rejection/exception Screen Shot 2021-08-26 at 09 27 22

Fixes: #26711
Fixes: #21904

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Aug 26, 2021
@ijjk

This comment has been minimized.

Sign in to view
@ijjk

This comment has been minimized.

Sign in to view
@ijjk ijjk marked this pull request as ready for review August 26, 2021 05:29
sokra
sokra requested changes Aug 26, 2021
View reviewed changes
packages/next/server/dev/next-dev-server.ts Outdated
let usedOriginalStack = false
try {
const frames = parseStack(err.stack)
const frame = frames[0]
Copy link
Member

Choose a reason for hiding this comment

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

I guess we should remap all frames

Copy link
Member Author

Choose a reason for hiding this comment

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

Is showing the other frames helpful usually? We collapse all but the first in the dev overlay

sokra
sokra requested changes Aug 26, 2021
View reviewed changes
Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

catching rejected Promises might also be a good idea

@ijjk

This comment has been minimized.

Sign in to view
@ijjk
Copy link
Member Author

ijjk commented Aug 26, 2021

Added catching of unhandled rejections/exceptions as well, good idea!

@ijjk ijjk requested a review from sokra August 26, 2021 14:51
@ijjk

This comment has been minimized.

Sign in to view
@ijjk
Merge remote-tracking branch 'upstream/canary' into fix/dev-server-si…
3e6d10e 
…de-errors

# Conflicts:
#	packages/next/server/render.tsx
@ijjk

This comment has been minimized.

Sign in to view
@ijjk
Copy link
Member Author

ijjk commented Aug 26, 2021

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
buildDuration 15.8s 16s ⚠️ +213ms
buildDurationCached 3.8s 3.7s -153ms
nodeModulesSize 61.7 MB 61.7 MB ⚠️ +11.8 kB
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
/ failed reqs 0 0 ✓
/ total time (seconds) 3.016 2.907 -0.11
/ avg req/sec 828.82 859.96 +31.14
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.924 1.856 -0.07
/error-in-render avg req/sec 1299.47 1346.62 +47.15
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
745.HASH.js gzip 179 B 179 B ✓
framework-HASH.js gzip 42.2 kB 42.2 kB ✓
main-HASH.js gzip 23.3 kB 23.3 kB ✓
webpack-HASH.js gzip 1.45 kB 1.45 kB ✓
Overall change 67.2 kB 67.2 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
polyfills-a4..dd70.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
_app-HASH.js gzip 979 B 979 B ✓
_error-HASH.js gzip 194 B 194 B ✓
amp-HASH.js gzip 312 B 312 B ✓
css-HASH.js gzip 329 B 329 B ✓
dynamic-HASH.js gzip 2.67 kB 2.67 kB ✓
head-HASH.js gzip 351 B 351 B ✓
hooks-HASH.js gzip 918 B 918 B ✓
image-HASH.js gzip 4.14 kB 4.14 kB ✓
index-HASH.js gzip 261 B 261 B ✓
link-HASH.js gzip 1.66 kB 1.66 kB ✓
routerDirect..HASH.js gzip 318 B 318 B ✓
script-HASH.js gzip 387 B 387 B ✓
withRouter-HASH.js gzip 320 B 320 B ✓
bb14e60e810b..30f.css gzip 125 B 125 B ✓
Overall change 13 kB 13 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
_buildManifest.js gzip 491 B 491 B ✓
Overall change 491 B 491 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
index.html gzip 541 B 541 B ✓
link.html gzip 552 B 552 B ✓
withRouter.html gzip 534 B 534 B ✓
Overall change 1.63 kB 1.63 kB ✓
Webpack 4 Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
buildDuration 13.7s 13.5s -154ms
buildDurationCached 5.7s 5.6s -69ms
nodeModulesSize 61.7 MB 61.7 MB ⚠️ +11.8 kB
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
/ failed reqs 0 0 ✓
/ total time (seconds) 3.016 2.988 -0.03
/ avg req/sec 829.02 836.58 +7.56
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.909 1.869 -0.04
/error-in-render avg req/sec 1309.33 1337.38 +28.05
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
16.HASH.js gzip 186 B 186 B ✓
677f882d2ed8..HASH.js gzip 14.1 kB 14.1 kB ✓
framework.HASH.js gzip 41.9 kB 41.9 kB ✓
main-HASH.js gzip 10.7 kB 10.7 kB ✓
webpack-HASH.js gzip 1.19 kB 1.19 kB ✓
Overall change 68.1 kB 68.1 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
polyfills-a4..dd70.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
_app-HASH.js gzip 964 B 964 B ✓
_error-HASH.js gzip 3.8 kB 3.8 kB ✓
amp-HASH.js gzip 552 B 552 B ✓
css-HASH.js gzip 333 B 333 B ✓
dynamic-HASH.js gzip 2.87 kB 2.87 kB ✓
head-HASH.js gzip 3.06 kB 3.06 kB ✓
hooks-HASH.js gzip 924 B 924 B ✓
index-HASH.js gzip 231 B 231 B ✓
link-HASH.js gzip 1.64 kB 1.64 kB ✓
routerDirect..HASH.js gzip 298 B 298 B ✓
script-HASH.js gzip 3.03 kB 3.03 kB ✓
withRouter-HASH.js gzip 295 B 295 B ✓
30809af5c834..565.css gzip 125 B 125 B ✓
Overall change 18.1 kB 18.1 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
_buildManifest.js gzip 500 B 500 B ✓
Overall change 500 B 500 B ✓
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/dev-server-side-errors Change
index.html gzip 584 B 584 B ✓
link.html gzip 597 B 597 B ✓
withRouter.html gzip 578 B 578 B ✓
Overall change 1.76 kB 1.76 kB ✓
Commit: 3efc5c8

@timneutkens timneutkens merged commit f1dbc92 into vercel:canary Aug 27, 2021
@timneutkens timneutkens deleted the fix/dev-server-side-errors branch August 27, 2021 12:29
@snyk-bot snyk-bot mentioned this pull request Aug 31, 2021
Open
This was referenced Sep 3, 2021
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
@snyk-bot snyk-bot mentioned this pull request Sep 19, 2021
Open
This was referenced Dec 11, 2021
Open
Open
@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@timneutkens timneutkens timneutkens approved these changes

@huozhi huozhi Awaiting requested review from huozhi

@padmaia padmaia Awaiting requested review from padmaia

@shuding shuding Awaiting requested review from shuding

@styfle styfle Awaiting requested review from styfle

@sokra sokra Awaiting requested review from sokra

Assignees
No one assigned
Labels
created-by: Next.js team PRs by the Next.js team. type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

console stack trace line numbers are incorrect or un-source-mapped Source Map Line Numbers Are Wrong
3 participants
@ijjk @sokra @timneutkens