fix: error logging so server errors are actually captured in Sentry

[SvenVw]

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of server-side error reporting and exception tracking
  • Enhanced error tracking with unique identifiers for easier troubleshooting

• Chores

  • Refactored error handling for consistency across application layers
  • Version bumped to 0.28.4

Closes #521

[github-actions]

👋 Hotfix Branch PR Detected!

Before merging this Pull Request into main, please ensure you have finalized the hotfix by manually running the 'Release' workflow on this hotfix/FDM521 branch.

This will:

1. Bump package versions.
2. Generate changelogs.
3. Create Git tags.

You can trigger the workflow from the 'Actions' tab, selecting the 'Release' workflow, and choosing this hotfix/FDM521 branch.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9305bd31-181d-4c75-b804-fa5906645444

📥 Commits

Reviewing files that changed from the base of the PR and between 5fa7662 and ada7694.

📒 Files selected for processing (2)

• fdm-app/CHANGELOG.md
• fdm-app/package.json

✅ Files skipped from review due to trivial changes (2)

• fdm-app/package.json
• fdm-app/CHANGELOG.md

---

📝 Walkthrough

Walkthrough

The PR improves Sentry error logging and configuration consistency by renaming environment variables from VITE_SENTRY_* to PUBLIC_SENTRY_*, making server-side Sentry initialization conditional, centralizing error reporting through a unified reportError() helper, adding searchable error_id tags to Sentry events, and filtering duplicate client-side error events.

Changes

Cohort / File(s)	Summary
Environment variable standardization docker-compose.yml, instrument.server.mjs	Renamed all Sentry-related env vars from VITE_SENTRY_* to PUBLIC_SENTRY_* and made server-side Sentry initialization conditional on PUBLIC_SENTRY_DSN presence with sample rate defaults.
Error reporting consolidation app/entry.server.tsx, app/lib/error.ts	Unified error handling by replacing direct Sentry.captureException() calls and scattered console.error() logging with centralized reportError() function, ensuring consistent errorId tracking across streaming and unhandled errors.
Client-side error filtering app/entry.client.tsx	Expanded Sentry ignoreErrors configuration to filter the duplicate "Unexpected Server Error" React Router shadow event.
Error reporting infrastructure app/lib/error.ts	Removed clientConfig-based Sentry gating, replaced with Sentry.getClient() check; added error_id to searchable Sentry tags; ensured console.error() always logs regardless of Sentry configuration.
Release metadata CHANGELOG.md, package.json	Documented error logging improvements and bumped version from 0.28.3 to 0.28.4.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Hotfix/20260312 #513 — Both PRs modify Sentry configuration and environment variable naming (PUBLIC_SENTRY_DSN setup), directly addressing initialization consistency.
• Hotfix/environmentals runtime #149 — Both PRs standardize env var naming convention by renaming VITE_* to PUBLIC_* for public-facing Sentry configuration.
• Add loading indicator and Sentry metrics for page load #488 — Both PRs modify Sentry initialization logic and error suppression rules in instrument.server.mjs and client initialization.

Suggested labels

fdm-app, bug, branch:main

Suggested reviewers

• gerardhros
• BoraIneviNMI

Poem

🐰 Errors now leave a searchable trace,
No more errorId lost in space!
Tags, not shadows, light the way—
Sentry finds bugs without delay! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: improving server error capture in Sentry through refactored error reporting.
Linked Issues check	✅ Passed	All requirements from issue #521 are implemented: environment variables renamed [docker-compose.yml, instrument.server.mjs], Sentry initialization made conditional [instrument.server.mjs], reportError uses Sentry.getClient() without clientConfig gate [app/lib/error.ts], error_id added to searchable tags [app/lib/error.ts], console.error always called [app/lib/error.ts], shadow event suppressed [app/entry.client.tsx], and streaming errors captured [app/entry.server.tsx].
Out of Scope Changes check	✅ Passed	All changes are within scope of issue #521: environment variable standardization, Sentry SDK initialization fixes, error reporting consolidation, and version bumps align with the error logging improvements.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hotfix/FDM521

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sentry]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

docker-compose.yml (1)

26-33: Consider leaving the sample Sentry vars unset by default.

fdm-app/instrument.server.mjs now treats any non-empty PUBLIC_SENTRY_DSN as configured, so YOUR_SENTRY_DSN keeps the compose template on the Sentry-enabled path instead of exercising the no-Sentry startup path this PR is trying to preserve. Commenting these out until real values are available would make the optional-Sentry behavior easier to validate.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 26 - 33, The docker-compose Sentry env vars
(notably PUBLIC_SENTRY_DSN) are populated with placeholder strings which
instrument.server.mjs treats as "configured"; remove or unset those placeholders
so the app can exercise the no‑Sentry startup path. Update the
docker-compose.yml by commenting out or setting to an empty value the
PUBLIC_SENTRY_DSN, PUBLIC_SENTRY_ORG, PUBLIC_SENTRY_PROJECT and
SENTRY_AUTH_TOKEN entries (you can leave sampling flags if desired), so
instrument.server.mjs will detect no Sentry configuration and follow the
optional‑Sentry behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@fdm-app/app/lib/error.ts`:
- Around line 134-137: The 5xx responses currently skip the shared reporting
path and only console.warn/return a generic message; modify the loader/action
error handling so that any error with status >= 500 is passed to reportError()
(same signature used later where errorId is created with reportError(error, {
scope: "loader" })) before constructing the response/toast, and use the returned
errorId in the response body/toast; apply the same change to the other
occurrence around the second block (the similar fallback at lines ~276-279) so
all server-side 5xx errors use consistent reporting and errorId generation.

---

Nitpick comments:
In `@docker-compose.yml`:
- Around line 26-33: The docker-compose Sentry env vars (notably
PUBLIC_SENTRY_DSN) are populated with placeholder strings which
instrument.server.mjs treats as "configured"; remove or unset those placeholders
so the app can exercise the no‑Sentry startup path. Update the
docker-compose.yml by commenting out or setting to an empty value the
PUBLIC_SENTRY_DSN, PUBLIC_SENTRY_ORG, PUBLIC_SENTRY_PROJECT and
SENTRY_AUTH_TOKEN entries (you can leave sampling flags if desired), so
instrument.server.mjs will detect no Sentry configuration and follow the
optional‑Sentry behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0ed8c701-b3a9-4c0d-8397-9d95e5ce038c

📥 Commits

Reviewing files that changed from the base of the PR and between 6f7df00 and 931b2a6.

📒 Files selected for processing (6)

• .changeset/fix-error-logging.md
• docker-compose.yml
• fdm-app/app/entry.client.tsx
• fdm-app/app/entry.server.tsx
• fdm-app/app/lib/error.ts
• fdm-app/instrument.server.mjs

[SvenVw]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.