fix(webapp): stop leaking exception messages on 5xx API responses#3536
fix(webapp): stop leaking exception messages on 5xx API responses#3536
Conversation
Across the webapp API routes, the catch-all 500 branch surfaced raw `error.message` verbatim. When the underlying exception originated from an internal subsystem, this leaked server-side details into customer-visible responses. Replace each leaking branch with a generic body and route the full error through logger.error so visibility is preserved server-side. 4xx branches that intentionally surface user-facing messages (e.g. ServiceValidationError) are left intact.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (14)
📜 Recent review details⏰ 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). (22)
WalkthroughThis PR systematically prevents API 500 responses from exposing raw exception messages by adding server-side error logging across 22 Remix route handlers. Each route now imports a logger service and wraps unexpected errors in catch blocks: full error details are logged server-side while clients receive a fixed generic message ("Something went wrong"). Validation errors and their messages (4xx responses) remain unchanged. The change is declared in a new server-changes manifest entry. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/admin.notifications.tsx (1)
248-276:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
handleArchiveAction,handleDeleteAction, andhandlePublishNowActionlack error handlingThis PR correctly sanitizes error handling in
handleCreateActionandhandleEditAction, but the three remaining action handlers call their respective service functions without anytry/catch. An unexpected throw from those services would escape unlogged and potentially surface raw details via Remix's default error handling — the same class of issue this PR addresses elsewhere in the file.🛡️ Suggested fix
async function handleArchiveAction(formData: FormData) { const notificationId = formData.get("notificationId") as string; if (!notificationId) { return typedjson({ error: "Missing notificationId" }, { status: 400 }); } - await archivePlatformNotification(notificationId); - return typedjson({ success: true }); + try { + await archivePlatformNotification(notificationId); + return typedjson({ success: true }); + } catch (error) { + logger.error("Failed to archive platform notification", { error }); + return typedjson({ error: "Something went wrong" }, { status: 500 }); + } } async function handleDeleteAction(formData: FormData) { const notificationId = formData.get("notificationId") as string; if (!notificationId) { return typedjson({ error: "Missing notificationId" }, { status: 400 }); } - await deletePlatformNotification(notificationId); - return typedjson({ success: true }); + try { + await deletePlatformNotification(notificationId); + return typedjson({ success: true }); + } catch (error) { + logger.error("Failed to delete platform notification", { error }); + return typedjson({ error: "Something went wrong" }, { status: 500 }); + } } async function handlePublishNowAction(formData: FormData) { const notificationId = formData.get("notificationId") as string; if (!notificationId) { return typedjson({ error: "Missing notificationId" }, { status: 400 }); } - await publishNowPlatformNotification(notificationId); - return typedjson({ success: true }); + try { + await publishNowPlatformNotification(notificationId); + return typedjson({ success: true }); + } catch (error) { + logger.error("Failed to publish platform notification", { error }); + return typedjson({ error: "Something went wrong" }, { status: 500 }); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/admin.notifications.tsx` around lines 248 - 276, Wrap the bodies of handleArchiveAction, handleDeleteAction, and handlePublishNowAction in try/catch blocks like handleCreateAction/handleEditAction: validate notificationId as you already do, then inside try call archivePlatformNotification/deletePlatformNotification/publishNowPlatformNotification respectively, and on success return typedjson({ success: true }); in catch, log the caught error (use the same logger used in the file or console.error) and return typedjson({ error: "Failed to <action> notification" }, { status: 500 }) where <action> is "archive", "delete", or "publish now" to avoid leaking raw error details. Ensure the catch returns a 500 typedjson response and does not rethrow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/webapp/app/routes/admin.notifications.tsx`:
- Around line 248-276: Wrap the bodies of handleArchiveAction,
handleDeleteAction, and handlePublishNowAction in try/catch blocks like
handleCreateAction/handleEditAction: validate notificationId as you already do,
then inside try call
archivePlatformNotification/deletePlatformNotification/publishNowPlatformNotification
respectively, and on success return typedjson({ success: true }); in catch, log
the caught error (use the same logger used in the file or console.error) and
return typedjson({ error: "Failed to <action> notification" }, { status: 500 })
where <action> is "archive", "delete", or "publish now" to avoid leaking raw
error details. Ensure the catch returns a 500 typedjson response and does not
rethrow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e393c041-4d32-4537-81c3-83e63f3c4f3e
📒 Files selected for processing (20)
.server-changes/sanitize-api-500-errors.mdapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/api.v1.deployments.tsapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v3.batches.$batchId.items.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts
💤 Files with no reviewable changes (1)
- apps/webapp/app/routes/api.v3.batches.$batchId.items.ts
📜 Review details
⏰ 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). (29)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/routes/admin.notifications.tsx
🧠 Learnings (4)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/routes/api.v1.runs.$runParam.result.tsapps/webapp/app/routes/admin.api.v1.platform-notifications.tsapps/webapp/app/routes/api.v1.runs.$runParam.attempts.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.tsapps/webapp/app/routes/api.v1.runs.$runId.tags.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.tsapps/webapp/app/routes/admin.notifications.tsxapps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.tsapps/webapp/app/routes/api.v1.queues.tsapps/webapp/app/routes/api.v1.batches.$batchParam.results.tsapps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.tsapps/webapp/app/routes/api.v1.runs.$runParam.reschedule.tsapps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.tsapps/webapp/app/routes/api.v1.tasks.$taskId.trigger.tsapps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.tsapps/webapp/app/routes/api.v1.schedules.tsapps/webapp/app/routes/api.v1.waitpoints.tokens.tsapps/webapp/app/routes/api.v1.deployments.ts
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
🪛 LanguageTool
.server-changes/sanitize-api-500-errors.md
[grammar] ~6-~6: Ensure spelling is correct
Context: ...eption messages on 500 responses across webapp API routes; return a generic error stri...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (20)
.server-changes/sanitize-api-500-errors.md (1)
1-6: LGTM!The changelog entry correctly describes the change and uses the proper frontmatter format.
apps/webapp/app/routes/api.v1.batches.$batchParam.results.ts (1)
39-42: LGTM!Error is correctly logged server-side and the generic response is returned.
apps/webapp/app/routes/api.v1.queues.ts (1)
34-41: LGTM!
ServiceValidationErrormessage is correctly preserved for 422 responses; unexpected errors are now logged and return a generic 500.apps/webapp/app/routes/admin.notifications.tsx (1)
230-239: LGTM — neverthrow non-validation error branch correctly sanitized.
err(theResult.errorvalue) is logged while the client receives only the generic message.apps/webapp/app/routes/api.v1.schedules.$scheduleId.deactivate.ts (1)
71-74: LGTM!apps/webapp/app/routes/api.v1.runs.$runId.tags.ts (1)
88-91: LGTM!apps/webapp/app/routes/api.v1.runs.$runParam.result.ts (1)
38-41: LGTM!apps/webapp/app/routes/api.v1.schedules.$scheduleId.activate.ts (1)
71-74: LGTM!apps/webapp/app/routes/api.v1.projects.$projectRef.alertChannels.ts (1)
8-8: Sanitized 500 handling is correctly implemented.This change removes client-side leakage of internal exception details while preserving server-side diagnostics via
logger.error.Also applies to: 92-93
apps/webapp/app/routes/api.v1.deployments.ts (1)
60-61: 500 response sanitization and logging look good.Unexpected errors are now logged server-side and no longer exposed to API clients.
apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts (1)
158-159: Error-path update is solid.The route now logs internal failures while keeping 500 responses generic, without changing intended typed-error behavior.
apps/webapp/app/routes/api.v1.runs.$runFriendlyId.input-streams.wait.ts (1)
14-14: Good sanitization update for catch-all failures.Unexpected exceptions are now logged and returned as a generic 500 response.
Also applies to: 144-145
apps/webapp/app/routes/admin.api.v1.platform-notifications.ts (1)
3-3: Looks good — error leakage is removed here as well.The non-validation failure path now correctly logs server-side and returns a safe generic 500 body.
Also applies to: 46-47
apps/webapp/app/routes/api.v1.waitpoints.tokens.ts (1)
13-13: Catch-all error handling change is correct.Internal errors are logged while clients receive a generic 500 response, preventing message disclosure.
Also applies to: 98-99
apps/webapp/app/routes/api.v1.runs.$runParam.reschedule.ts (1)
9-9: This route’s 500 sanitization is implemented correctly.Unexpected failures are now handled with server-side logging plus a non-leaking generic error response.
Also applies to: 90-91
apps/webapp/app/routes/api.v1.runs.$runParam.attempts.ts (1)
5-5: LGTM for this error-path update.The catch-all branch now avoids exposing exception text and retains server-side diagnostics.
Also applies to: 44-45
apps/webapp/app/routes/api.v1.schedules.$scheduleId.ts (2)
61-63: Good 500 sanitization for DELETE path.Line 61-Line 63 correctly logs the server-side error and returns a generic 500 body, preventing exception-message leakage to clients.
112-113: Good 500 sanitization for PUT path.Line 112-Line 113 applies the same safe pattern (log internally, return generic message) while preserving typed validation error behavior.
apps/webapp/app/routes/realtime.v1.sessions.$session.$io.append.ts (1)
138-143: Solid non-validation failure handling on append.Line 138-Line 143 now logs internal details and returns a generic 500, which closes the previous leakage path without changing client-visible validation errors.
apps/webapp/app/routes/api.v1.schedules.ts (1)
75-76: Catch-block hardening looks correct.Line 75-Line 76 properly logs the unexpected exception and returns a generic 500 response, removing raw message exposure.
… in try/catch The three Prisma-backed dashboard action helpers (archivePlatformNotification, deletePlatformNotification, publishNowPlatformNotification) throw on underlying DB errors. Without try/catch in the route handler, the throw escapes to Remix's default action error response, which serialises message + stack into the JSON body — exposing server-side details and absolute build paths to the dashboard caller. Wrap each handler in try/catch with logger.error + a generic 500 typedjson body, matching the pattern already used by handleCreateAction and handleEditAction in the same file.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/webapp/app/routes/admin.notifications.tsx (1)
257-259: ⚡ Quick winConsider including
notificationIdin the error log context.Without it, correlating a logged failure to a specific notification requires cross-referencing other signals. A one-line change closes that gap.
🔧 Proposed improvement
- logger.error("Failed to archive platform notification", { error }); + logger.error("Failed to archive platform notification", { notificationId, error }); - logger.error("Failed to delete platform notification", { error }); + logger.error("Failed to delete platform notification", { notificationId, error }); - logger.error("Failed to publish platform notification", { error }); + logger.error("Failed to publish platform notification", { notificationId, error });Also applies to: 272-274, 287-289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/webapp/app/routes/admin.notifications.tsx` around lines 257 - 259, The catch blocks calling logger.error in admin.notifications.tsx should include the notificationId in the structured log context to make failures traceable; locate the catch handlers that currently call logger.error("Failed to archive/platform/unarchive platform notification", { error }) (and similar at the other two catch sites) and add the notificationId value to the context object (e.g., { error, notificationId }) so each error log includes the offending notification's id while leaving the typedjson responses unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/webapp/app/routes/admin.notifications.tsx`:
- Around line 257-259: The catch blocks calling logger.error in
admin.notifications.tsx should include the notificationId in the structured log
context to make failures traceable; locate the catch handlers that currently
call logger.error("Failed to archive/platform/unarchive platform notification",
{ error }) (and similar at the other two catch sites) and add the notificationId
value to the context object (e.g., { error, notificationId }) so each error log
includes the offending notification's id while leaving the typedjson responses
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 938a9eb4-9a49-44d2-bda4-cfdf8e8715f9
📒 Files selected for processing (1)
apps/webapp/app/routes/admin.notifications.tsx
📜 Review details
⏰ 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). (29)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/routes/admin.notifications.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/routes/admin.notifications.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/routes/admin.notifications.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathUse named constants for sentinel/placeholder values (e.g.
const UNSET_VALUE = '__unset__') instead of raw string literals scattered across comparisons
Files:
apps/webapp/app/routes/admin.notifications.tsx
apps/webapp/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Only use
useCallback/useMemofor context provider values, expensive derived data that is a dependency elsewhere, or stable refs required by a dependency array. Don't wrap ordinary event handlers or trivial computations
Files:
apps/webapp/app/routes/admin.notifications.tsx
{apps,internal-packages}/**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpm run typecheckto verify changes in apps and internal packages (apps/*,internal-packages/*) instead ofbuild, which proves almost nothing about correctness
Files:
apps/webapp/app/routes/admin.notifications.tsx
{package.json,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin Zod to version 3.25.76 exactly across the entire monorepo - never use a different version or version range
Files:
apps/webapp/app/routes/admin.notifications.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js}: Import from@trigger.dev/coreusing subpaths only, never the root export
Always import tasks from@trigger.dev/sdk, never from@trigger.dev/sdk/v3or deprecatedclient.defineJob
Add crumbs to code using//@Crumbscomments or `// `#region` `@crumbsblocks for debug tracing during development
Files:
apps/webapp/app/routes/admin.notifications.tsx
**/*.{ts,tsx,js,jsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code formatting is enforced using Prettier. Run
pnpm run formatbefore committing
Files:
apps/webapp/app/routes/admin.notifications.tsx
🧠 Learnings (4)
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/routes/admin.notifications.tsx
🔇 Additional comments (3)
apps/webapp/app/routes/admin.notifications.tsx (3)
40-40: LGTM — logger import.
238-239: LGTM — neverthrow non-validation error paths correctly sanitised.
logger.errorcaptures the full error server-side, and the generic 500 body replaces the previously-leakingerr.message. BothhandleCreateActionandhandleEditActionfollow the same correct pattern.Also applies to: 330-331
254-260: ⚡ Quick winNo action required — the three functions already throw as expected.
The three functions (
archivePlatformNotification,deletePlatformNotification, andpublishNowPlatformNotification) returnPromise<void>and directly await prisma operations without using neverthrow'sfromPromisewrapper. UnlikecreatePlatformNotificationandupdatePlatformNotificationwhich returnResultAsync, these functions will throw errors directly on failure, so thetry/catchblocks correctly capture and handle service-level errors. The catch block will fire as intended.> Likely an incorrect or invalid review comment.
Add notificationId to the structured log context for the archive/delete/publishNow catch handlers so failures are traceable to the offending row.
Devin's PR review flagged three deployment finalize routes (api.v1/v2/v3.deployments.$deploymentId.finalize.ts) that still surfaced raw `error.message` via the template-string variant (`Internal server error: ${error.message}`) on the catch-all 500 branch. Replace with a generic body and route the full error through logger.error for server-side visibility, matching the pattern used in the rest of this PR.
Also fixes the same template-string leak inside the v3 SSE stream's .catch handler, where the raw message would have been written into the SSE event:error data payload.
Update the user-facing copy I introduced in the 5xx sanitisation sweep to include a retry hint ("Something went wrong, please try again." / "Failed to <action> notification, please try again."). Routes that already used "Internal server error" or "Something went wrong" before this PR are left as-is — no copy churn for unaffected routes.
e5f92d5 to
d559275
Compare
When a webapp API route's catch-all 500 branch handles a non-typed exception, it returns the raw
error.messageto the caller. If the exception originates from an internal subsystem (the ORM client, an infra dependency, etc.) the server-side error string is surfaced verbatim in the response body — exposing implementation details the API surface shouldn't carry.The leak shows up in three shapes across the routes:
return json({ error: error.message }, { status: 500 })return json({ error: error instanceof Error ? error.message : "Internal Server Error" }, { status: 500 })return json({ error: `Internal server error: ${error.message}` }, { status: 500 })(plus a couple of analogous neverthrow-Result variants on admin routes.)
Fix
Across 19 webapp routes, replace each leaking branch with a generic body (
"Something went wrong"/"Internal Server Error"to match the file's existing fallback) and addlogger.error(...)so full visibility is preserved server-side. Catch blocks that branch on typed user-input errors (ServiceValidationError,EngineServiceValidationError,OutOfEntitlementError,PrismaClientKnownRequestError) are left intact — those messages are constructed deliberately and intended to be customer-facing.Test plan
pnpm run typecheck --filter webappErrorat the top of the catch'dtryblock (or fake the wrapped call's rejection / Result error), curl the route with the dev API key, confirm the response body changed from the synthetic message verbatim → generic body. 21/21 leak sites verified end-to-end.ServiceValidationErrorfrom inside the catch'd try still surfaces its message at 422 as intended.