Skip to content

feat(server): add beforeSend to server-side track - #208

Open
AaronBuxbaum wants to merge 1 commit into
vercel:mainfrom
AaronBuxbaum:server-track-before-send
Open

feat(server): add beforeSend to server-side track#208
AaronBuxbaum wants to merge 1 commit into
vercel:mainfrom
AaronBuxbaum:server-track-before-send

Conversation

@AaronBuxbaum

Copy link
Copy Markdown

The server SDK resolves the event's page URL itself — from the Vercel request context, falling back to the Referer header — so a caller has no way to edit or withhold it. Apps whose URLs are themselves credentials (a one-time token in the path: signing links, password resets, magic links) therefore report a live secret with every custom event, and no amount of care at the call site can prevent it.

This PR replicates the client SDK's existing beforeSend contract as a track option. It receives { type: 'event', url } and returns the event to send, or null to drop it.

It is deliberately fail-closed: the hook runs before the request body is built, so returning null sends nothing at all, and a hook that throws is caught by the existing handler rather than falling through to the un-edited URL. The missing-headers check moves above the hook so the hook is not invoked for an event that was never going to be sent.

BeforeSend and BeforeSendEvent are re-exported from @vercel/analytics/server so the hook can be typed without reaching into the root entry point.

The server SDK resolves the event's page URL itself — from the Vercel
request context, falling back to the `Referer` header — so a caller has
no way to edit or withhold it. Apps whose URLs are themselves
credentials (a one-time token in the path: signing links, password
resets, magic links) therefore report a live secret with every custom
event, and no amount of care at the call site can prevent it.

Accept the client SDK's existing `beforeSend` contract as a `track`
option. It receives `{ type: 'event', url }` and returns the event to
send, or `null` to drop it.

It is deliberately fail-closed: the hook runs before the request body is
built, so returning `null` sends nothing at all, and a hook that throws
is caught by the existing handler rather than falling through to the
un-edited URL. The missing-headers check moves above the hook so the
hook is not invoked for an event that was never going to be sent.

`BeforeSend` and `BeforeSendEvent` are re-exported from
`@vercel/analytics/server` so the hook can be typed without reaching
into the root entry point.
@vercel

vercel Bot commented Aug 15, 2026

Copy link
Copy Markdown

@AaronBuxbaum is attempting to deploy a commit to the Analytics Test Projects Team on Vercel.

A member of the Team first needs to authorize it.

AaronBuxbaum added a commit to AaronBuxbaum/diveday that referenced this pull request Aug 15, 2026
The contract test asserted the server SDK still offers no way for a caller
to supply the event's page URL, by matching an `options.url` override. The
request actually went upstream as vercel/analytics#208, which adds a
`beforeSend` hook mirroring the browser SDK's -- source reading
`options?.beforeSend` and `event.url`, matching neither that regex nor
anything else the test looked for.

So the tripwire would have stayed green through exactly the API it was
watching for, and the shim would have outlived its own replacement without
anyone being told. Assert on `beforeSend` by name as well, so bumping the
dependency past the release that carries it is a red test rather than
something to remember.

Records the upstream PR on the follow-up entry, which until now only said
to open one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AaronBuxbaum added a commit to AaronBuxbaum/diveday that referenced this pull request Aug 15, 2026
The neighbouring assertion pinned the page URL to the `o:` property it was
written inline against:

  /o:\s*\(?\s*requestContext[\s\S]{0,60}?\.url/

vercel/analytics#208 hoists that same expression into a `pageUrl` local so a
`beforeSend` hook can edit it before the body is built, leaving `o: pageUrl`.
The SDK reads `requestContext.url` exactly as before and the shim keeps
working -- but pinned to the old shape this test goes red on that release
claiming the redaction contract broke. False, and the opposite of the actual
news.

Match the derivation instead. The arrival of a supported API is the
`beforeSend` assertion's job to report, and only its job, so the two
conditions now fail independently and say what they mean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AaronBuxbaum added a commit to AaronBuxbaum/diveday that referenced this pull request Aug 15, 2026
…552)

Follow-on to #550, prompted by the upstream PR actually being opened:
vercel/analytics#208

## The gap

`analytics-request-context.test.ts` carries a tripwire asserting the
server SDK still gives a caller no way to supply the event's page URL —
the condition that justifies the global shim existing at all:

```js
expect(sdk).not.toMatch(/options\s*(\?\.)?\s*\.\s*url\b/);
```

That matches an `options.url` override. But the request went upstream as
**vercel/analytics#208**, which adds a `beforeSend` hook mirroring the
browser SDK's — source reading `options?.beforeSend` and `event.url`,
matching neither that regex nor anything else the test looked for.

So the tripwire would have stayed green through **exactly the API it was
watching for**. We'd have bumped the dependency past the release
carrying `beforeSend` and kept the shim — an undocumented hook into a
Vercel internal — outliving its own supported replacement, with nothing
to say so. The test's own comment said "(or a `beforeSend`)"; the
assertion never covered it.

## The change

Assert on `beforeSend` by name as well, so the bump becomes a red test
rather than something to remember. The current release has zero
occurrences in that bundle, so it passes today and fires on the release
that lands #208.

Also records the upstream PR on the follow-up entry, which until now
only said to open one — and notes that `beforeSend` is a strictly better
fit than the `options.url` override the entry originally proposed, since
it can withhold an event entirely, which is what our `trackEvent`
already does when it cannot redact.

## Owned paths

- `src/lib/analytics-request-context.test.ts`
- `docs/product/follow-ups/FU-20260814-vercel-analytics-url-override.md`

No production code changes. Lint, typecheck, `check:repo`,
`check:follow-ups`, and both analytics specs green (26/26).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

## Summary by Sourcery

Tighten the analytics server SDK tripwire so dependency bumps fail as
soon as the upstream `beforeSend` hook that can replace our shim is
introduced, and update the associated follow-up documentation to
reference the landed upstream PR and its preferred API shape.

Documentation:
- Update the follow-up document to record the upstream vercel/analytics
pull request adding a `beforeSend` hook and clarify that this hook is
the preferred replacement for the current shim.

Tests:
- Extend the analytics request context tripwire test to assert that the
server bundle does not include a `beforeSend` hook, ensuring the test
fails once the upstream replacement API ships.
AaronBuxbaum added a commit to AaronBuxbaum/diveday that referenced this pull request Aug 15, 2026
One conflict, in AGENTS.md, where two independent edits landed on adjacent
rows of the commands table. Both sides kept, each for its own row:

- `scripts/stray-processes.mjs` takes main's row. #553 taught the reporter two
  more events (`SubagentStop` reports, `SessionEnd` reaps), which this branch
  predates and does not touch.
- `pnpm gates` keeps this branch's row, which ages both rooms of the follow-up
  register rather than one.

Everything else auto-merged, including
docs/product/follow-ups/FU-20260814-vercel-analytics-url-override.md, which
this branch moved into `waiting/` while main edited its body -- git tracked the
rename and took both. That edit made this branch's `**Waiting on:**` line stale
(it still described opening the upstream request as pending, where main records
it as done: vercel/analytics#208), so the line is rewritten to wait only on the
release, and to note that the contract test goes red on the dependency bump
rather than anyone having to remember.

Verified on the merged tree: `pnpm check` green, 5,737 tests, 25 safeguards.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant