feat(analytics): add Speed Insights, URL scrubbing, and dose_logged event#87
feat(analytics): add Speed Insights, URL scrubbing, and dose_logged event#87JWhite212 wants to merge 4 commits into
Conversation
…vent
- Install @vercel/speed-insights and inject from src/routes/+layout.ts
to surface Core Web Vitals (LCP/CLS/INP) in the Vercel dashboard.
- Extend the existing Web Analytics injection with a beforeSend that
strips query strings from /log so the free-text search term (?q=)
never reaches the recorded URL.
- Add a server-side track("dose_logged") call on the dashboard quick-log
action. Only safe metadata is sent (source, hasNotes, hasSideEffects);
no medication id/name, notes content, or side-effect strings. Wrapped
in try/catch so a Vercel beacon failure cannot break a dose log.
No CSP changes required: Vercel insights are served same-origin via
/_vercel/insights/* and /_vercel/speed-insights/*.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR integrates Vercel Analytics and Speed Insights into a medication tracking application. The root layout now configures analytics with URL sanitisation that strips query parameters from /log page events, and injects Speed Insights. The dashboard server page instruments dose-logging events by tracking safe metadata (notes presence, side effects presence, and source) without exposing personally identifiable information. ChangesVercel Analytics and Speed Insights
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🧹 Nitpick comments (1)
src/routes/+layout.ts (1)
7-17: ⚡ Quick winAdd error handling for URL parsing.
The
new URL(event.url)call on line 12 could throw aTypeErrorifevent.urlis malformed. Although the library likely provides valid URLs, defensive error handling would prevent thebeforeSendhook from breaking if unexpected data is received.🛡️ Proposed fix to add error handling
beforeSend(event: BeforeSendEvent) { // The /log page accepts a free-text search (`?q=...`) that can contain // medication names. Strip the entire query string so it never reaches // the recorded URL. - if (event.url.includes("/log")) { + try { const u = new URL(event.url); - u.search = ""; - return { ...event, url: u.toString() }; + if (u.pathname === "/log" || u.pathname.startsWith("/log/")) { + u.search = ""; + return { ...event, url: u.toString() }; + } + } catch { + // If URL parsing fails, return event unchanged + return event; } return event; },🤖 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/routes/`+layout.ts around lines 7 - 17, Wrap the URL parsing in beforeSend in a try/catch: when calling new URL(event.url) (inside beforeSend) catch any TypeError/exception, log a short warning (including the thrown error and event.url) and return the original event unchanged; only when parsing succeeds continue to set u.search = "" and return the modified event.
🤖 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/routes/`(app)/dashboard/+page.server.ts:
- Around line 113-123: The telemetry call is meant to be fire-and-forget but
currently uses await on track("dose_logged") inside the try/catch, which blocks;
change the code to call track("dose_logged", { ... }) without awaiting (e.g.,
call it and optionally append .catch(() => {}) to swallow errors) so the dose
logging path doesn't wait for analytics; update the call site referencing
track("dose_logged") in the same try block and keep the existing non-PII payload
(source, hasNotes, hasSideEffects).
In `@src/routes/`+layout.ts:
- Around line 11-14: The current includes check is too broad; instead parse the
URL (const u = new URL(event.url)) and only clear query params when the pathname
is exactly the /log page (e.g., check u.pathname === '/log' or u.pathname ===
'/log/'), then set u.search = '' and return the modified event; keep references
to event.url, new URL(event.url), u.pathname and u.search when updating the
code.
---
Nitpick comments:
In `@src/routes/`+layout.ts:
- Around line 7-17: Wrap the URL parsing in beforeSend in a try/catch: when
calling new URL(event.url) (inside beforeSend) catch any TypeError/exception,
log a short warning (including the thrown error and event.url) and return the
original event unchanged; only when parsing succeeds continue to set u.search =
"" and return the modified event.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc7934b3-993f-4aed-bec0-d12cbc5d7881
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.jsonsrc/routes/(app)/dashboard/+page.server.tssrc/routes/+layout.ts
The previous beforeSend used `event.url.includes("/log")`, which is a
substring match — it would also strip query strings from `/auth/login`,
`/auth/logout`, and any future route containing `/log`. Parse the URL
and compare `pathname` exactly to `/log` (with trailing-slash tolerance)
instead.
Wrap the URL parsing in a try/catch so a malformed `event.url` logs a
warning and lets analytics continue with the event unchanged.
Signed-off-by: Jamie White <106563693+JWhite212@users.noreply.github.com>
Signed-off-by: Jamie White <106563693+JWhite212@users.noreply.github.com>
1 participant
Summary
@vercel/speed-insightsand inject fromsrc/routes/+layout.tsso Core Web Vitals (LCP / CLS / INP) show up in the Vercel dashboard alongside Web Analytics.beforeSendthat strips query strings from/logURLs — the free-text search input (?q=...) can carry medication names, so it never reaches the recorded URL.track("dose_logged")call on the dashboard quick-log action. Only safe metadata is sent (source,hasNotes,hasSideEffects); no medication id/name, notes content, or side-effect strings. Wrapped intry/catchso a Vercel beacon failure can never break a dose log.Privacy posture
/logquery strings scrubbed at the source./medications/[id]is a CUID (not a name) — safe to leave.No CSP changes required
Vercel insights scripts are served same-origin via
/_vercel/insights/*and/_vercel/speed-insights/*, which the existingscript-src 'self'/connect-src 'self'already allow.Follow-ups (not in this PR)
Same
track()pattern can be applied to:medication_created/medication_archived(medications/new, medications/[id])export_pdf/export_csvauth_signup/totp_enrolledpush_subscribedinventory_adjustedTest plan
npx vitest run— 363/363 passnpm run check— 0 errors (25 pre-existing warnings)npm run build— succeedsdose_loggedevent appears in Vercel Analytics → Events/log?q=testis recorded as/log(no query string)