fix: pages router instrumentation error reporting for api routes

examples/app-router-cloudflare/pages/index.tsx

@@ -0,0 +1,4 @@
+// Ensure app router works when a pages directory is present
+export default function Page() {
+  return <div>pages index</div>;
+}

packages/vinext/src/server/api-handler.ts

@@ -10,6 +10,7 @@
 import type { ViteDevServer } from "vite";
 import type { IncomingMessage, ServerResponse } from "node:http";
 import { type Route, matchRoute } from "../routing/pages-router.js";
+import { reportRequestError } from "./instrumentation.js";
 import { addQueryParam } from "../utils/query.js";
 
 /**
@@ -207,6 +208,17 @@ export async function handleApiRoute(
   } catch (e) {
     server.ssrFixStacktrace(e as Error);
     console.error(e);
+    reportRequestError(
+      e instanceof Error ? e : new Error(String(e)),
+      {
+        path: url,
+        method: req.method ?? "GET",
+        headers: Object.fromEntries(
+          Object.entries(req.headers).map(([k, v]) => [k, Array.isArray(v) ? v.join(", ") : String(v ?? "")]),
+        ),
+      },
+      { routerKind: "Pages Router", routePath: match.route.pattern, routeType: "route" },
+    ).catch(() => { /* ignore reporting errors */ });
     if ((e as Error).message === "Request body too large") {
       res.statusCode = 413;
       res.end("Request body too large");

tests/e2e/pages-router/instrumentation.spec.ts

@@ -0,0 +1,136 @@
+/**
+ * E2E tests for instrumentation.ts support in Pages Router apps.
+ *
+ * ## Regression covered
+ *
+ * Pages Router apps have no RSC environment, so the loader selection in
+ * index.ts falls back to `server` (i.e. `server.ssrLoadModule`). In Vite 7,
+ * calling `ssrLoadModule` during `configureServer` — before the dev server is
+ * listening — crashes with:
+ *
+ *   TypeError: Cannot read properties of undefined (reading 'outsideEmitter')
+ *
+ * because `SSRCompatModuleRunner` requires a hot channel that only exists
+ * after the server is fully set up. The fix must avoid using `ssrLoadModule`
+ * at startup for Pages Router apps.
+ *
+ * If the regression is present, the dev server crashes on startup and the
+ * test below can never pass (Playwright's webServer will fail to connect).
+ *
+ * References:
+ * - https://nextjs.org/docs/app/building-your-application/optimizing/instrumentation
+ */
+
+import { test, expect } from "@playwright/test";
+
+const BASE = "http://localhost:4173";
+
+/**
+ * Startup test — must NOT reset before running, because register() is called
+ * once at server startup. If the server crashed before it started listening
+ * (the regression), Playwright would never reach this test.
+ */
+test.describe("instrumentation.ts startup (Pages Router)", () => {
+  test("dev server starts without crashing when instrumentation.ts is present", async ({
+    request,
+  }) => {
+    // If we reach this line at all, the server started successfully —
+    // the startup crash regression is not present.
+    const res = await request.get(`${BASE}/api/instrumentation-test`);
+    expect(res.status()).toBe(200);
+  });
+
+  test("register() was called before the first request", async ({
+    request,
+  }) => {
+    // Do NOT reset first — register() fires once at startup and won't be
+    // re-invoked after a DELETE reset.
+    const res = await request.get(`${BASE}/api/instrumentation-test`);
+    expect(res.status()).toBe(200);
+
+    const data = await res.json();
+    // register() must have been invoked once when the dev server started.
+    expect(data.registerCalled).toBe(true);
+    expect(Array.isArray(data.errors)).toBe(true);
+  });
+});
+
+test.describe("instrumentation.ts onRequestError (Pages Router)", () => {
+  test.beforeEach(async ({ request }) => {
+    // Reset captured state before each test so errors from earlier tests
+    // don't bleed through. Note: this also resets registerCalled, which is
+    // why the startup tests live in a separate describe block above.
+    const res = await request.delete(`${BASE}/api/instrumentation-test`);
+    expect(res.status()).toBe(200);
+  });
+
+  test("successful requests do not trigger onRequestError()", async ({
+    request,
+  }) => {
+    const okRes = await request.get(`${BASE}/api/hello`);
+    expect(okRes.status()).toBe(200);
+
+    // Give any async reportRequestError() call a moment to settle.
+    await new Promise((resolve) => setTimeout(resolve, 200));
+
+    const stateRes = await request.get(`${BASE}/api/instrumentation-test`);
+    const data = await stateRes.json();
+    // After the reset in beforeEach, no errors should have been recorded
+    // by the successful /api/hello request.
+    expect(data.errors.length).toBe(0);
+  });
+
+  test("onRequestError() is called when a route handler throws", async ({
+    request,
+  }) => {
+    // /api/error-route throws an unhandled Error — vinext should invoke
+    // the onRequestError handler registered in instrumentation.ts.
+    const errorRes = await request.get(`${BASE}/api/error-route`);
+    expect(errorRes.status()).toBe(500);
+
+    // Give the async reportRequestError() call a moment to complete.
+    await new Promise((resolve) => setTimeout(resolve, 200));
+
+    const stateRes = await request.get(`${BASE}/api/instrumentation-test`);
+    expect(stateRes.status()).toBe(200);
+
+    const data = await stateRes.json();
+    expect(data.errors.length).toBeGreaterThanOrEqual(1);
+
+    const err = data.errors[data.errors.length - 1];
+    expect(err.message).toBe("Intentional route handler error");
+    expect(err.path).toBe("/api/error-route");
+    expect(err.method).toBe("GET");
+    expect(err.routerKind).toBe("Pages Router");
+    expect(err.routeType).toBe("route");
+	});
+
+  test("onRequestError() receives the correct route path pattern", async ({
+    request,
+  }) => {
+    const errorRes = await request.get(`${BASE}/api/error-route`);
+    expect(errorRes.status()).toBe(500);
+
+    await new Promise((resolve) => setTimeout(resolve, 200));
+
+    const stateRes = await request.get(`${BASE}/api/instrumentation-test`);
+    const data = await stateRes.json();
+    expect(data.errors.length).toBeGreaterThanOrEqual(1);
+
+    const err = data.errors[data.errors.length - 1];
+    // The routePath should be the file-system route pattern, not the concrete URL.
+    expect(err.routePath).toContain("error-route");
+  });
+
+  test("multiple errors are captured independently", async ({ request }) => {
+    // Fire the error route twice and verify both entries are recorded.
+    await request.get(`${BASE}/api/error-route`);
+    await request.get(`${BASE}/api/error-route`);
+
+    await new Promise((resolve) => setTimeout(resolve, 400));
+
+    const stateRes = await request.get(`${BASE}/api/instrumentation-test`);
+    const data = await stateRes.json();
+    expect(data.errors.length).toBeGreaterThanOrEqual(2);
+  });
+});

tests/fixtures/pages-basic/instrumentation-state.ts

@@ -0,0 +1,63 @@
+/**
+ * Shared in-memory state for instrumentation.ts testing in pages-basic.
+ *
+ * ## Why globalThis?
+ *
+ * vinext loads instrumentation.ts via a direct-call ModuleRunner built on the
+ * SSR DevEnvironment's fetchModule() method. That runner creates its own isolated
+ * module graph — separate from the Vite SSR module graph that handles API routes.
+ *
+ * If state were stored as plain module-level variables, the runner instance would
+ * set registerCalled = true in its copy, but the API route would read from its own
+ * copy (always false). Storing everything on globalThis bridges the gap: both
+ * module graph instances run in the same Node.js process and share the same global
+ * object, so writes from one are immediately visible to reads from the other.
+ *
+ * This is the same pattern used by instrumentation.ts itself to store the
+ * onRequestError handler across Vite environment boundaries.
+ */
+
+export interface CapturedRequestError {
+  message: string;
+  path: string;
+  method: string;
+  routerKind: string;
+  routePath: string;
+  routeType: string;
+}
+
+const REGISTER_KEY = "__vinext_pages_test_registerCalled__";
+const ERRORS_KEY = "__vinext_pages_test_capturedErrors__";
+
+function _errors(): CapturedRequestError[] {
+  if (!(globalThis as any)[ERRORS_KEY]) {
+    (globalThis as any)[ERRORS_KEY] = [];
+  }
+  return (globalThis as any)[ERRORS_KEY] as CapturedRequestError[];
+}
+
+/** True once instrumentation.ts register() has been called. */
+export function isRegisterCalled(): boolean {
+  return (globalThis as any)[REGISTER_KEY] === true;
+}
+
+/** All errors captured by onRequestError() so far. */
+export function getCapturedErrors(): CapturedRequestError[] {
+  return _errors();
+}
+
+/** Called by instrumentation.ts register(). */
+export function markRegisterCalled(): void {
+  (globalThis as any)[REGISTER_KEY] = true;
+}
+
+/** Called by instrumentation.ts onRequestError(). */
+export function recordRequestError(entry: CapturedRequestError): void {
+  _errors().push(entry);
+}
+
+/** Reset all state (used between test runs). */
+export function resetInstrumentationState(): void {
+  (globalThis as any)[REGISTER_KEY] = false;
+  (globalThis as any)[ERRORS_KEY] = [];
+}

tests/fixtures/pages-basic/instrumentation.ts

@@ -0,0 +1,44 @@
+/**
+ * instrumentation.ts for pages-basic test fixture.
+ *
+ * This file exercises the instrumentation.ts support for Pages Router apps.
+ * The key regression it covers: calling server.ssrLoadModule() during
+ * configureServer() (at startup, before the server is listening) crashes in
+ * Vite 7 with:
+ *
+ *   TypeError: Cannot read properties of undefined (reading 'outsideEmitter')
+ *
+ * because SSRCompatModuleRunner requires a hot channel that doesn't exist yet.
+ * Pages Router apps have no RSC environment, so the loader selection in
+ * index.ts falls back to `server` (ssrLoadModule). The fix must avoid calling
+ * ssrLoadModule at startup — if the dev server crashes before it starts
+ * listening, the e2e test for register() being called will never pass.
+ */
+
+import {
+  markRegisterCalled,
+  recordRequestError,
+} from "./instrumentation-state";
+
+export async function register(): Promise<void> {
+  markRegisterCalled();
+}
+
+export async function onRequestError(
+  error: Error,
+  request: { path: string; method: string; headers: Record<string, string> },
+  context: {
+    routerKind: string;
+    routePath: string;
+    routeType: string;
+  },
+): Promise<void> {
+  recordRequestError({
+    message: error.message,
+    path: request.path,
+    method: request.method,
+    routerKind: context.routerKind,
+    routePath: context.routePath,
+    routeType: context.routeType,
+  });
+}

tests/fixtures/pages-basic/pages/api/error-route.ts

@@ -0,0 +1,7 @@
+import type { NextApiRequest, NextApiResponse } from "next";
+
+// API route that throws an error — used by instrumentation e2e tests to verify
+// that onRequestError() is called when a Pages Router API handler throws.
+export default function handler(_req: NextApiRequest, _res: NextApiResponse) {
+  throw new Error("Intentional route handler error");
+}

tests/fixtures/pages-basic/pages/api/instrumentation-test.ts

@@ -0,0 +1,29 @@
+import type { NextApiRequest, NextApiResponse } from "next";
+import {
+  isRegisterCalled,
+  getCapturedErrors,
+  resetInstrumentationState,
+} from "../../instrumentation-state";
+
+/**
+ * API route that exposes the current instrumentation state for e2e testing.
+ *
+ * GET /api/instrumentation-test
+ *   Returns { registerCalled, errors } so Playwright tests can assert that
+ *   instrumentation.ts register() was called on startup and that
+ *   onRequestError() fired for any unhandled route errors.
+ *
+ * DELETE /api/instrumentation-test
+ *   Resets the captured state so tests can start from a clean slate.
+ */
+export default function handler(req: NextApiRequest, res: NextApiResponse) {
+  if (req.method === "DELETE") {
+    resetInstrumentationState();
+    return res.status(200).json({ ok: true });
+  }
+
+  return res.status(200).json({
+    registerCalled: isRegisterCalled(),
+    errors: getCapturedErrors(),
+  });
+}