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

fix: handle 404 errors in HotReload client #51637

Merged
merged 9 commits into from
Jun 23, 2023
Merged
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
27 changes: 14 additions & 13 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -434,18 +434,14 @@ function Router({
return findHeadInCache(cache, tree[1])
}, [cache, tree])

const notFoundProps = { notFound, notFoundStyles, asNotFound }

const content = (
<NotFoundBoundary
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
>
<RedirectBoundary>
{head}
{cache.subTreeData}
<AppRouterAnnouncer tree={tree} />
</RedirectBoundary>
</NotFoundBoundary>
<RedirectBoundary>
{head}
{cache.subTreeData}
<AppRouterAnnouncer tree={tree} />
</RedirectBoundary>
)

return (
Expand Down Expand Up @@ -478,9 +474,14 @@ function Router({
}}
>
{HotReloader ? (
<HotReloader assetPrefix={assetPrefix}>{content}</HotReloader>
// HotReloader implements a separate NotFoundBoundary to maintain the HMR ping interval
<HotReloader assetPrefix={assetPrefix} {...notFoundProps}>
{content}
</HotReloader>
) : (
content
<NotFoundBoundary {...notFoundProps}>
{content}
</NotFoundBoundary>
)}
</LayoutRouterContext.Provider>
</AppRouterContext.Provider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import stripAnsi from 'next/dist/compiled/strip-ansi'
import formatWebpackMessages from '../../dev/error-overlay/format-webpack-messages'
import { useRouter } from '../navigation'
import {
ACTION_NOT_FOUND,
ACTION_VERSION_INFO,
errorOverlayReducer,
} from './internal/error-overlay-reducer'
Expand All @@ -34,13 +35,16 @@ import {
} from './internal/helpers/use-websocket'
import { parseComponentStack } from './internal/helpers/parse-component-stack'
import type { VersionInfo } from '../../../server/dev/parse-version-info'
import { isNotFoundError } from '../not-found'
import { NotFoundBoundary } from '../not-found-boundary'

interface Dispatcher {
onBuildOk(): void
onBuildError(message: string): void
onVersionInfo(versionInfo: VersionInfo): void
onBeforeRefresh(): void
onRefresh(): void
onNotFound(): void
}

// TODO-APP: add actual type
Expand Down Expand Up @@ -414,8 +418,22 @@ function processMessage(
if (invalid) {
// Payload can be invalid even if the page does exist.
// So, we check if it can be created.
// @ts-ignore it exists, it's just hidden
router.fastRefresh()
fetch(window.location.href, {
credentials: 'same-origin',
}).then((pageRes) => {
if (pageRes.status === 200) {
// Page exists now, reload
startTransition(() => {
// @ts-ignore it exists, it's just hidden
router.fastRefresh()
dispatcher.onRefresh()
})
} else {
// We are still on the page,
// dispatch an error so it's caught by the NotFound handler
dispatcher.onNotFound()
}
})
}
return
}
Expand All @@ -428,14 +446,21 @@ function processMessage(
export default function HotReload({
assetPrefix,
children,
notFound,
notFoundStyles,
asNotFound,
}: {
assetPrefix: string
children?: ReactNode
notFound?: React.ReactNode
notFoundStyles?: React.ReactNode
asNotFound?: boolean
}) {
const [state, dispatch] = useReducer(errorOverlayReducer, {
nextId: 1,
buildError: null,
errors: [],
notFound: false,
refreshState: { type: 'idle' },
versionInfo: { installed: '0.0.0', staleness: 'unknown' },
})
Expand All @@ -456,6 +481,9 @@ export default function HotReload({
onVersionInfo(versionInfo) {
dispatch({ type: ACTION_VERSION_INFO, versionInfo })
},
onNotFound() {
dispatch({ type: ACTION_NOT_FOUND })
},
}
}, [dispatch])

Expand All @@ -477,7 +505,9 @@ export default function HotReload({
frames: parseStack(reason.stack!),
})
}, [])
const handleOnReactError = useCallback(() => {
const handleOnReactError = useCallback((error: Error) => {
// not found errors are handled by the parent boundary, not the dev overlay
if (isNotFoundError(error)) throw error
RuntimeErrorHandler.hadRuntimeError = true
}, [])
useErrorHandler(handleOnUnhandledError, handleOnUnhandledRejection)
Expand All @@ -487,6 +517,7 @@ export default function HotReload({
const sendMessage = useSendMessage(webSocketRef)

const router = useRouter()

useEffect(() => {
const handler = (event: MessageEvent<PongEvent>) => {
try {
Expand All @@ -507,8 +538,15 @@ export default function HotReload({
}, [sendMessage, router, webSocketRef, dispatcher])

return (
<ReactDevOverlay onReactError={handleOnReactError} state={state}>
{children}
</ReactDevOverlay>
<NotFoundBoundary
key={`${state.notFound}`}
notFound={notFound}
notFoundStyles={notFoundStyles}
asNotFound={asNotFound}
>
<ReactDevOverlay onReactError={handleOnReactError} state={state}>
{children}
</ReactDevOverlay>
</NotFoundBoundary>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { parseStack } from './helpers/parseStack'
import { Base } from './styles/Base'
import { ComponentStyles } from './styles/ComponentStyles'
import { CssReset } from './styles/CssReset'
import { notFound } from '../../not-found'

interface ReactDevOverlayState {
reactError: SupportedErrorEvent | null
Expand Down Expand Up @@ -58,6 +59,10 @@ class ReactDevOverlay extends React.PureComponent<
reactError ||
rootLayoutMissingTagsError

if (state.notFound) {
notFound()
}

return (
<>
{reactError ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const ACTION_REFRESH = 'fast-refresh'
export const ACTION_UNHANDLED_ERROR = 'unhandled-error'
export const ACTION_UNHANDLED_REJECTION = 'unhandled-rejection'
export const ACTION_VERSION_INFO = 'version-info'
export const ACTION_NOT_FOUND = 'not-found'

interface BuildOkAction {
type: typeof ACTION_BUILD_OK
Expand All @@ -24,6 +25,11 @@ interface BeforeFastRefreshAction {
interface FastRefreshAction {
type: typeof ACTION_REFRESH
}

interface NotFoundAction {
type: typeof ACTION_NOT_FOUND
}

export interface UnhandledErrorAction {
type: typeof ACTION_UNHANDLED_ERROR
reason: Error
Expand Down Expand Up @@ -59,6 +65,7 @@ export interface OverlayState {
}
refreshState: FastRefreshState
versionInfo: VersionInfo
notFound: boolean
}

function pushErrorFilterDuplicates(
Expand All @@ -81,25 +88,30 @@ export const errorOverlayReducer: React.Reducer<
| BuildErrorAction
| BeforeFastRefreshAction
| FastRefreshAction
| NotFoundAction
| UnhandledErrorAction
| UnhandledRejectionAction
| VersionInfoAction
>
> = (state, action) => {
switch (action.type) {
case ACTION_BUILD_OK: {
return { ...state, buildError: null }
return { ...state, buildError: null, notFound: false }
}
case ACTION_BUILD_ERROR: {
return { ...state, buildError: action.message }
}
case ACTION_BEFORE_REFRESH: {
return { ...state, refreshState: { type: 'pending', errors: [] } }
}
case ACTION_NOT_FOUND: {
return { ...state, notFound: true }
}
case ACTION_REFRESH: {
return {
...state,
buildError: null,
notFound: false,
errors:
// Errors can come in during updates. In this case, UNHANDLED_ERROR
// and UNHANDLED_REJECTION events might be dispatched between the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export function useWebsocketPing(

useEffect(() => {
// Taken from on-demand-entries-client.js
// TODO-APP: check 404 case
const interval = setInterval(() => {
sendMessage(
JSON.stringify({
Expand Down
8 changes: 7 additions & 1 deletion test/e2e/app-dir/not-found/app/not-found.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
export default function Page() {
return <h1>This Is The Not Found Page</h1>
return (
<>
<h1>This Is The Not Found Page</h1>

<div id="timestamp">{Date.now()}</div>
</>
)
}
29 changes: 29 additions & 0 deletions test/e2e/app-dir/not-found/not-found.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { check } from 'next-test-utils'

createNextDescribe(
'app dir - not-found',
Expand All @@ -18,6 +19,34 @@ createNextDescribe(
expect(html).toContain("I'm still a valid page")
})

if (isNextDev) {
it('should not reload the page', async () => {
ztanner marked this conversation as resolved.
Show resolved Hide resolved
const browser = await next.browser('/random-content')
const timestamp = await browser.elementByCss('#timestamp').text()

await new Promise((resolve) => {
setTimeout(resolve, 3000)
})

await check(async () => {
const newTimestamp = await browser.elementByCss('#timestamp').text()
return newTimestamp !== timestamp ? 'failure' : 'success'
}, 'success')
})

it('should render the 404 page when the file is removed, and restore the page when re-added', async () => {
const browser = await next.browser('/')
await check(() => browser.elementByCss('h1').text(), 'My page')
await next.renameFile('./app/page.js', './app/foo.js')
await check(
() => browser.elementByCss('h1').text(),
'This Is The Not Found Page'
)
await next.renameFile('./app/foo.js', './app/page.js')
await check(() => browser.elementByCss('h1').text(), 'My page')
})
}

if (!isNextDev) {
it('should create the 404 mapping and copy the file to pages', async () => {
const html = await next.readFile('.next/server/pages/404.html')
Expand Down