Skip to content

fix: prevent crash in buildLocation when params stringification fails (#6310) #6349

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

Open
Ju-MINJAE wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −1
Open

fix: prevent crash in buildLocation when params stringification fails (#6310) #6349

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 5 additions & 1 deletion packages/router-core/src/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,11 @@ export class RouterCore<
const fn =
route.options.params?.stringify ?? route.options.stringifyParams
if (fn) {
Object.assign(nextParams, fn(nextParams))
try {
Copy link
Contributor

@brenelz brenelz Jan 17, 2026

Choose a reason for hiding this comment

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

im not sure if just ignoring the exception is a good fix

Copy link
Author

@Ju-MINJAE Ju-MINJAE Jan 17, 2026

Choose a reason for hiding this comment

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

Should I add a console.warn inside a process.env.NODE_ENV !== 'production' check as CodeRabbit suggested?

catch (err) {
-  //
+  if (process.env.NODE_ENV !== 'production') {
+    console.warn(
+      `Failed to stringify params for route ${route.id}:`,
+      err,
+    )
+  }
 }

Object.assign(nextParams, fn(nextParams))
} catch (err) {
//
}
Comment on lines +1704 to +1708
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add logging for stringify failures to aid debugging.

While preventing crashes is the right approach for resilience, the empty catch block makes it difficult to diagnose when and why parameter stringification fails. This could hide bugs or configuration issues that developers need to be aware of.

Consider logging the error at least in development mode:

🔍 Proposed enhancement to add development logging 
 try {
   Object.assign(nextParams, fn(nextParams))
 } catch (err) {
-  //
+  if (process.env.NODE_ENV !== 'production') {
+    console.warn(
+      `Failed to stringify params for route ${route.id}:`,
+      err,
+    )
+  }
 }

This maintains the crash prevention while providing visibility into stringify failures during development.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
Object.assign(nextParams, fn(nextParams))
} catch (err) {
//
}
try {
Object.assign(nextParams, fn(nextParams))
} catch (err) {
if (process.env.NODE_ENV !== 'production') {
console.warn(
`Failed to stringify params for route ${route.id}:`,
err,
)
}
}
🤖 Prompt for AI Agents 
In @packages/router-core/src/router.ts around lines 1704 - 1708, The empty catch
swallowing errors in the try around Object.assign(nextParams, fn(nextParams))
hides stringify failures; update the catch to log the error (including the error
object and relevant context such as nextParams and fn.name or a stringified fn)
so developers can debug, using the router's existing logger if available (e.g.,
this.logger or a module logger) or fallback to console.error, and guard the log
behind a development check (e.g., process.env.NODE_ENV !== 'production' or an
existing debug flag) so production noise is avoided.

}
}
}
Expand Down