fix: non-blocking startup + safe JSON parsing in client hooks#324
fix: non-blocking startup + safe JSON parsing in client hooks#324
Conversation
…ooks - Make welcome campaign bootstrap fire-and-forget so it no longer blocks server startup for up to 15s; reduce timeout from 15s to 5s (fixes #321) - Add .catch() to all success-path res.json() calls in client hooks to prevent unhandled SyntaxError when proxies return non-JSON 200s (fixes #320) https://claude.ai/code/session_01CXUwxHoN9LHLXgNFMBU6gL
…edundant DB call - Add .catch() to use-campaigns.ts fetchJson() success path (skeptic finding) - Add trailing .catch() to welcome campaign IIFE for consistency - Wrap ErrorLogger import in nested try/catch to prevent unhandled rejection - Return campaignConfigsReady from registerRoutes() to avoid redundant ensureAutomatedCampaignConfigsTable() call during startup - Add 18 "handles non-JSON success responses" tests across 7 hook files https://claude.ai/code/session_01CXUwxHoN9LHLXgNFMBU6gL
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds JSON parsing error handling across 12 client hooks by wrapping Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested labels
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 35.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 two main fixes: non-blocking server startup and safe JSON parsing in client hooks, matching the core objectives. |
| Linked Issues check | ✅ Passed | All coding objectives from #321 and #320 are comprehensively addressed: server bootstrap is non-blocking with reduced timeout, campaignConfigsReady is returned from registerRoutes, and all 19 affected res.json() calls are wrapped with .catch() handlers. |
| Out of Scope Changes check | ✅ Passed | All changes are directly scoped to fixing #321 and #320. The test environment switch (happy-dom to jsdom) and 18 new tests support validation of the fixes; no extraneous refactoring or unrelated changes detected. |
✏️ 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
claude/bugfix-issues-v4SBv
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 help to get the list of available commands and usage tips.
Attach a no-op .catch() to the bootstrapWelcomeCampaign() promise so that if the 5s timeout wins the Promise.race and the campaign later fails, the rejection is handled rather than becoming an unhandled promise rejection. https://claude.ai/code/session_01CXUwxHoN9LHLXgNFMBU6gL
Summary
Fixes two bugs: the welcome campaign bootstrap blocking server startup for up to 15s (#321), and unguarded
res.json()success-path calls in client hooks throwing unhelpfulSyntaxErrorwhen proxies return non-JSON 200 responses (#320).Changes
Server startup (#321)
campaignConfigsReadyfromregisterRoutes()to avoid redundantensureAutomatedCampaignConfigsTable()DB callErrorLoggerdynamic import in nested try/catch to prevent unhandled rejection.catch()on IIFE for consistency with scheduler patternClient hooks (#320)
.catch(() => { throw new Error("Unexpected response format from server"); })to 19 success-pathres.json()calls across 8 hook files:use-monitors.ts(5 calls)use-tags.ts(1 call)use-notification-channels.ts(4 calls)use-conditions.ts(2 calls)use-api-keys.ts(2 calls)use-slack.ts(2 calls)use-notification-preferences.ts(2 calls)use-campaigns.ts(1 call —fetchJsonutility)Tests
happy-domtojsdomHow to test
text/plain200 from MSW produces"Unexpected response format from server"error instead of a rawSyntaxErrornpm run check && npm run test— all 2075 tests passnpm run build— production build succeedsRelated issues
use-auth.tssame vulnerability, out of scope)https://claude.ai/code/session_01CXUwxHoN9LHLXgNFMBU6gL
Summary by CodeRabbit
Release Notes
Bug Fixes
Chores