[PR #493] [MERGED] fix: register reportRequestError with ctx.waitUntil on Workers #615

New issue

Closed

opened 2026-05-06 13:09:05 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:09:05 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/493
Author: @NathanDrake2406
Created: 3/12/2026
Status: ✅ Merged
Merged: 3/12/2026
Merged by: @james-elicx

Base: main ← Head: fix/report-request-error-waituntil

📝 Commits (1)

687397b fix: register reportRequestError with ctx.waitUntil on Workers

📊 Changes

7 files changed (+83 additions, -83 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+3 -9)
📝 packages/vinext/src/entries/pages-server-entry.ts (+2 -2)
📝 packages/vinext/src/server/api-handler.ts (+1 -3)
📝 packages/vinext/src/server/dev-server.ts (+1 -3)
📝 packages/vinext/src/server/instrumentation.ts (+21 -10)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+20 -56)
📝 tests/instrumentation.test.ts (+35 -0)

📄 Description

Summary

Follow-up to #492. On Cloudflare Workers, the isolate can terminate after the Response is sent, killing in-flight error reports (e.g. Sentry HTTP requests) that were fire-and-forget.

reportRequestError() now self-registers with ctx.waitUntil() via the ALS-backed getRequestExecutionContext(), matching the existing pattern used by KVCacheHandler, ISR regeneration, and fetch cache revalidation
On Node.js (dev or vinext start), getRequestExecutionContext() returns null — fire-and-forget remains correct since the process doesn't die
Since reportRequestError never rejects (internal try/catch), the redundant .catch() at all 5 call sites across both routers is removed

Test plan

New test: registers the report promise with ctx.waitUntil on Workers — verifies waitUntil is called with the report promise inside runWithExecutionContext
New test: does not call waitUntil when no execution context is available — verifies no-op on Node.js
All 13 instrumentation tests pass
pages-router.test.ts passes (166 tests)
entry-templates.test.ts passes (12 tests)
api-handler.test.ts passes (40 tests)
CI: full Vitest + Playwright E2E suite

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/493 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 3/12/2026 **Status:** ✅ Merged **Merged:** 3/12/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/report-request-error-waituntil` --- ### 📝 Commits (1) - [`687397b`](https://github.com/cloudflare/vinext/commit/687397bdfd0922d5a639f27e084a69ec11e9a84c) fix: register reportRequestError with ctx.waitUntil on Workers ### 📊 Changes **7 files changed** (+83 additions, -83 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+3 -9) 📝 `packages/vinext/src/entries/pages-server-entry.ts` (+2 -2) 📝 `packages/vinext/src/server/api-handler.ts` (+1 -3) 📝 `packages/vinext/src/server/dev-server.ts` (+1 -3) 📝 `packages/vinext/src/server/instrumentation.ts` (+21 -10) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+20 -56) 📝 `tests/instrumentation.test.ts` (+35 -0) </details> ### 📄 Description ## Summary Follow-up to #492. On Cloudflare Workers, the isolate can terminate after the Response is sent, killing in-flight error reports (e.g. Sentry HTTP requests) that were fire-and-forget. - `reportRequestError()` now self-registers with `ctx.waitUntil()` via the ALS-backed `getRequestExecutionContext()`, matching the existing pattern used by `KVCacheHandler`, ISR regeneration, and fetch cache revalidation - On Node.js (dev or `vinext start`), `getRequestExecutionContext()` returns `null` — fire-and-forget remains correct since the process doesn't die - Since `reportRequestError` never rejects (internal try/catch), the redundant `.catch()` at all 5 call sites across both routers is removed ## Test plan - [x] New test: `registers the report promise with ctx.waitUntil on Workers` — verifies `waitUntil` is called with the report promise inside `runWithExecutionContext` - [x] New test: `does not call waitUntil when no execution context is available` — verifies no-op on Node.js - [x] All 13 instrumentation tests pass - [x] `pages-router.test.ts` passes (166 tests) - [x] `entry-templates.test.ts` passes (12 tests) - [x] `api-handler.test.ts` passes (40 tests) - [ ] CI: full Vitest + Playwright E2E suite --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>