test(sentry): pin /invites/validate 400 suppression in fetchWithSentry#2449
test(sentry): pin /invites/validate 400 suppression in fetchWithSentry#2449innolope-dev wants to merge 1 commit into
Conversation
The SKIP_REPORTING entry added in f3d8090 stopped expected 'Invalid Invite' 400s from the setup-flow invite input flooding Sentry (PEANUT-UI-QDR, ~2,850 events / 701 users in 14 days). Nothing pinned that behavior: no test exercised the suppression path in fetchWithSentry at all, so a refactor of the skip rules could silently bring the noise back. Cover the three contract points: /invites/validate 400 emits neither a Sentry event nor a console.warn, a 500 on the same endpoint still reports as an error, and unlisted endpoints still report their 400s.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughExpanded ChangesSentry response reporting
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
Code-analysis diffPainscore total: 6208.86 → 6208.86 (0) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/__tests__/sentry.utils.test.ts (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider restoring
global.fetchinafterEachto avoid cross-suite leakage.
global.fetchis reassigned via direct assignment (global.fetch = jest.fn()...) in each test.jest.clearAllMocks()resets call history but does not restore the originalglobal.fetchimplementation. If otherdescribeblocks in this file (or future tests) run after this suite and rely on the real or a differentfetchmock, they could silently use the last assigned mock. Usingjest.spyOn(global, 'fetch')or saving/restoring the original inafterEachwould make this suite self-contained.This is not causing a failure today (all 23 tests pass), so it's a defensive improvement only.
♻️ Optional: save and restore global.fetch
beforeEach(() => { jest.clearAllMocks() warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}) }) afterEach(() => { warnSpy.mockRestore() + // Restore fetch so subsequent suites get the real implementation + jest.restoreAllMocks() })Alternatively, switch to
jest.spyOn(global, 'fetch')in each test sorestoreAllMockshandles cleanup automatically.Also applies to: 54-54, 65-65
🤖 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/utils/__tests__/sentry.utils.test.ts` at line 41, Restore global.fetch after each test in the affected test suite instead of relying only on jest.clearAllMocks. Save the original fetch before the tests and reassign it in afterEach, or replace direct assignments with jest.spyOn(global, 'fetch') and restore the spy, ensuring no mock leaks into other describe blocks.
🤖 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 `@src/utils/__tests__/sentry.utils.test.ts`:
- Line 41: Restore global.fetch after each test in the affected test suite
instead of relying only on jest.clearAllMocks. Save the original fetch before
the tests and reassign it in afterEach, or replace direct assignments with
jest.spyOn(global, 'fetch') and restore the spy, ensuring no mock leaks into
other describe blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83f8170a-10a4-4040-bd6f-bd105df22713
📒 Files selected for processing (1)
src/utils/__tests__/sentry.utils.test.ts
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
1 participant
Problem
Sentry issue PEANUT-UI-QDR —
POST to https://api.peanut.me/invites/validate failed with status 400— accumulated 2,850 events from 701 users in 14 days, culprit page/setup.Root cause
The 400 is an expected user-input outcome, not a bug:
/setup,JoinWaitlistdebounce-validates the invite-code input (750ms, min-length gated) againstPOST /invites/validate.peanut-api-ts/src/routes/invite.ts) returns400 Invalid Invitewhenever the typed code doesn't resolve to an existing inviter with app access — i.e. any typo'd or unknown username. The UI already handles this gracefully with an inline "inviter not found" error.fetchWithSentryreported every non-OK response to Sentry, so each invalid attempt became an event (~4 events/user matches debounced retyping)./setup.The suppression fix already landed: f3d8090 (July 8) added
/invites/validate400 toSKIP_REPORTINGonmain, and it reachedfeat/mobile-releaseon July 9 — which is why the issue is tapering. The residual trickle is stale clients (old native bundles pre-OTA / cached web sessions) and will die off on its own.Gap found: nothing pinned that fix.
sentry.utils.test.tsnever exercised theshouldSkipReportingpath offetchWithSentryat all, so a refactor of the skip rules could silently resurrect the noise.Fix
Regression tests for the suppression contract in
fetchWithSentry:/invites/validate400 → noSentry.captureMessage, noconsole.warn(which would otherwise become a breadcrumb via forward-logs)/invites/validate500 → still reported aterrorlevel (real server failures stay visible)warninglevelNo production code change needed — the suppression is correct and shipped.
Impact
invite_code_validatedfor funnel analytics.Testing
pnpm jest src/utils/__tests__/sentry.utils.test.ts— 23 passed (3 new + 20 existing). File formatted with prettier.Summary by CodeRabbit