Fix Sentry server instrumentation and error page UI#524
Merged
Conversation
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR fixes server-side Sentry error capture and error page clipboard functionality. It removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
fdm-app/app/components/custom/error.tsx (2)
57-72: Good fix for the clipboard permission issue.The async/await pattern correctly addresses the original bug where the unhandled Promise caused false success indication. The fallback text selection using Range/Selection APIs is a solid UX recovery path.
One minor consideration: the catch block swallows the error silently. For debugging in production, you may want to log it.
💡 Optional: Add error logging for debugging
} catch { + console.warn("Clipboard write failed, falling back to text selection") // Fallback: select the text in the pre element so the user can copy manually🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/custom/error.tsx` around lines 57 - 72, The catch block in copyStackTrace currently swallows the thrown error; update the catch to accept the error parameter (e.g., catch (err)) and log the error (e.g., console.error or your app logger) along with context before running the fallback selection and setCopyState("failed") so failures are recorded for debugging; reference the copyStackTrace function, the errorDetails variable, and setCopyState to locate where to add the error logging.
124-128: Consider the button width with the long failure message.The failure message "Browser staat niet toe om te kopiëren — selecteer de tekst hieronder en kopieer handmatig" is quite long and may cause layout issues on smaller screens or overflow the button container.
Consider either:
- Shortening the message (e.g., "Kopiëren geblokkeerd — selecteer handmatig")
- Moving the detailed instruction to a tooltip or separate text element below the button
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/custom/error.tsx` around lines 124 - 128, The long failure label rendered in the JSX ternary for copyState causes layout/overflow issues; update the copyState === "failed" branch in the component (the ternary that currently returns "Browser staat niet toe om te kopiëren — selecteer de tekst hieronder en kopieer handmatig") to use a shorter button label (e.g., "Kopiëren geblokkeerd — selecteer handmatig") and move the full instruction into a separate element or tooltip adjacent to the button (so keep the ternary logic on copyState, but render detailed guidance in a small <span> or tooltip component under/next to the button).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@fdm-app/app/components/custom/error.tsx`:
- Around line 57-72: The catch block in copyStackTrace currently swallows the
thrown error; update the catch to accept the error parameter (e.g., catch (err))
and log the error (e.g., console.error or your app logger) along with context
before running the fallback selection and setCopyState("failed") so failures are
recorded for debugging; reference the copyStackTrace function, the errorDetails
variable, and setCopyState to locate where to add the error logging.
- Around line 124-128: The long failure label rendered in the JSX ternary for
copyState causes layout/overflow issues; update the copyState === "failed"
branch in the component (the ternary that currently returns "Browser staat niet
toe om te kopiëren — selecteer de tekst hieronder en kopieer handmatig") to use
a shorter button label (e.g., "Kopiëren geblokkeerd — selecteer handmatig") and
move the full instruction into a separate element or tooltip adjacent to the
button (so keep the ternary logic on copyState, but render detailed guidance in
a small <span> or tooltip component under/next to the button).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 23a5f8d7-5675-4d17-a0b3-e20bf0430968
📒 Files selected for processing (8)
.changeset/nice-views-carry.mdfdm-app/app/components/custom/error.tsxfdm-app/app/entry.client.tsxfdm-app/app/entry.server.tsxfdm-app/app/root.tsxfdm-app/app/routes/sentry-tunnel.tsxfdm-app/instrument.server.mjsfdm-app/package.json
💤 Files with no reviewable changes (2)
- fdm-app/app/entry.client.tsx
- fdm-app/app/routes/sentry-tunnel.tsx
This was referenced Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Chores
closes #523