diff --git a/examples/app-router-cloudflare/pages/index.tsx b/examples/app-router-cloudflare/pages/index.tsx new file mode 100644 index 00000000..a016be35 --- /dev/null +++ b/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
pages index
; +} diff --git a/packages/vinext/src/server/api-handler.ts b/packages/vinext/src/server/api-handler.ts index 91c8e7dd..4a63d3cc 100644 --- a/packages/vinext/src/server/api-handler.ts +++ b/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"); diff --git a/tests/e2e/pages-router/instrumentation.spec.ts b/tests/e2e/pages-router/instrumentation.spec.ts new file mode 100644 index 00000000..dcbdda6b --- /dev/null +++ b/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); + }); +}); diff --git a/tests/fixtures/pages-basic/instrumentation-state.ts b/tests/fixtures/pages-basic/instrumentation-state.ts new file mode 100644 index 00000000..c725f837 --- /dev/null +++ b/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] = []; +} diff --git a/tests/fixtures/pages-basic/instrumentation.ts b/tests/fixtures/pages-basic/instrumentation.ts new file mode 100644 index 00000000..246389b1 --- /dev/null +++ b/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 { + markRegisterCalled(); +} + +export async function onRequestError( + error: Error, + request: { path: string; method: string; headers: Record }, + context: { + routerKind: string; + routePath: string; + routeType: string; + }, +): Promise { + recordRequestError({ + message: error.message, + path: request.path, + method: request.method, + routerKind: context.routerKind, + routePath: context.routePath, + routeType: context.routeType, + }); +} diff --git a/tests/fixtures/pages-basic/pages/api/error-route.ts b/tests/fixtures/pages-basic/pages/api/error-route.ts new file mode 100644 index 00000000..d3b64b44 --- /dev/null +++ b/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"); +} diff --git a/tests/fixtures/pages-basic/pages/api/instrumentation-test.ts b/tests/fixtures/pages-basic/pages/api/instrumentation-test.ts new file mode 100644 index 00000000..58f379ca --- /dev/null +++ b/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(), + }); +}