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

feat(after): surface after errors in error overlay #73449

Draft
wants to merge 3 commits into
base: lubieowoce/after-errors
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ import { normalizeAppPath } from '../../../../shared/lib/router/utils/app-paths'
import type { SizeLimit } from '../../../../types'
import { internal_getCurrentFunctionWaitUntil } from '../../../../server/web/internal-edge-wait-until'
import type { PAGE_TYPES } from '../../../../lib/page-types'
import type { NextRequestHint } from '../../../../server/web/adapter'
import type {
HandlerExtraOpts,
NextRequestHint,
} from '../../../../server/web/adapter'
import { InvariantError } from '../../../../shared/lib/invariant-error'

export function getRender({
dev,
Expand Down Expand Up @@ -157,11 +161,31 @@ export function getRender({

return async function render(
request: NextRequestHint,
event?: NextFetchEvent
event?: NextFetchEvent,
extraOpts?: HandlerExtraOpts
) {
const extendedReq = new WebNextRequest(request)
const extendedRes = new WebNextResponse(undefined)

if (dev) {
if (extraOpts) {
const { onAfterTaskError } = extraOpts
// in practice `onAfterTaskError` should be constant, so it's fine to set it like this
// but double check that it's not changing across request to prevent future bugs
if (
server['afterTaskErrorHandler'] &&
onAfterTaskError &&
server['afterTaskErrorHandler'] !== onAfterTaskError
) {
throw new InvariantError(
'Expected `extraOpts.afterTaskErrorHandler` to not change'
)
}

server['afterTaskErrorHandler'] = onAfterTaskError
}
}

handler(extendedReq, extendedRes)
const result = await extendedRes.toResponse()
request.fetchMetrics = extendedReq.fetchMetrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import stripAnsi from 'next/dist/compiled/strip-ansi'
import formatWebpackMessages from '../internal/helpers/format-webpack-messages'
import { useRouter } from '../../navigation'
import {
ACTION_AFTER_ERROR,
ACTION_BEFORE_REFRESH,
ACTION_BUILD_ERROR,
ACTION_BUILD_OK,
Expand Down Expand Up @@ -38,10 +39,12 @@ import type { HydrationErrorState } from '../internal/helpers/hydration-error-in
import type { DebugInfo } from '../types'
import { useUntrackedPathname } from '../../navigation-untracked'
import { getReactStitchedError } from '../internal/helpers/stitched-error'
import { decorateServerError } from '../../../../shared/lib/error-source'

export interface Dispatcher {
onBuildOk(): void
onBuildError(message: string): void
onAfterError(error: Error): void
onVersionInfo(versionInfo: VersionInfo): void
onDebugInfo(debugInfo: DebugInfo): void
onBeforeRefresh(): void
Expand Down Expand Up @@ -276,7 +279,19 @@ function processMessage(
return
}

function handleErrors(errors: ReadonlyArray<unknown>) {
function handleAfterErrors(errors: ReadonlyArray<Error>) {
console.log('handleAfterErrors', errors)
for (const error of errors) {
dispatcher.onAfterError(error)
}

// // Also log them to the console.
// for (let i = 0; i < errors.length; i++) {
// console.error(errors[i])
// }
}

function handleBuildErrors(errors: ReadonlyArray<unknown>) {
// "Massage" webpack messages.
const formatted = formatWebpackMessages({
errors: errors,
Expand Down Expand Up @@ -387,7 +402,7 @@ function processMessage(
})
)

handleErrors(errors)
handleBuildErrors(errors)
return
}

Expand Down Expand Up @@ -501,13 +516,24 @@ function processMessage(
// TODO-APP: potentially only refresh if the currently viewed page was added/removed.
return router.hmrRefresh()
}
case HMR_ACTIONS_SENT_TO_BROWSER.AFTER_ERROR: {
Copy link
Member

Choose a reason for hiding this comment

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

I was assuming we just console.error these. That way we cover Terminal, Node.js debuggers and the error overlay.

Copy link
Member Author

Choose a reason for hiding this comment

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

as discussed offline, the problem is that after errors occur after everything else, so we can't rely on e.g. react's error replaying to get them here, we need to push them (technically the client could also try to pull, maybe that's a better strategy?)

const { errorJSON, source } = obj
if (errorJSON) {
const { message, stack } = JSON.parse(errorJSON)
const error = new Error(message)
error.stack = stack
decorateServerError(error, source ?? 'server')
handleAfterErrors([error])
}
return
}
case HMR_ACTIONS_SENT_TO_BROWSER.SERVER_ERROR: {
const { errorJSON } = obj
if (errorJSON) {
const { message, stack } = JSON.parse(errorJSON)
const error = new Error(message)
error.stack = stack
handleErrors([error])
handleBuildErrors([error])
}
return
}
Expand Down Expand Up @@ -536,6 +562,13 @@ export default function HotReload({
onBuildError(message) {
dispatch({ type: ACTION_BUILD_ERROR, message })
},
onAfterError(reason) {
dispatch({
type: ACTION_AFTER_ERROR,
reason,
frames: parseStack(reason.stack!),
})
},
onBeforeRefresh() {
dispatch({ type: ACTION_BEFORE_REFRESH })
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { useState, useEffect, useMemo, useCallback } from 'react'
import {
ACTION_AFTER_ERROR,
ACTION_UNHANDLED_ERROR,
ACTION_UNHANDLED_REJECTION,
type AfterErrorAction,
type UnhandledErrorAction,
type UnhandledRejectionAction,
} from '../../shared'
Expand Down Expand Up @@ -38,7 +40,7 @@ import {

export type SupportedErrorEvent = {
id: number
event: UnhandledErrorAction | UnhandledRejectionAction
event: UnhandledErrorAction | UnhandledRejectionAction | AfterErrorAction
}
export type ErrorsProps = {
isAppDir: boolean
Expand Down Expand Up @@ -98,6 +100,7 @@ function ErrorDescription({
function getErrorSignature(ev: SupportedErrorEvent): string {
const { event } = ev
switch (event.type) {
case ACTION_AFTER_ERROR:
case ACTION_UNHANDLED_ERROR:
case ACTION_UNHANDLED_REJECTION: {
return `${event.reason.name}::${event.reason.message}::${event.reason.stack}`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
ACTION_AFTER_ERROR,
ACTION_UNHANDLED_ERROR,
ACTION_UNHANDLED_REJECTION,
} from '../../shared'
Expand All @@ -22,6 +23,7 @@ export async function getErrorByType(
): Promise<ReadyRuntimeError> {
const { id, event } = ev
switch (event.type) {
case ACTION_AFTER_ERROR:
case ACTION_UNHANDLED_ERROR:
case ACTION_UNHANDLED_REJECTION: {
const readyRuntimeError: ReadyRuntimeError = {
Expand Down
10 changes: 10 additions & 0 deletions packages/next/src/client/components/react-dev-overlay/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const ACTION_REFRESH = 'fast-refresh'
export const ACTION_VERSION_INFO = 'version-info'
export const ACTION_UNHANDLED_ERROR = 'unhandled-error'
export const ACTION_UNHANDLED_REJECTION = 'unhandled-rejection'
export const ACTION_AFTER_ERROR = 'after-error'
export const ACTION_DEBUG_INFO = 'debug-info'

interface StaticIndicatorAction {
Expand Down Expand Up @@ -66,6 +67,12 @@ export interface UnhandledRejectionAction {
frames: StackFrame[]
}

export interface AfterErrorAction {
type: typeof ACTION_AFTER_ERROR
reason: Error
frames: StackFrame[]
}

export interface DebugInfoAction {
type: typeof ACTION_DEBUG_INFO
debugInfo: any
Expand All @@ -83,6 +90,7 @@ export type BusEvent =
| FastRefreshAction
| UnhandledErrorAction
| UnhandledRejectionAction
| AfterErrorAction
| VersionInfoAction
| StaticIndicatorAction
| DebugInfoAction
Expand Down Expand Up @@ -147,8 +155,10 @@ export function useErrorOverlayReducer() {
refreshState: { type: 'idle' },
}
}
case ACTION_AFTER_ERROR:
case ACTION_UNHANDLED_ERROR:
case ACTION_UNHANDLED_REJECTION: {
console.log('useErrorOverlayReducer', action)
switch (_state.refreshState.type) {
case 'idle': {
return {
Expand Down
6 changes: 4 additions & 2 deletions packages/next/src/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ export default abstract class Server<
protected readonly enabledDirectories: NextEnabledDirectories
protected abstract getEnabledDirectories(dev: boolean): NextEnabledDirectories

protected afterTaskErrorHandler: ((error: unknown) => void) | undefined

protected readonly experimentalTestProxy?: boolean

protected abstract findPageComponents(params: {
Expand Down Expand Up @@ -2437,7 +2439,7 @@ export default abstract class Server<
postponed,
waitUntil: this.getWaitUntil(),
onClose: res.onClose.bind(res),
onAfterTaskError: undefined,
onAfterTaskError: this.afterTaskErrorHandler,
// only available in dev
setAppIsrStatus: (this as any).setAppIsrStatus,
}
Expand Down Expand Up @@ -2484,7 +2486,7 @@ export default abstract class Server<
isRevalidate: isSSG,
waitUntil: this.getWaitUntil(),
onClose: res.onClose.bind(res),
onAfterTaskError: undefined,
onAfterTaskError: this.afterTaskErrorHandler,
onInstrumentationRequestError:
this.renderOpts.onInstrumentationRequestError,
buildId: this.renderOpts.buildId,
Expand Down
9 changes: 9 additions & 0 deletions packages/next/src/server/dev/hot-reloader-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { RouteDefinition } from '../route-definitions/route-definition'
import type { Project, Update as TurbopackUpdate } from '../../build/swc/types'
import type { VersionInfo } from './parse-version-info'
import type { DebugInfo } from '../../client/components/react-dev-overlay/types'
import type { ErrorSourceType } from '../../shared/lib/error-source'

export const enum HMR_ACTIONS_SENT_TO_BROWSER {
ADDED_PAGE = 'addedPage',
Expand All @@ -22,6 +23,7 @@ export const enum HMR_ACTIONS_SENT_TO_BROWSER {
DEV_PAGES_MANIFEST_UPDATE = 'devPagesManifestUpdate',
TURBOPACK_MESSAGE = 'turbopack-message',
SERVER_ERROR = 'serverError',
AFTER_ERROR = 'afterError',
TURBOPACK_CONNECTED = 'turbopack-connected',
APP_ISR_MANIFEST = 'appIsrManifest',
}
Expand All @@ -31,6 +33,12 @@ interface ServerErrorAction {
errorJSON: string
}

export interface AfterErrorAction {
action: HMR_ACTIONS_SENT_TO_BROWSER.AFTER_ERROR
source: ErrorSourceType
errorJSON: string
}

export interface TurbopackMessageAction {
action: HMR_ACTIONS_SENT_TO_BROWSER.TURBOPACK_MESSAGE
data: TurbopackUpdate | TurbopackUpdate[]
Expand Down Expand Up @@ -130,6 +138,7 @@ export type HMR_ACTION_TYPES =
| ServerOnlyChangesAction
| DevPagesManifestUpdateAction
| ServerErrorAction
| AfterErrorAction
| AppIsrManifestAction

export type TurbopackMsgToBrowser =
Expand Down
9 changes: 9 additions & 0 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,15 @@ export default class DevServer extends Server {
this.pagesDir = pagesDir
this.appDir = appDir

if (this.nextConfig.experimental.after) {
this.afterTaskErrorHandler = (error: unknown) => {
return this.bundlerService.reportAfterTaskError(
error,
process.env.NEXT_RUNTIME === 'edge' ? 'edge-server' : 'server'
)
}
}

if (this.nextConfig.experimental.serverComponentsHmrCache) {
this.serverComponentsHmrCache = new LRUCache(
this.nextConfig.cacheMaxMemorySize,
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/server/lib/dev-bundler-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ export class DevBundlerService {
public logErrorWithOriginalStack =
this.bundler.logErrorWithOriginalStack.bind(this.bundler)

public reportAfterTaskError: typeof this.bundler.reportAfterTaskError =
async (...args) => {
return await this.bundler.reportAfterTaskError(...args)
}

public async getFallbackErrorComponents(url?: string) {
await this.bundler.hotReloader.buildFallbackError()
// Build the error page to ensure the fallback is built too.
Expand Down
29 changes: 29 additions & 0 deletions packages/next/src/server/lib/router-utils/setup-dev-bundler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ import { normalizeMetadataPageToRoute } from '../../../lib/metadata/get-metadata
import { createEnvDefinitions } from '../experimental/create-env-definitions'
import { JsConfigPathsPlugin } from '../../../build/webpack/plugins/jsconfig-paths-plugin'
import { store as consoleStore } from '../../../build/output/store'
import { type ErrorSourceType } from '../../../shared/lib/error-source'
import { stringifyError } from '../../../shared/lib/utils'
import isError from '../../../lib/is-error'

export type SetupOpts = {
renderServer: LazyRenderServerInstance
Expand Down Expand Up @@ -950,11 +953,37 @@ async function startWatcher(opts: SetupOpts) {
}
}

async function reportAfterTaskError(error: unknown, source: ErrorSourceType) {
const serializeErrorToJSON = (err: unknown) => {
if (isError(err)) {
return stringifyError(err)
}

try {
return JSON.stringify(err)
} catch (_) {
return '<unknown>'
}
}

// TODO:
// 1. what if the client hasn't come online yet? we need to detect that, buffer these, and send them later
// 2. what if multiple clients are connected? we need to somehow know which one sent the request.
// we might not even have a session id yet because that's in-memory and they may have refreshed the tab.
// maybe we need to push that id to the client in a set-cookie or something.
hotReloader.send({
action: HMR_ACTIONS_SENT_TO_BROWSER.AFTER_ERROR,
source: source,
errorJSON: serializeErrorToJSON(error),
})
}

return {
serverFields,
hotReloader,
requestHandler,
logErrorWithOriginalStack,
reportAfterTaskError,

async ensureMiddleware(requestUrl?: string) {
if (!serverFields.actualMiddlewareFile) return
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1494,6 +1494,7 @@ export default class NextNodeServer extends BaseServer<
body: getRequestMeta(params.request, 'clonableBody'),
signal: signalFromNodeResponse(params.response.originalResponse),
waitUntil: this.getWaitUntil(),
onAfterTaskError: this.afterTaskErrorHandler,
},
useCache: true,
onWarning: params.onWarning,
Expand Down Expand Up @@ -1798,6 +1799,7 @@ export default class NextNodeServer extends BaseServer<
body: getRequestMeta(params.req, 'clonableBody'),
signal: signalFromNodeResponse(params.res.originalResponse),
waitUntil: this.getWaitUntil(),
onAfterTaskError: this.afterTaskErrorHandler,
},
useCache: true,
onError: params.onError,
Expand Down
Loading
Loading