Add global and route-level React ErrorBoundaries + error reporting #134
Conversation
…ptional Sentry) and tests
WalkthroughAdds a frontend error-reporting flow: React ErrorBoundary and ErrorFallback components, a reportError utility with optional Sentry integration and beacon/fetch reporting to a new backend endpoint, tests, and documentation; integrates ErrorBoundary into app routes and main render. Changes
sequenceDiagram
participant React as React Component
participant EB as ErrorBoundary
participant EF as ErrorFallback
participant Reporter as reportError
participant Console as Console
participant Sentry as Sentry SDK
participant Beacon as Beacon/Fetch
participant Backend as /api/frontend/errors
React->>EB: render (error thrown)
EB->>EB: getDerivedStateFromError / componentDidCatch
EB->>Reporter: reportError(error, errorInfo)
Reporter->>Console: console.error(formatted)
alt Sentry enabled
Reporter->>Sentry: captureException(...)
Sentry-->>Reporter: ack
end
alt navigator.sendBeacon available
Reporter->>Beacon: sendBeacon(payload)
else
Reporter->>Beacon: fetch POST /api/frontend/errors (keepalive)
end
Beacon->>Backend: POST payload
Backend-->>Beacon: 204 No Content
Reporter-->>EB: reporting complete
EB->>EF: render fallback UI
EF-->>React: user clicks "Try again"
EF->>EB: onReset()
EB->>React: render children normally
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
frontend/package.json (1)
82-88: Consider moving Vitest configuration to a dedicated config file.Placing Vitest configuration directly in
package.jsonis unconventional. The recommended approach is to usevitest.config.tsor add the configuration to your existingvite.config.tsfile for better maintainability and IDE support.Create a
vitest.config.tsfile:import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { environment: 'jsdom', globals: true, }, });Or add to your existing
vite.config.ts:import { defineConfig } from 'vite'; import react from '@vitejs/plugin-react-swc'; export default defineConfig({ plugins: [react()], test: { environment: 'jsdom', globals: true, }, });frontend/ERROR_REPORTING.md (2)
13-24: Add language specifiers to code blocks for better rendering.The code blocks are missing language specifiers, which improves syntax highlighting and readability.
Apply these changes:
-``` +```bash VITE_SENTRY_DSN=https://<public-key>@o0.ingest.sentry.io/<project-id>-
+bash
cd frontend
npm install
npm install @sentry/react @sentry/tracing --save
npm run devBased on static analysis hints from markdownlint-cli2.
30-33: Add language specifier to the test commands code block.-``` +```bash cd frontend npm install npm run testBased on static analysis hints from markdownlint-cli2. </blockquote></details> <details> <summary>frontend/src/components/ErrorBoundary.tsx (1)</summary><blockquote> `44-55`: **Consider removing the type assertion for better type safety.** Line 54 casts `children` to `React.ReactElement`, but `children` is typed as `React.ReactNode` which can include fragments, arrays, strings, and other types. This assertion is unnecessary since the render method returns `React.ReactNode`. ```diff - return this.props.children as React.ReactElement; + return this.props.children;frontend/src/components/ErrorFallback.tsx (1)
9-26: Consider using Tailwind CSS classes for consistency with the codebase.The rest of the codebase uses Tailwind CSS, but this component uses inline styles. For consistency and maintainability, consider refactoring to use Tailwind classes.
-export default function ErrorFallback({ error, errorInfo, onReset }: Props) { - return ( - <div style={{ padding: 24, fontFamily: 'Inter, system-ui, sans-serif' }}> - <h2>Something went wrong</h2> - {error && <pre style={{ whiteSpace: 'pre-wrap' }}>{error.message}</pre>} - {errorInfo && ( - <details style={{ whiteSpace: 'pre-wrap', marginTop: 12 }}> - {errorInfo.componentStack} - </details> - )} - <div style={{ marginTop: 16 }}> - <button onClick={onReset} style={{ padding: '8px 12px' }}> - Try again - </button> - </div> - </div> - ); -} +export default function ErrorFallback({ error, errorInfo, onReset }: Props) { + return ( + <div className="p-6 font-sans"> + <h2 className="text-xl font-semibold mb-4">Something went wrong</h2> + {error && <pre className="whitespace-pre-wrap mb-3 text-sm">{error.message}</pre>} + {errorInfo && ( + <details className="whitespace-pre-wrap mt-3 text-xs text-gray-600"> + <summary className="cursor-pointer mb-2">Component stack</summary> + {errorInfo.componentStack} + </details> + )} + <div className="mt-4"> + <button + onClick={onReset} + className="px-4 py-2 bg-blue-600 text-white rounded hover:bg-blue-700" + > + Try again + </button> + </div> + </div> + ); +}Note: The
<details>element currently lacks a<summary>child, which means the content is visible by default. The suggested refactor adds a summary for better UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
frontend/ERROR_REPORTING.md(1 hunks)frontend/package.json(3 hunks)frontend/src/App.tsx(3 hunks)frontend/src/components/ErrorBoundary.tsx(1 hunks)frontend/src/components/ErrorFallback.tsx(1 hunks)frontend/src/components/__tests__/ErrorBoundary.test.tsx(1 hunks)frontend/src/main.tsx(1 hunks)frontend/src/utils/errorReporting.ts(1 hunks)frontend/src/utils/sentry.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/utils/errorReporting.ts (1)
frontend/src/utils/sentry.ts (2)
initSentry(5-28)captureException(30-41)
frontend/src/components/__tests__/ErrorBoundary.test.tsx (1)
frontend/src/components/ErrorBoundary.tsx (1)
render(44-55)
🪛 markdownlint-cli2 (0.18.1)
frontend/ERROR_REPORTING.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (11)
frontend/package.json (2)
10-10: LGTM!The test script addition is correct and follows standard conventions.
62-65: Verify the testing library versions are compatible.The testing library versions look reasonable, but ensure they're compatible with React 18.3.1 and vitest 0.34.0.
frontend/src/main.tsx (1)
7-15: LGTM!The global ErrorBoundary is correctly positioned to catch errors from the entire application tree, including the router and all route components.
frontend/src/components/__tests__/ErrorBoundary.test.tsx (1)
1-33: LGTM!The test comprehensively verifies the ErrorBoundary's core functionality including error catching, fallback display, and the retry/reset flow. The test structure using a controllable "Bomb" component is a clean pattern for testing error boundaries.
frontend/src/App.tsx (1)
74-74: Route-level error boundaries provide good isolation for critical features.Wrapping these debate and game routes with individual ErrorBoundaries creates a two-tier error handling strategy:
- Route-level boundaries isolate errors to specific pages
- The global boundary (in main.tsx) catches errors elsewhere
This prevents a crash in one debate room from affecting navigation or other routes, which is appropriate for these real-time collaborative features.
Also applies to: 94-99
frontend/src/components/ErrorBoundary.tsx (2)
5-14: LGTM!The Props and State types are well-defined and appropriate for an error boundary component.
23-42: LGTM!The error handling logic is solid:
- Properly uses both
getDerivedStateFromErrorandcomponentDidCatch- Defensive try/catch around
reportErrorprevents reporting failures from crashing the boundary- Reset mechanism correctly clears state and invokes the optional callback
frontend/src/utils/errorReporting.ts (3)
3-8: Eager Sentry initialization at module load is acceptable but worth noting.The
initSentry()call runs immediately when this module is imported. This is generally safe because:
- The sentry module checks for
VITE_SENTRY_DSNand skips initialization if not present- Vite environment variables are available at module load time
- Failures are caught and swallowed
However, be aware that this executes before the React app mounts, which could affect initialization order in some scenarios.
10-21: LGTM!The multi-layered error handling approach is appropriate for error reporting infrastructure:
- Console logging provides immediate visibility
- Sentry capture is isolated in its own try/catch
- Failures in one channel don't affect others
22-52: LGTM!The backend error reporting implementation follows best practices:
- Prefers
navigator.sendBeaconfor reliability (especially during page unload)- Correctly creates a Blob with the appropriate content type
- Falls back to
fetchwithkeepalive: truewhen beacon is unavailable- Silently handles reporting failures to avoid cascading errors
frontend/src/utils/sentry.ts (1)
43-45: LGTM!The implementation is straightforward and correct.
| "@testing-library/jest-dom": "^6.0.1", | ||
| "@testing-library/react": "^14.0.0", | ||
| "@testing-library/user-event": "^14.4.3", | ||
| "vitest": "^0.34.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable vitest version 2024 2025
💡 Result:
- Latest stable in 2024: Vitest 2.x (the Vitest 2 major was released mid‑2024). [1][2]
- Latest stable in 2025: Vitest 3 series — 3.0 was released Jan 17, 2025 and the latest 3.x stable published in 2025 was 3.2.4 (June 17, 2025). [1][3]
Sources:
- Vitest blog — Vitest 3.0 announcement (notes Vitest 2 was released ~mid‑2024). [vitest.dev/blog/vitest-3].
- Vitest GitHub discussion / timeline for Vitest 2. [github.com/Vitest 2 vitest-dev/vitest#5210].
- Release listing showing Vitest 3.2.4 (Jun 17, 2025). [releasealert.dev/npm/_/vitest].
Update vitest to the latest stable version.
Version 0.34.0 is significantly outdated (2023 release). Current stable versions are in the 3.x range, with 3.2.4 being the latest stable release as of mid-2025. Upgrading will provide important bug fixes, features, and maintenance.
🤖 Prompt for AI Agents
In frontend/package.json around line 65, the vitest dependency is pinned to the
old version "^0.34.0"; update the vitest entry to the current stable range (e.g.
"^3.2.4"), then reinstall dependencies (npm/yarn/pnpm install) and run the test
suite; if any test or config failures occur, update Vitest config and any
deprecated API usages (vite/vitest config keys, reporter options, globals, and
setup files) to match Vitest 3.x breaking changes and re-run tests until green.
| @@ -0,0 +1,45 @@ | |||
| // Lightweight Sentry wrapper. Initialization is optional and controlled by Vite env var VITE_SENTRY_DSN. | |||
| // This module intentionally avoids throwing when Sentry isn't installed or DNS is missing. | |||
There was a problem hiding this comment.
Fix typo: "DNS" should be "DSN".
The comment mentions "DNS" but should be "DSN" (Data Source Name).
Apply this diff:
-// This module intentionally avoids throwing when Sentry isn't installed or DNS is missing.
+// This module intentionally avoids throwing when Sentry isn't installed or DSN is missing.📝 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.
| // This module intentionally avoids throwing when Sentry isn't installed or DNS is missing. | |
| // This module intentionally avoids throwing when Sentry isn't installed or DSN is missing. |
🤖 Prompt for AI Agents
In frontend/src/utils/sentry.ts around line 2, the file comment incorrectly says
"DNS" instead of "DSN"; update the comment text to replace "DNS" with "DSN" so
it correctly refers to the Sentry Data Source Name (e.g., "This module
intentionally avoids throwing when Sentry isn't installed or DSN is missing.").
| export function initSentry() { | ||
| try { | ||
| // Use dynamic import to avoid forcing the dependency at runtime if not installed | ||
| const dsn = (import.meta as any).env?.VITE_SENTRY_DSN; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use proper typing for Vite environment variables.
The type assertion (import.meta as any) bypasses type safety. In Vite projects, import.meta.env is properly typed and VITE_SENTRY_DSN should be accessible directly.
Apply this diff:
- const dsn = (import.meta as any).env?.VITE_SENTRY_DSN;
+ const dsn = import.meta.env.VITE_SENTRY_DSN;If TypeScript complains about the type, add the variable to your vite-env.d.ts:
interface ImportMetaEnv {
readonly VITE_SENTRY_DSN?: string;
}🤖 Prompt for AI Agents
In frontend/src/utils/sentry.ts around line 8, replace the unsafe cast
(import.meta as any).env?.VITE_SENTRY_DSN with the properly typed
import.meta.env.VITE_SENTRY_DSN; if TypeScript complains, add a vite-env.d.ts
declaration including interface ImportMetaEnv { readonly VITE_SENTRY_DSN?:
string; } so the environment var is correctly typed and no any cast is used.
| const Sentry = require('@sentry/react'); | ||
| const Tracing = require('@sentry/tracing'); |
There was a problem hiding this comment.
Replace require() with dynamic import() for ES module compatibility.
Vite uses ES modules by default, and require() (CommonJS syntax) may fail at runtime. The comments mention "dynamic import" but the code uses require(), which is inconsistent. Use proper dynamic import() instead.
Apply this diff:
- // dynamic import so builds that don't include @sentry won't fail at parse-time
- // eslint-disable-next-line @typescript-eslint/no-var-requires
- const Sentry = require('@sentry/react');
- const Tracing = require('@sentry/tracing');
- Sentry.init({
- dsn,
- integrations: [new Tracing.BrowserTracing()],
- tracesSampleRate: 0.05,
- });
+ // dynamic import so builds that don't include @sentry won't fail at parse-time
+ const [{ default: Sentry }, { BrowserTracing }] = await Promise.all([
+ import('@sentry/react'),
+ import('@sentry/react'),
+ ]);
+ Sentry.init({
+ dsn,
+ integrations: [new BrowserTracing()],
+ tracesSampleRate: 0.05,
+ });Note: This also makes initSentry async, so callers must await it. Update the return type to Promise<boolean> and adjust the signature:
-export function initSentry() {
+export async function initSentry() {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/utils/sentry.ts around lines 12-13, replace the CommonJS
require() calls for '@sentry/react' and '@sentry/tracing' with dynamic ES module
imports using import(), make initSentry an async function, update its return
type to Promise<boolean>, and update the function signature accordingly; ensure
you await the dynamic imports before using Sentry/Tracing and update any callers
to await initSentry() so they handle the Promise.
| const Sentry = require('@sentry/react'); | ||
| const Tracing = require('@sentry/tracing'); | ||
| Sentry.init({ | ||
| dsn, | ||
| integrations: [new Tracing.BrowserTracing()], | ||
| tracesSampleRate: 0.05, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check Sentry versions in package.json
find . -name "package.json" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Repository: AOSSIE-Org/DebateAI
Length of output: 2672
🏁 Script executed:
# Search for other Sentry imports in the codebase
rg "@sentry" --type json --type ts --type tsx -A 2 -B 2Repository: AOSSIE-Org/DebateAI
Length of output: 90
🌐 Web query:
What is the correct import for BrowserTracing in Sentry React SDK version 7 and later?
💡 Result:
Use the SDK export browserTracingIntegration from @sentry/react. Example:
import * as Sentry from "@sentry/react";
Sentry.init({ integrations: [Sentry.browserTracingIntegration()] });. [1][2]
Sources:
[1] Sentry v7 deprecations (BrowserTracing → browserTracingIntegration).
[2] Sentry BrowserTracing docs (shows Sentry.browserTracingIntegration usage).
🏁 Script executed:
# Find the sentry.ts file
find . -name "sentry.ts" -o -name "sentry.js" -o -name "*sentry*" -type f | grep -E "\.(ts|tsx|js|jsx)$"Repository: AOSSIE-Org/DebateAI
Length of output: 93
🏁 Script executed:
# Check the actual file content if it exists
cat -n frontend/src/utils/sentry.ts 2>/dev/null || echo "File not found"Repository: AOSSIE-Org/DebateAI
Length of output: 2001
🏁 Script executed:
# Search for any Sentry imports or configuration in the codebase
rg "@sentry" -t typescript -A 3 -B 1Repository: AOSSIE-Org/DebateAI
Length of output: 97
Update to Sentry SDK 7+ compatible API.
The code uses the deprecated BrowserTracing class from @sentry/tracing (line 13-16), which was removed in Sentry SDK 7.x. Replace with Sentry.browserTracingIntegration() exported from @sentry/react:
const Sentry = require('@sentry/react');
Sentry.init({
dsn,
integrations: [Sentry.browserTracingIntegration()],
tracesSampleRate: 0.05,
});
Remove the @sentry/tracing import entirely and add @sentry/react to package.json dependencies.
🤖 Prompt for AI Agents
In frontend/src/utils/sentry.ts around lines 12 to 18, the code imports and uses
the removed BrowserTracing class from @sentry/tracing; remove the
@sentry/tracing import, call Sentry.browserTracingIntegration() from
@sentry/react in the integrations array when initializing Sentry, and ensure
@sentry/react is listed as a dependency in package.json (remove @sentry/tracing
if no longer needed).
| export function captureException(err: unknown, context?: Record<string, unknown>) { | ||
| try { | ||
| if (!sentryInitialized) return; | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| const Sentry = require('@sentry/react'); | ||
| Sentry.captureException(err, { extra: context }); | ||
| } catch (e) { | ||
| // swallow — reporting must never crash the app | ||
| // eslint-disable-next-line no-console | ||
| console.warn('Sentry capture failed', e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Replace require() with dynamic import() in captureException.
Similar to initSentry, this function uses require() which may fail in Vite's ES module environment. The error handling approach is good—non-fatal and logs warnings appropriately.
Apply this diff:
-export function captureException(err: unknown, context?: Record<string, unknown>) {
+export async function captureException(err: unknown, context?: Record<string, unknown>) {
try {
if (!sentryInitialized) return;
- // eslint-disable-next-line @typescript-eslint/no-var-requires
- const Sentry = require('@sentry/react');
+ const { default: Sentry } = await import('@sentry/react');
Sentry.captureException(err, { extra: context });
} catch (e) {
// swallow — reporting must never crash the app
// eslint-disable-next-line no-console
console.warn('Sentry capture failed', e);
}
}Note: This makes the function async. Callers should be updated to handle the Promise (though they don't need to await it if fire-and-forget behavior is acceptable).
📝 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.
| export function captureException(err: unknown, context?: Record<string, unknown>) { | |
| try { | |
| if (!sentryInitialized) return; | |
| // eslint-disable-next-line @typescript-eslint/no-var-requires | |
| const Sentry = require('@sentry/react'); | |
| Sentry.captureException(err, { extra: context }); | |
| } catch (e) { | |
| // swallow — reporting must never crash the app | |
| // eslint-disable-next-line no-console | |
| console.warn('Sentry capture failed', e); | |
| } | |
| } | |
| export async function captureException(err: unknown, context?: Record<string, unknown>) { | |
| try { | |
| if (!sentryInitialized) return; | |
| const { default: Sentry } = await import('@sentry/react'); | |
| Sentry.captureException(err, { extra: context }); | |
| } catch (e) { | |
| // swallow — reporting must never crash the app | |
| // eslint-disable-next-line no-console | |
| console.warn('Sentry capture failed', e); | |
| } | |
| } |
🤖 Prompt for AI Agents
frontend/src/utils/sentry.ts lines 30-41: the function uses require() which can
fail under Vite/ESM; make captureException async, replace the synchronous
require with a dynamic import by awaiting import('@sentry/react') inside the try
block, then call Sentry.captureException(err, { extra: context }) on the
imported module (handle default vs named export if necessary), keep the existing
try/catch and warning behavior, and note callers must accept the returned
Promise (they can fire-and-forget if desired).
… error reporting, and server endpoint for reports
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/routes/frontend_errors.go (1)
10-18: Consider adding HTTP-level request size limits.The struct has no field-length constraints, which means large stack traces or other fields could consume excessive memory. While this may be handled by Gin's default limits, explicitly setting a reasonable maximum request body size (e.g., 100KB) for this endpoint would provide better protection against resource exhaustion.
This can be configured in main.go when registering the route:
router.MaxMultipartMemory = 100 << 10 // 100 KB limit for this endpointOr using middleware to limit request body size specifically for this route.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/cmd/server/main.go(1 hunks)backend/routes/frontend_errors.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/cmd/server/main.go (1)
backend/routes/frontend_errors.go (1)
HandleFrontendError(23-39)
| // Frontend error reporting endpoint (best-effort): accepts lightweight error payloads | ||
| router.POST("/api/frontend/errors", routes.HandleFrontendError) | ||
|
|
There was a problem hiding this comment.
Add rate limiting to prevent abuse of the public error reporting endpoint.
This endpoint is publicly accessible without authentication and lacks visible rate limiting. Malicious actors could spam the endpoint to exhaust server resources (memory, disk space via logs) or cause denial-of-service.
Consider implementing rate limiting middleware (e.g., by IP address) to restrict the number of error reports accepted per time window.
Example using a hypothetical rate limiter:
// Frontend error reporting endpoint (best-effort): accepts lightweight error payloads
rateLimitedErrorHandler := middlewares.RateLimitMiddleware(
100, // max requests
time.Minute, // time window
)
router.POST("/api/frontend/errors", rateLimitedErrorHandler, routes.HandleFrontendError)🤖 Prompt for AI Agents
In backend/cmd/server/main.go around lines 105-107, the public frontend error
reporting endpoint lacks rate limiting and can be abused; add an IP-based rate
limiting middleware and attach it to the POST /api/frontend/errors route.
Implement or use an existing middleware that tracks requests per remote IP
(e.g., 100 requests per minute as a sensible default), returns HTTP 429 when the
limit is exceeded, and does minimal in-memory bookkeeping (LRU or token-bucket)
to avoid unbounded memory growth; update the route to use that middleware before
routes.HandleFrontendError, ensure relevant imports are added, and add a brief
log entry when requests are rejected so operators can monitor abuse.
|
@soumyalimje Pls include the testing evidences |
|
sure sir , now I have included the testing evidence |
include relevant screenshots |
Screen.Recording.2026-01-03.at.6.28.10.PM.movsir I was trying to upload ss but its showing the error since days |
2 participants
Summary: Add a safe, opt-in error-handling layer so React render/lifecycle errors no longer unmount the entire app. Introduce a global ErrorBoundary (wraps app), route-level wrappers for critical pages (debate/chat/game), a simple fallback UI with retry, and a best-effort error reporter that logs to console, posts to /api/frontend/errors and forwards to Sentry when VITE_SENTRY_DSN is provided.
Behavior changes
Global: any uncaught render error shows ErrorFallback instead of blank screen.
Route-level: failing pages are isolated; other pages remain usable.
Reporting: errors are logged locally and sent to /api/frontend/errors; optional Sentry capture if DSN is configured.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.