cloudflare / cloudflare/vinext

Await instrumentation hook in route handler prepare (parity with Next.js #94306)

Open
#1,718 0 comments 0 reactions 0 assignees View on GitHub
nextjs-tracking
Dominant language
TypeScript
Stars
8.8k
Forks
406
Avg merge
2d 6h
Merged PRs (30d)
120

Description

## Upstream

Next.js commit [`62467ee`](https://github.com/vercel/next.js/commit/62467ee5cc32a4c0ca3da8e0f7b973bf9068b307) — PR [vercel/next.js#94306](https://github.com/vercel/next.js/pull/94306), closes [vercel/next.js#93994](https://github.com/vercel/next.js/issues/93994).

> The per-request call to `ensureInstrumentationRegistered()` inside `RouteModule.prepare` was missing an `await`, so when an adapter dispatches the compiled app-route template's handler without going through `BaseServer.handleRequest` (where `prepareImpl` awaits it via `runInstrumentationHookIfAvailable`), a cold-start request could begin executing the userland route handler before `instrumentation.ts#register()` resolved.

## What changed in Next.js

Single-line fix in `packages/next/src/server/route-modules/route-module.ts`:

\`\`\`diff
- ensureInstrumentationRegistered(absoluteProjectDir, this.distDir)
+ await ensureInstrumentationRegistered(absoluteProjectDir, this.distDir)
\`\`\`

Plus a new e2e test (`test/e2e/instrumentation-hook/with-async-node-app-route/`) that:
- Registers an async `instrumentation.js` `register()` that sleeps 1s and sets `globalThis.instrumentationFinished = true`.
- Has an app route handler at `/api/check` that returns that flag.
- Runs with `skipDeployment: false` so it exercises the adapter path that bypasses `BaseServer.handleRequest`.

## Why this matters for vinext

vinext implements its own request lifecycle (`server/prod-server.ts`, `cloudflare/worker-entry.ts`, `entries/app-rsc-entry.ts`) rather than going through `BaseServer.handleRequest`. The exact bug described in the Next.js PR — async `register()` not completing before the first route handler runs on a cold start — applies to any adapter-style runtime, which is what vinext is on Cloudflare Workers.

We need to verify:

1. Whether vinext supports `instrumentation.ts` / `instrumentation.js` `register()` at all (App Router and Pages Router).
2. If it does, whether the `register()` promise is awaited before the first request is served on cold start, in:
- App Router dev (`entries/app-rsc-entry.ts`)
- Pages Router dev (`server/dev-server.ts`)
- Pages Router prod (`server/prod-server.ts`)
- Cloudflare Workers entry (`cloudflare/worker-entry.ts`)
3. Whether async `register()` (e.g., dynamic import of `instrumentation.node.js`, awaiting setup work) is correctly serialized before route handlers run.

## Suggested test

Port the upstream fixture to `tests/fixtures/` or the existing instrumentation tests:

- `instrumentation.js` with async `register()` that sleeps and sets a global flag.
- App route handler at `/api/check` that returns `{ finished: Boolean(globalThis.instrumentationFinished) }`.
- Assert the first request returns `{ finished: true }` on a cold start.

Include a comment linking back to:
- https://github.com/vercel/next.js/blob/canary/test/e2e/instrumentation-hook/with-async-node-app-route/
- https://github.com/vercel/next.js/pull/94306

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.