Implement custom error page with improved handling #877
Conversation
- Added `src/hooks.client.ts` to capture and route unhandled client-side errors - Created `src/routes/error/+page.svelte` for user-friendly error reporting - Updated `+layout.svelte` to include error-catching logic and routing - Enhanced lexicon page error handling and redirection in `+page.ts` - Modified root `+page.js` to support new error routing structure
WalkthroughThis update introduces a new client-side error hook, adds a dedicated error page component, and enhances error handling in several route modules. It also changes the initialization of certain state variables in the layout. Error information is now surfaced to users via redirection and a detailed error page, improving visibility and reporting of client-side issues. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant ErrorPage
User->>App: Navigates to a route
App->>App: Processes load function
alt Error occurs in load
App->>User: Redirects to /error with message and stack in URL
User->>ErrorPage: Loads error page
ErrorPage->>User: Displays error message, stack, and reporting options
else No error
App->>User: Displays requested page
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/hooks.client.ts (2)
2-2: Remove unused import.The
gotofunction is imported but never used in this file.-import { goto } from '$app/navigation';
4-10: Consider enhancing error logging with additional context.The current implementation provides basic error logging, which is good. Consider adding more context for debugging purposes.
-export function handleError({ error, event }) { +export function handleError({ error, event }) { if (error instanceof Error) { - console.error('Client error:', error.message); + console.error('Client error:', { + message: error.message, + stack: error.stack, + url: event?.url?.href, + timestamp: new Date().toISOString() + }); } else { - console.error('Client error (unknown type):', error); + console.error('Client error (unknown type):', { + error, + url: event?.url?.href, + timestamp: new Date().toISOString() + }); } }src/routes/+page.js (1)
1-1: Remove unused import.The
redirectfunction is imported but not used. The code useswindow.location.hashinstead.-import { redirect } from '@sveltejs/kit';src/routes/lexicon/+page.ts (1)
10-10: Remove unused import.The
redirectfunction is imported but not used in this file.-import { redirect } from '@sveltejs/kit';src/routes/error/+page.svelte (1)
19-21: Consider using SvelteKit navigation instead of direct hash manipulation.While this approach works, using SvelteKit's navigation utilities would be more consistent with the framework's patterns.
+ import { goto } from '$app/navigation'; + function returnToText() { - window.location.hash = '/text'; + goto('/text'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
-
src/hooks.client.ts(1 hunks) -
src/routes/+layout.svelte(1 hunks) -
src/routes/+page.js(1 hunks) -
src/routes/error/+page.svelte(1 hunks) -
src/routes/lexicon/+page.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard `throw error(status, message)` syntax shown in official documentation.
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message `error(400, 'message')` or an object with a message property `error(400, { message: 'message' })`. Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
src/routes/lexicon/+page.ts (2)
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit, the correct syntax for throwing HTTP errors in load functions is error(statusCode, { message: 'error message' }) without the throw keyword. The error function from '@sveltejs/kit' should be called directly, not thrown like a regular JavaScript Error.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard throw error(status, message) syntax shown in official documentation.
src/routes/+page.js (3)
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit, the correct syntax for throwing HTTP errors in load functions is error(statusCode, { message: 'error message' }) without the throw keyword. The error function from '@sveltejs/kit' should be called directly, not thrown like a regular JavaScript Error.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard throw error(status, message) syntax shown in official documentation.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message error(400, 'message') or an object with a message property error(400, { message: 'message' }). Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
src/hooks.client.ts (3)
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard throw error(status, message) syntax shown in official documentation.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message error(400, 'message') or an object with a message property error(400, { message: 'message' }). Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit, the correct syntax for throwing HTTP errors in load functions is error(statusCode, { message: 'error message' }) without the throw keyword. The error function from '@sveltejs/kit' should be called directly, not thrown like a regular JavaScript Error.
src/routes/error/+page.svelte (6)
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard throw error(status, message) syntax shown in official documentation.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In Svelte 5, the page store is imported from $app/state instead of $app/stores. This is part of the migration from Svelte 4 to Svelte 5 where $app/stores is being deprecated in favor of $app/state.
Learnt from: judah-sotomayor
PR: #820
File: src/lib/components/Modal.svelte:28-34
Timestamp: 2025-06-05T17:25:45.457Z
Learning: In the sillsdev/appbuilder-pwa repository, the Modal.svelte component has an existing bug where {{ id }.showModal()} syntax was used instead of calling the exported showModal function directly. This bug predates Svelte 5 migration efforts.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message error(400, 'message') or an object with a message property error(400, { message: 'message' }). Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit, the correct syntax for throwing HTTP errors in load functions is error(statusCode, { message: 'error message' }) without the throw keyword. The error function from '@sveltejs/kit' should be called directly, not thrown like a regular JavaScript Error.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In SvelteKit 2.12 and Svelte 5, $app/stores was deprecated in favor of $app/state. When migrating from Svelte 4 to Svelte 5, imports should be changed from import { page } from '$app/stores'; to import { page } from '$app/state';. This change aligns with Svelte 5's reactivity model and provides more granular control over state updates.
src/routes/+layout.svelte (7)
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:380-381
Timestamp: 2025-07-04T18:48:12.143Z
Learning: In Svelte 5, variables declared with $state() are state runes and should be accessed directly without the $ prefix. The $ prefix is only needed for stores (writable(), readable(), derived()), not for state runes. State runes are inherently reactive without requiring the $ prefix.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In SvelteKit 2.12 and Svelte 5, $app/stores was deprecated in favor of $app/state. When migrating from Svelte 4 to Svelte 5, imports should be changed from import { page } from '$app/stores'; to import { page } from '$app/state';. This change aligns with Svelte 5's reactivity model and provides more granular control over state updates.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:201-209
Timestamp: 2025-07-04T18:53:08.369Z
Learning: In Svelte 5, $state creates a reactive rune, not a store. Stores are created with functions like writable(), readable(), and derived(). When using bind:this to bind DOM elements, use plain variables rather than reactive runes to ensure direct DOM property access works correctly.
Learnt from: judah-sotomayor
PR: #820
File: src/lib/components/Modal.svelte:28-34
Timestamp: 2025-06-05T17:25:45.457Z
Learning: In the sillsdev/appbuilder-pwa repository, the Modal.svelte component has an existing bug where {{ id }.showModal()} syntax was used instead of calling the exported showModal function directly. This bug predates Svelte 5 migration efforts.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In Svelte 5, the page store is imported from $app/state instead of $app/stores. This is part of the migration from Svelte 4 to Svelte 5 where $app/stores is being deprecated in favor of $app/state.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:322-324
Timestamp: 2025-07-04T18:50:06.709Z
Learning: In Svelte 5, variables that hold the result of $derived runes should be accessed directly without the $ prefix. The $ prefix is only needed for traditional Svelte stores (writable(), readable(), derived()), not for variables that hold the result of runes like $state() or $derived().
Learnt from: judah-sotomayor
PR: #840
File: src/lib/components/TextAppearanceSelector.svelte:165-197
Timestamp: 2025-06-16T18:47:15.122Z
Learning: In src/lib/components/TextAppearanceSelector.svelte, the user judah-sotomayor considers theme selection buttons not relevant for screen-reader users and prefers not to add accessibility labels to theme buttons.
🧬 Code Graph Analysis (1)
src/routes/lexicon/+page.ts (1)
src/lib/lexicon/index.ts (1)
ReversalIndex(1-3)
🔇 Additional comments (3)
src/routes/+layout.svelte (1)
88-89: LGTM! Proper Svelte 5 state initialization.The initialization of these state variables with
$state()follows the correct Svelte 5 pattern and ensures they start with defined state rather than being undefined.src/routes/error/+page.svelte (2)
13-17: Consider security implications of auto-filling GitHub issues with error details.The current implementation includes potentially sensitive information (full URL, user agent, stack trace) in the GitHub issue. Consider sanitizing this data or allowing users to review before submission.
Potential security concerns:
- Stack traces may contain file paths and internal application structure
- URLs may contain sensitive query parameters or tokens
- User agent strings provide detailed system information
Consider adding a confirmation step or sanitizing the data before pre-filling the GitHub issue.
24-50: Well-designed user experience for error reporting.The error page provides a good user experience with clear messaging, collapsible stack trace, and actionable buttons. The styling and layout are appropriate for an error page.
| try { | ||
| const ref = url.searchParams.get('ref'); | ||
| const audio = url.searchParams.get('audio') | ||
|
|
||
| if (!ref && !audio) { | ||
| throw new Error('Missing required query parameters: "ref" or "audio".'); | ||
| }; | ||
|
|
||
| return { ref, audio }; | ||
| return { ref, audio }; | ||
| } catch (err) { | ||
| // Redirect to your error page with message | ||
| const message = err instanceof Error ? err.message : 'Unknown error'; | ||
| window.location.hash = `/error?message=${encodeURIComponent(message)}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use proper SvelteKit error handling instead of client-side navigation.
The current approach using window.location.hash for error handling is inconsistent with SvelteKit patterns. Load functions should use SvelteKit's error() function for proper server-side error handling.
Based on the retrieved learnings, SvelteKit error handling should use the error() function. Here's the recommended approach:
+import { error } from '@sveltejs/kit';
+
/** @type {import('./$types').PageLoad} */
export async function load({ url }) {
- try {
- const ref = url.searchParams.get('ref');
- const audio = url.searchParams.get('audio')
+ const ref = url.searchParams.get('ref');
+ const audio = url.searchParams.get('audio');
- if (!ref && !audio) {
- throw new Error('Missing required query parameters: "ref" or "audio".');
- };
+ if (!ref && !audio) {
+ error(400, { message: 'Missing required query parameters: "ref" or "audio".' });
+ }
- return { ref, audio };
- } catch (err) {
- // Redirect to your error page with message
- const message = err instanceof Error ? err.message : 'Unknown error';
- window.location.hash = `/error?message=${encodeURIComponent(message)}`;
- }
+ return { ref, audio };
}🤖 Prompt for AI Agents
In src/routes/+page.js around lines 5 to 18, replace the client-side error
handling using window.location.hash with SvelteKit's server-side error handling
by importing and using the error() function from '@sveltejs/kit'. Instead of
catching and redirecting manually, throw an error with error(statusCode,
message) inside the load function when required query parameters are missing,
ensuring consistent and proper error handling in SvelteKit.
| @@ -0,0 +1,50 @@ | |||
| <script lang="ts"> | |||
| import { page } from '$app/stores'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update to use Svelte 5 compatible import.
Based on the retrieved learnings, $app/stores is deprecated in Svelte 5 in favor of $app/state.
- import { page } from '$app/stores';
+ import { page } from '$app/state';🤖 Prompt for AI Agents
In src/routes/error/+page.svelte at line 2, update the import statement to
replace '$app/stores' with '$app/state' to ensure compatibility with Svelte 5.
Change the import from "import { page } from '$app/stores';" to "import { page }
from '$app/state';".
| try { | ||
| const response = await fetch(`${base}/reversal/${key}/index.json`); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to load reversal index for language: ${key}`); | ||
| } | ||
| reversalIndexes[key] = (await response.json()) as ReversalIndex; | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : `Unknown error loading ${key}`; | ||
| const stack = err instanceof Error && err.stack ? err.stack : 'No stack trace'; | ||
| window.location.hash = `/error?message=${encodeURIComponent(message)}&stack=${encodeURIComponent(stack)}`; | ||
| } |
There was a problem hiding this comment.
Use consistent error handling and avoid exposing stack traces in URLs.
The error handling here is inconsistent with the rest of the load function and may expose sensitive information through stack traces in URLs.
Issues:
- Security concern: Stack traces in URLs may expose sensitive file paths and internal application structure
- Inconsistency: Other errors in this function use
throw new Error()which is the proper SvelteKit pattern - Client-side navigation: Using
window.location.hashin a load function is not the recommended SvelteKit approach
- try {
- const response = await fetch(`${base}/reversal/${key}/index.json`);
- if (!response.ok) {
- throw new Error(`Failed to load reversal index for language: ${key}`);
- }
- reversalIndexes[key] = (await response.json()) as ReversalIndex;
- } catch (err) {
- const message = err instanceof Error ? err.message : `Unknown error loading ${key}`;
- const stack = err instanceof Error && err.stack ? err.stack : 'No stack trace';
- window.location.hash = `/error?message=${encodeURIComponent(message)}&stack=${encodeURIComponent(stack)}`;
+ const response = await fetch(`${base}/reversal/${key}/index.json`);
+ if (!response.ok) {
+ throw new Error(`Failed to load reversal index for language: ${key}`);
}
+ reversalIndexes[key] = (await response.json()) as ReversalIndex;📝 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.
| try { | |
| const response = await fetch(`${base}/reversal/${key}/index.json`); | |
| if (!response.ok) { | |
| throw new Error(`Failed to load reversal index for language: ${key}`); | |
| } | |
| reversalIndexes[key] = (await response.json()) as ReversalIndex; | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : `Unknown error loading ${key}`; | |
| const stack = err instanceof Error && err.stack ? err.stack : 'No stack trace'; | |
| window.location.hash = `/error?message=${encodeURIComponent(message)}&stack=${encodeURIComponent(stack)}`; | |
| } | |
| const response = await fetch(`${base}/reversal/${key}/index.json`); | |
| if (!response.ok) { | |
| throw new Error(`Failed to load reversal index for language: ${key}`); | |
| } | |
| reversalIndexes[key] = (await response.json()) as ReversalIndex; |
🤖 Prompt for AI Agents
In src/routes/lexicon/+page.ts around lines 46 to 56, the current error handling
exposes stack traces in the URL, uses client-side navigation with
window.location.hash, and is inconsistent with the rest of the load function. To
fix this, remove the stack trace from the error handling to avoid exposing
sensitive information, replace the window.location.hash navigation with throwing
a new Error to follow SvelteKit's recommended pattern, and ensure all errors in
the load function consistently use throw new Error with appropriate messages.
|
Issue #493 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/routes/+page.js (1)
5-18: Use proper SvelteKit error handling instead of client-side navigation.The current approach using
window.location.hashfor error handling is inconsistent with SvelteKit patterns. Load functions should use SvelteKit'serror()function for proper server-side error handling.
🧹 Nitpick comments (1)
src/routes/+page.js (1)
1-1: Remove unused import.The
redirectimport is not used in this file. Consider removing it to keep imports clean.-import { redirect } from '@sveltejs/kit';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/hooks.client.ts(1 hunks)src/routes/+page.js(1 hunks)src/routes/lexicon/+page.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/routes/lexicon/+page.ts
- src/hooks.client.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard `throw error(status, message)` syntax shown in official documentation.
Learnt from: judah-sotomayor
PR: sillsdev/appbuilder-pwa#850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message `error(400, 'message')` or an object with a message property `error(400, { message: 'message' })`. Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
src/routes/+page.js (5)
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit, the correct syntax for throwing HTTP errors in load functions is error(statusCode, { message: 'error message' }) without the throw keyword. The error function from '@sveltejs/kit' should be called directly, not thrown like a regular JavaScript Error.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:54:56.765Z
Learning: In SvelteKit projects, verify the specific error handling pattern used in the codebase, as there may be variations from the standard throw error(status, message) syntax shown in official documentation.
Learnt from: judah-sotomayor
PR: #850
File: src/routes/notes/edit/[noteid]/+page.js:0-0
Timestamp: 2025-06-23T20:58:13.226Z
Learning: In SvelteKit error handling, the error function can be called with either a string message error(400, 'message') or an object with a message property error(400, { message: 'message' }). Both approaches are valid, but using the object format provides consistency when additional error properties might be needed.
Learnt from: chrisvire
PR: #845
File: src/routes/text/+page.svelte:3-3
Timestamp: 2025-07-04T18:51:27.821Z
Learning: In SvelteKit 2.12 and Svelte 5, $app/stores was deprecated in favor of $app/state. When migrating from Svelte 4 to Svelte 5, imports should be changed from import { page } from '$app/stores'; to import { page } from '$app/state';. This change aligns with Svelte 5's reactivity model and provides more granular control over state updates.
Learnt from: judah-sotomayor
PR: #820
File: src/lib/components/Modal.svelte:28-34
Timestamp: 2025-06-05T17:25:45.457Z
Learning: In the sillsdev/appbuilder-pwa repository, the Modal.svelte component has an existing bug where {{ id }.showModal()} syntax was used instead of calling the exported showModal function directly. This bug predates Svelte 5 migration efforts.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: lint
🔇 Additional comments (1)
src/routes/+page.js (1)
9-11: Good parameter validation logic.The validation correctly ensures at least one of the required parameters (
reforaudio) is present, with a clear error message.
1 participant
src/hooks.client.tsto capture and route unhandled client-side errorssrc/routes/error/+page.sveltefor user-friendly error reporting+layout.svelteto include error-catching logic and routing+page.ts+page.jsto support new error routing structureSummary by CodeRabbit
New Features
Bug Fixes
Refactor