Skip to content

fix(router-core): Prevents the app from crashing when a primitive value is thrown in beforeLoad #4724

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
49 changes: 49 additions & 0 deletions packages/react-router/tests/link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,55 @@ describe('Link', () => {
expect(onError).toHaveBeenCalledOnce()
})

test('when navigating to /posts with a beforeLoad that throws an primitive type error', async () => {
const onError = vi.fn()
const rootRoute = createRootRoute()
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
component: () => {
return (
<>
<h1>Index</h1>
<Link to="/posts">Posts</Link>
</>
)
},
})

const PostsComponent = () => {
return <h1>Posts</h1>
}

const postsRoute = createRoute({
getParentRoute: () => rootRoute,
path: 'posts',
beforeLoad: () => {
throw 'This is a string error'
},
onError,
errorComponent: () => <span>Oops! Something went wrong!</span>,
component: PostsComponent,
})

const router = createRouter({
context: { userId: 'userId' },
routeTree: rootRoute.addChildren([indexRoute, postsRoute]),
history,
})

render(<RouterProvider router={router} />)

const postsLink = await screen.findByRole('link', { name: 'Posts' })

await act(() => fireEvent.click(postsLink))

const errorText = await screen.findByText('Oops! Something went wrong!')
expect(errorText).toBeInTheDocument()

expect(onError).toHaveBeenCalledOnce()
})

test('when navigating to /posts with a beforeLoad that throws an error bubbles to the root', async () => {
const rootRoute = createRootRoute({
errorComponent: () => <span>Oops! Something went wrong!</span>,
Expand Down
4 changes: 2 additions & 2 deletions packages/router-core/src/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type {
} from './Matches'
import type { RootRouteId } from './root'
import type { ParseRoute, RouteById, RoutePaths } from './routeInfo'
import type { AnyRouter, RegisteredRouter } from './router'
import type { AnyRouter, RegisteredRouter, RouterCode } from './router'
import type { BuildLocationFn, NavigateFn } from './RouterProvider'
import type {
Assign,
Expand Down Expand Up @@ -1072,7 +1072,7 @@ export interface UpdatableRouteOptions<
SearchFilter<ResolveFullSearchSchema<TParentRoute, TSearchValidator>>
>
onCatch?: (error: Error) => void
onError?: (err: any) => void
onError?: (err: any, routerCode?: RouterCode) => void
// These functions are called as route matches are loaded, stick around and leave the active
// matches
onEnter?: (
Expand Down
33 changes: 23 additions & 10 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,8 @@ export type AnyRouterWithContext<TContext> = RouterCore<

export type AnyRouter = RouterCore<any, any, any, any, any>

export type RouterCode = 'BEFORE_LOAD' | 'PARSE_PARAMS' | 'VALIDATE_SEARCH'

export interface ViewTransitionOptions {
types:
| Array<string>
Expand Down Expand Up @@ -2106,7 +2108,11 @@ export class RouterCore<
triggerOnReady()
}

const handleRedirectAndNotFound = (match: AnyRouteMatch, err: any) => {
const handleRedirectAndNotFound = (
match: AnyRouteMatch,
err: any,
routerCode?: RouterCode,
) => {
if (isRedirect(err) || isNotFound(err)) {
if (isRedirect(err)) {
if (err.redirectHandled) {
Expand Down Expand Up @@ -2145,7 +2151,7 @@ export class RouterCore<
err = this.resolveRedirect(err)
throw err
} else if (isNotFound(err)) {
this._handleNotFound(matches, err, {
this._handleNotFound(matches, err, routerCode, {
updateMatch,
})
throw err
Expand Down Expand Up @@ -2175,7 +2181,7 @@ export class RouterCore<
const handleSerialError = (
index: number,
err: any,
routerCode: string,
routerCode: RouterCode,
) => {
const { id: matchId, routeId } = matches[index]!
const route = this.looseRoutesById[routeId]!
Expand All @@ -2187,15 +2193,22 @@ export class RouterCore<
throw err
}

err.routerCode = routerCode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The app crashes at this point when err is a primitive type.

firstBadMatchIndex = firstBadMatchIndex ?? index
handleRedirectAndNotFound(this.getMatch(matchId)!, err)
handleRedirectAndNotFound(
this.getMatch(matchId)!,
err,
routerCode,
Comment on lines +2197 to +2200
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passes the routerCode, which was previously included in err, as a separate parameter to _handleNotFound.

)

try {
route.options.onError?.(err)
route.options.onError?.(err, routerCode)
Comment on lines -2195 to +2204
Copy link
Contributor Author

@leesb971204 leesb971204 Jul 20, 2025

Choose a reason for hiding this comment

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

Applies minimal changes to preserve compatibility for users accessing err.routerCode in onError.
In fact, there are quite a few people who use it this way.

} catch (errorHandlerErr) {
err = errorHandlerErr
handleRedirectAndNotFound(this.getMatch(matchId)!, err)
handleRedirectAndNotFound(
this.getMatch(matchId)!,
err,
routerCode,
)
}

updateMatch(matchId, (prev) => {
Expand Down Expand Up @@ -3019,6 +3032,7 @@ export class RouterCore<
_handleNotFound = (
matches: Array<AnyRouteMatch>,
err: NotFoundError,
routerCode?: string,
{
updateMatch = this.updateMatch,
}: {
Expand Down Expand Up @@ -3069,10 +3083,9 @@ export class RouterCore<
error: err,
isFetching: false,
}))

if ((err as any).routerCode === 'BEFORE_LOAD' && routeCursor.parentRoute) {
if (routerCode === 'BEFORE_LOAD' && routeCursor.parentRoute) {
err.routeId = routeCursor.parentRoute.id
this._handleNotFound(matches, err, {
this._handleNotFound(matches, err, routerCode, {
Comment on lines -3075 to +3088
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passes routerCode separately alongside err.

updateMatch,
})
}
Expand Down