nightly: 1 production fix (2026-06-14) - #294
Conversation
…rors/day)
100% of production runtime errors in the last 24h were the same pattern:
POST /golf/admin 500 "Error: Forbidden" firing every ~5 minutes, two per
tick (one each from getAdminDashboardData and getAdminDashboardRollup —
the dashboard's visibility-aware refresh polls both in parallel). Both
actions throw bare `Error('Forbidden')` on the role check, which surfaces
as a Vercel runtime error and floods the runtime log faster than real
issues can show up.
Two failure paths fed this:
1. A tab kept open through a session/role change still polls every 5 min.
The SSR layout's redirect only fires on a fresh navigation; the data
actions then 500 on every subsequent tick.
2. The client guard that's supposed to freeze polling matched the error
with exact equality (`message === 'Forbidden'`). The inner rollup
helper re-throws with a `rollupA failed: Forbidden` prefix, so a real
admin hitting an internal RLS denial would slip past the guard and
poll forever.
Fix is two small, targeted changes:
- New `checkAdminAccess()` server action: non-throwing access probe.
Returns `{ allowed, reason }` so the auth-fail path no longer 500s on
its own — keeps prod logs clean. `loadData` now gates on this before
the heavy data actions; on `{ allowed: false }` it short-circuits and
trips the session-expired path, tearing down the polling interval.
- Harden the catch-block guard to match wrapped messages via
`/\b(Unauthorized|Forbidden)\b/`. Restores the freeze-on-denial
behaviour for the wrapped rollup case.
Existing server-side throws stay as defense-in-depth. SSR layout guard
is unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 6 minutes and 39 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…on expiry Greptile P1 on PR #294: checkAdminAccess() was treating any error from the `users` role lookup the same as `role !== 'admin'` — both returned { allowed: false, reason: 'forbidden' }, which the client maps to `throw new Error('Forbidden')`, hits the /\bForbidden\b/ regex, and permanently sets sessionExpired=true. A real admin whose role query hit a transient Supabase hiccup would see "Session Expired" and have to reload, even though their session is fine. Split the cases: re-throw the raw userErr so the generic catch path surfaces it as a retriable error (timer keeps running), and only return `forbidden` when the role is actually wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Folded into #304 (feat/coachhelm-stats-roundup) — merged clean, combined gates green. Branch intact + reopenable. |
2 participants
Daily brief — 2026-06-14
24h totals
Host not in allowlist: mcp.supabase.com) — admin_events, error_logs, and pg_stat_statements were not queried.Top patterns (by occurrences × severity)
Error: Forbiddenthrown from two server actions (getAdminDashboardData,getAdminDashboardRollup)Error('Forbidden')on the role check, returning a 500 per request (×2 per tick). The SSR layout guard catches a fresh navigation, but a tab kept open across a session/role change still polls — and the inner rollup helper re-throws with arollupA failed: Forbiddenwrapper that the client'smessage === 'Forbidden'guard missed entirely, so a real admin hitting an internal RLS denial would never freeze the timer. Result: one tab → ~576 runtime errors/day.Performance
pg_stat_statementsnot pulled. No slow-query report this run.Deployment health
READY, 2 currentlyBUILDING(PR fix(golf): premium course-picker redesign — cards, header, motion #293, branchfix/picker-premium-redesign, SHAs61c5380+ab51d44). ZeroErrorstate in the window.dpl_HsvUDsTr62ZFQZqotbAZYeoPH1Jy(SHA0984669— "fix(golf): full-page picker, 3 carousels…", PR fix(golf): full-page picker, 3 carousels (Recently played / Team / Library), snappier motion #292 squash-merged).What I couldn't fully verify
admin_events,error_logs,pg_stat_statements) — MCP host blocked by env network policy. Run separately to confirm there isn't a second pattern hiding only inerror_logs(the app'slogServerError()sink writes there, not to Vercel).Fixes applied
1.
/golf/adminForbidden flood (576+ errors/day → expected 0)Pattern:
POST /golf/admin → 500 → Error: Forbidden at U/at ahFiles touched:
src/app/golf/actions/admin-data.ts— newcheckAdminAccess()server action (non-throwing access probe).src/app/golf/admin/page.tsx—loadDatanow callscheckAdminAccess()first and short-circuits cleanly when access drops; error-guard regex widened to/\b(Unauthorized|Forbidden)\b/so wrapped messages (rollupA failed: Forbidden) also freeze polling.Diagnosis: Two failure modes fed the flood:
message === 'Forbidden'(strict equality). The inner rollup helper re-throws with arollupA failed:prefix, slipping past the guard — a real admin hitting an internal RLS denial would poll forever.Approach: Added a non-throwing
checkAdminAccess()that returns{ allowed, reason }. The page calls it before the heavy data actions; on{ allowed: false }the catch path tripssessionExpired=true, which feedsnulltouseVisibilityAwareIntervaland tears the timer down. No 500. Existing throws stay as backstops on the actions themselves. Guard regex widened to match wrapped messages with the same intent.Not fixed (and why):
/api/cron/*log entry was an informational warning from the coachhelm-roster-sweep ([insights.triggerPlayerInsi…) with status 200. Not actionable.DO NOT auto-merge — leaving for human review.
Generated by Claude Code
Greptile Summary
This PR fixes a 576+ errors/day flood on
POST /golf/admincaused by the admin dashboard's 5-minute visibility-aware polling continuing to call auth-gating server actions after a tab's session or role had dropped. A second bug let wrapped error messages likerollupA failed: Forbiddenslip past the strict equality guard, keeping the timer alive after a real RLS denial.checkAdminAccess()server action that returns{ allowed, reason }— the page calls it first on each poll tick, short-circuits on{ allowed: false }, and never reaches the 500-throwing data actions. Transient DB errors are re-thrown (not collapsed toforbidden) so a Supabase hiccup leaves the timer running.=== 'Forbidden'to/\b(Unauthorized|Forbidden)\b/so wrapped messages from inner rollup helpers also stop the timer.Confidence Score: 5/5
The change is safe to merge — it eliminates a concrete 576+/day 500 flood by gating polls behind a non-throwing access probe and correctly preserves the polling timer on transient DB errors.
Both changes are tightly scoped: the new server action returns a plain object rather than throwing, the DB-error re-throw keeps the timer alive during Supabase hiccups, and the regex widening only catches messages that already semantically mean auth denial. No new error paths are introduced and the prior review concern about DB errors collapsing to forbidden has been addressed correctly.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Browser as Browser Tab participant SA_Check as checkAdminAccess() [server] participant SA_Data as getAdminDashboardData() [server] participant SA_Rollup as getAdminDashboardRollup() [server] participant DB as Supabase DB Note over Browser: Every 5 min (visibility-aware) Browser->>SA_Check: call (access probe) SA_Check->>DB: auth.getUser() SA_Check->>DB: "SELECT role FROM users WHERE id=?" alt "user not found or role != admin" SA_Check-->>Browser: allowed: false Browser->>Browser: throw Error Unauthorized or Forbidden Browser->>Browser: setSessionExpired true Browser->>Browser: timer torn down, no more polls, no 500s else transient DB error SA_Check-->>Browser: throws DB error Browser->>Browser: setError message, timer stays alive else allowed true SA_Check-->>Browser: allowed: true Browser->>SA_Data: call Browser->>SA_Rollup: call with catch null SA_Data-->>Browser: data SA_Rollup-->>Browser: rollup or null Browser->>Browser: setData setRollup setLastRefresh endReviews (2): Last reviewed commit: "fix(admin): don't collapse transient DB ..." | Re-trigger Greptile