fix(baseball): mobile bottom-nav clipped at 320/390px on every coach route - #899
Conversation
…dRejection reporting Every baseball coach route failed e2e mobile-viewport checks with '"Home" [left -2]': FairwayBottomNav's five flex-1 tab columns lacked min-w-0, so truncate never engaged, the row's content minimum exceeded the viewport, and justify-around's negative-free-space fallback centered the overflowing row — pushing the first tab 2px off-screen. Added min-w-0 to each li and Link (the More button already had it, masking the bug for one of five items). Also: unhandledRejection handler now logs the raw reason before synthesizing an Error (name 'UnhandledRejection', reason threaded into Bridge metadata) so Next dev code frames stop blaming instrumentation.ts for every non-Error rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Summary by CodeRabbit
WalkthroughThe pull request updates bottom navigation flex sizing for constrained layouts and expands process-level unhandled rejection diagnostics with preserved non-Error rejection metadata. ChangesBottom navigation sizing
Unhandled rejection diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml 🔧 ESLint
src/components/fairway/app-shell/FairwayBottomNav.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/instrumentation.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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: 1
🤖 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.
Inline comments:
In `@src/instrumentation.ts`:
- Around line 359-368: Update the non-Error handling in the unhandled rejection
handler before Sentry.captureException: safely serialize object reasons with
circular-reference and BigInt handling so the synthetic Error message preserves
distinguishing details, attach the raw rejection reason to Sentry extra context,
and pass a sanitized, serializable metadata value to logProcessErrorToBridge
instead of the raw object. Preserve the existing Error-reason path and
UnhandledRejection naming.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81658d7b-ecd7-4497-b7bb-69a1dbd46472
📒 Files selected for processing (2)
src/components/fairway/app-shell/FairwayBottomNav.tsxsrc/instrumentation.ts
| console.error('[instrumentation] unhandledRejection: non-Error reason', reason); | ||
| error = new Error(`${String(reason)} (unhandled promise rejection with a non-Error reason)`); | ||
| error.name = 'UnhandledRejection'; | ||
| } | ||
| Sentry.captureException(error); | ||
| logProcessErrorToBridge('process.unhandledRejection', error); | ||
| logProcessErrorToBridge( | ||
| 'process.unhandledRejection', | ||
| error, | ||
| reason instanceof Error ? undefined : { reason } | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent Sentry grouping collapse and Bridge logging failures for object rejections.
When reason is an object, String(reason) evaluates to "[object Object]". Because the stack trace for this synthetic Error always points to this handler, Sentry will incorrectly group all non-Error object rejections into a single issue. Additionally, if the raw reason object contains circular references or BigInts (common in HTTP client rejections), JSON.parse(JSON.stringify(...)) inside logServerException's normalizeContext will synchronously throw, completely dropping the Bridge log.
Safely serialize the object for the error message to ensure distinct Sentry grouping, attach the raw object to Sentry's extra context, and ensure the metadata passed to Bridge is stripped of unserializable values.
🛠️ Proposed fix to safely serialize rejection reasons
console.error('[instrumentation] unhandledRejection: non-Error reason', reason);
- error = new Error(`${String(reason)} (unhandled promise rejection with a non-Error reason)`);
+
+ let prefix = String(reason);
+ if (typeof reason === 'object' && reason !== null) {
+ try {
+ prefix = JSON.stringify(reason);
+ } catch {
+ prefix = '[Unserializable Object]';
+ }
+ }
+
+ error = new Error(`${prefix} (unhandled promise rejection with a non-Error reason)`);
error.name = 'UnhandledRejection';
}
- Sentry.captureException(error);
+
+ Sentry.captureException(error, {
+ extra: reason instanceof Error ? undefined : { reason }
+ });
+
+ let safeMetadata: Record<string, unknown> | undefined;
+ if (!(reason instanceof Error)) {
+ try {
+ safeMetadata = { reason: JSON.parse(JSON.stringify(reason)) };
+ } catch {
+ safeMetadata = { reason: String(reason) };
+ }
+ }
+
logProcessErrorToBridge(
'process.unhandledRejection',
error,
- reason instanceof Error ? undefined : { reason }
+ safeMetadata
);📝 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.
| console.error('[instrumentation] unhandledRejection: non-Error reason', reason); | |
| error = new Error(`${String(reason)} (unhandled promise rejection with a non-Error reason)`); | |
| error.name = 'UnhandledRejection'; | |
| } | |
| Sentry.captureException(error); | |
| logProcessErrorToBridge('process.unhandledRejection', error); | |
| logProcessErrorToBridge( | |
| 'process.unhandledRejection', | |
| error, | |
| reason instanceof Error ? undefined : { reason } | |
| ); | |
| console.error('[instrumentation] unhandledRejection: non-Error reason', reason); | |
| let prefix = String(reason); | |
| if (typeof reason === 'object' && reason !== null) { | |
| try { | |
| prefix = JSON.stringify(reason); | |
| } catch { | |
| prefix = '[Unserializable Object]'; | |
| } | |
| } | |
| error = new Error(`${prefix} (unhandled promise rejection with a non-Error reason)`); | |
| error.name = 'UnhandledRejection'; | |
| } | |
| Sentry.captureException(error, { | |
| extra: reason instanceof Error ? undefined : { reason } | |
| }); | |
| let safeMetadata: Record<string, unknown> | undefined; | |
| if (!(reason instanceof Error)) { | |
| try { | |
| safeMetadata = { reason: JSON.parse(JSON.stringify(reason)) }; | |
| } catch { | |
| safeMetadata = { reason: String(reason) }; | |
| } | |
| } | |
| logProcessErrorToBridge( | |
| 'process.unhandledRejection', | |
| error, | |
| safeMetadata | |
| ); |
🤖 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 `@src/instrumentation.ts` around lines 359 - 368, Update the non-Error handling
in the unhandled rejection handler before Sentry.captureException: safely
serialize object reasons with circular-reference and BigInt handling so the
synthetic Error message preserves distinguishing details, attach the raw
rejection reason to Sentry extra context, and pass a sanitized, serializable
metadata value to logProcessErrorToBridge instead of the raw object. Preserve
the existing Error-reason path and UnhandledRejection naming.
|
🤖 Mission Control — PR summary What it changes
Risk / areas
Reviewers should watch
CI: all required checks green — TypeScript, ESLint, unit, Next build, Supabase RLS, Playwright a11y smoke, semgrep/ast-grep, CodeQL. ✅ ( |
1 participant
Found by tonight's cloud E2E sweep: every baseball coach route failed the mobile-viewport fit check with
"Home" [left -2, right 66] vs viewport 320— a standing regression on main (pre-dates #894; the 20:31 and 00:47 main-push runs both failed the same way).Root cause:
FairwayBottomNav's fiveflex-1tab columns have the implicit flexmin-width: autofloor and nomin-w-0, sotruncatenever engages; the row's minimum content width exceeds 320/390px, andjustify-aroundfalls back tocenteron negative free space — shifting the first tab ("Home") 2px past the left viewport edge. One of five items (the "More" button) already carriedmin-w-0, which is how this slipped through.Fix:
min-w-0on each destinationli+Link(mirroring the More button). No structural changes.Also improves the
unhandledRejectionhandler's reporting (raw reason logged before Error synthesis; synthetic errors namedUnhandledRejectionwith the original reason in Bridge metadata) so dev code frames stop pointing at instrumentation.ts.Validation: typecheck + eslint clean; this PR's own Playwright run re-executes the failing
mobile-viewports.spec.tsassertions in CI — that check going green is the proof.🤖 Generated with Claude Code
https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg