Pass the webhook request through without buffering its body - #3166
Conversation
🦋 Changeset detectedLatest commit: 8ff9026 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📊 Workflow Benchmarkscommit Backend:
📜 Previous results (2)acf61a3Tue, 28 Jul 2026 21:20:47 GMT · run logs
d8a70f9Tue, 28 Jul 2026 20:28:48 GMT · run logs
ℹ️ Metric definitions & methodologyBest/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests▲ Vercel Production (1 failed)fastify (1 failed):
💻 Local Development (1 failed)nuxt-stable (1 failed):
E2E Test SummarySummary
Details by Category❌ ▲ Vercel Production
❌ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
d8a70f9 to
ef2f142
Compare
TooTallNate
changed the title
|
No backport to This removes an unnecessary To override, re-run the Backport to stable workflow manually via |
|
Backport PR opened against |
Problem
The Astro and SvelteKit webhook wrappers copied the incoming request via
normalizeRequest()— which reads the whole body witharrayBuffer()— before calling the handler. That read happened before the handler had a chance to look at the token, so requests carrying an unknown token did unnecessary work before being rejected.Fix
The copy turns out to be unnecessary. Both frameworks already hand the route a standard
Request, so the wrappers now pass it straight through:The body is left untouched until
resumeWebhook()has accepted the token. This deletes code rather than adding a second normalizer — an earlier revision of this PR added a streaming variant ofnormalizeRequest, but with the body read removed that wrapper was just an identity copy of the request, so it's gone.The flow route keeps
normalizeRequest(): it authenticates via the queue trigger rather than a URL token, so the ordering doesn't matter there. Whether the shim is needed there at all is a separate question — see below.On the origin of the shim
Per prior discussion,
normalizeRequestwas introduced to get an e2e test passing before a launch, and the specific failure is no longer remembered. For the webhook route I can confirm it is not needed: with it removed, the fulle2e.test.tspasses on both frameworks, including the webhook cases that assert exact received bodies across the default, static and manualrespondWithpaths.I have not tested removing it from the flow route. That's a larger change and unrelated to this one, so it's left for a follow-up rather than folded in.
Verification
e2e.test.tsNotes for reviewers
Astro dev server. It reads request bodies upstream of the route handler — a route that never touches the body behaves the same way — so the change is only observable in built output (node adapter / Vercel), which is what gets deployed.
No automated regression guard. The property "the webhook wrapper must not read the body" is currently enforced only by the comment in each builder. Making it testable would mean extracting the wrapper template into an exported constant so a unit test could assert it; happy to add that if reviewers want it.