From 1396b16cc64bbb3fe307c19d2cb25291fd0a4871 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Mon, 20 Nov 2023 18:59:55 +0900 Subject: [PATCH 1/8] fix(vite): apply `ssrFixStacktrace` to server error --- packages/remix-dev/vite/node/adapter.ts | 8 +++++++- packages/remix-dev/vite/plugin.ts | 1 + packages/remix-server-runtime/server.ts | 9 +++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/remix-dev/vite/node/adapter.ts b/packages/remix-dev/vite/node/adapter.ts index 2e6d3c57443..3c2df404693 100644 --- a/packages/remix-dev/vite/node/adapter.ts +++ b/packages/remix-dev/vite/node/adapter.ts @@ -199,15 +199,21 @@ export let createRequestHandler = ( { mode = "production", criticalCss, + ssrFixStacktrace, }: { mode?: string; criticalCss?: string; + ssrFixStacktrace?: (error: Error) => void; } ) => { let handler = createBaseRequestHandler(build, mode); return async (req: IncomingMessage, res: ServerResponse) => { let request = createRequest(req); - let response = await handler(request, {}, { __criticalCss: criticalCss }); + let response = await handler( + request, + {}, + { __criticalCss: criticalCss, __ssrFixStacktrace: ssrFixStacktrace } + ); handleNodeResponse(response, res); }; }; diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index f60161f5153..5f8e385da56 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -749,6 +749,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { let handle = createRequestHandler(build, { mode: "development", criticalCss, + ssrFixStacktrace: vite.ssrFixStacktrace, }); await handle(req, res); diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 471047a43b1..dc38e68a73e 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -37,6 +37,7 @@ export type RequestHandler = ( * @private This is an internal API intended for use by the Remix Vite plugin in dev mode */ __criticalCss?: string; + __ssrFixStacktrace?: (error: Error) => void; } ) => Promise; @@ -83,7 +84,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( return async function requestHandler( request, loadContext = {}, - { __criticalCss: criticalCss } = {} + { __criticalCss: criticalCss, __ssrFixStacktrace: ssrFixStacktrace } = {} ) { _build = typeof build === "function" ? await build() : build; if (typeof build === "function") { @@ -103,12 +104,16 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let url = new URL(request.url); let matches = matchServerRoutes(routes, url.pathname); - let handleError = (error: unknown) => + let handleError = (error: unknown) => { + if (ssrFixStacktrace && error instanceof Error) { + ssrFixStacktrace(error); + } errorHandler(error, { context: loadContext, params: matches && matches.length > 0 ? matches[0].params : {}, request, }); + }; let response: Response; if (url.searchParams.has("_data")) { From 25e865c0a92dc2454d25a41655585d6801828549 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 09:12:29 +0900 Subject: [PATCH 2/8] test: add test --- integration/vite-dev-test.ts | 46 ++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/integration/vite-dev-test.ts b/integration/vite-dev-test.ts index 0edf0593afa..7c856309a6d 100644 --- a/integration/vite-dev-test.ts +++ b/integration/vite-dev-test.ts @@ -197,6 +197,39 @@ test.describe("Vite dev", () => { } `, + "app/routes/error-stacktrace.tsx": js` + import type { LoaderFunction, MetaFunction } from "@remix-run/node"; + import { Link, useLocation } from "@remix-run/react"; + + export const loader: LoaderFunction = ({ request }) => { + if (request.url.includes("crash-loader")) { + throw new Error("crash-loader"); + } + return null; + }; + + export default function TestRoute() { + const location = useLocation(); + + if (import.meta.env.SSR && location.search.includes("crash-server-render")) { + throw new Error("crash-server-render"); + } + + return ( +
+
    + {["crash-loader", "crash-server-render"].map( + (v) => ( +
  • + {v} +
    • + ) + )} +
+
+ ); + } + `, }, }); @@ -354,6 +387,19 @@ test.describe("Vite dev", () => { expect(pageErrors).toEqual([]); }); + + test("server error stacktrace", async ({ page }) => { + let pageErrors: unknown[] = []; + page.on("pageerror", (error) => pageErrors.push(error)); + + await page.goto(`http://localhost:${devPort}/error-stacktrace?crash-server-render`); + await expect(page.locator("main")).toContainText("Error: crash-server-render"); + await expect(page.locator("main")).toContainText("error-stacktrace.tsx:16:11"); + + await page.goto(`http://localhost:${devPort}/error-stacktrace?crash-loader`); + await expect(page.locator("main")).toContainText("Error: crash-loader"); + await expect(page.locator("main")).toContainText("error-stacktrace.tsx:7:11"); + }) }); let bufferize = (stream: Readable): (() => string) => { From 9b7b08f28fe41865a7b39cf70a6e11540e903025 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 09:14:50 +0900 Subject: [PATCH 3/8] chore: changeset --- .changeset/mean-knives-beam.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/mean-knives-beam.md diff --git a/.changeset/mean-knives-beam.md b/.changeset/mean-knives-beam.md new file mode 100644 index 00000000000..e737ca86739 --- /dev/null +++ b/.changeset/mean-knives-beam.md @@ -0,0 +1,6 @@ +--- +"@remix-run/dev": patch +"@remix-run/server-runtime": patch +--- + +Fix server error stacktrace From cc9d5f95dd471c0e3074c528ea66dcf6027c97d2 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 23 Nov 2023 13:03:36 +0900 Subject: [PATCH 4/8] refactor: add DevServerHooks.fixStackTrace --- packages/remix-dev/vite/plugin.ts | 1 + packages/remix-server-runtime/dev.ts | 1 + packages/remix-server-runtime/server.ts | 5 +++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 671252d9296..4b0539234fd 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -682,6 +682,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { url ); }, + fixStackTrace: vite.ssrFixStacktrace, }); // We cache the pluginConfig here to make sure we're only invalidating virtual modules when necessary. diff --git a/packages/remix-server-runtime/dev.ts b/packages/remix-server-runtime/dev.ts index 622ffa22c2b..0b7bfc2d50a 100644 --- a/packages/remix-server-runtime/dev.ts +++ b/packages/remix-server-runtime/dev.ts @@ -31,6 +31,7 @@ type DevServerHooks = { build: ServerBuild, pathname: string ) => Promise; + fixStackTrace: (error: Error) => void; }; const globalDevServerHooksKey = "__remix_devServerHooks"; diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 85556c63a37..dc71d03ae60 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -95,8 +95,9 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let matches = matchServerRoutes(routes, url.pathname); let handleError = (error: unknown) => { - if (ssrFixStacktrace && error instanceof Error) { - ssrFixStacktrace(error); + let devServerHooks = getDevServerHooks(); + if (devServerHooks && error instanceof Error) { + devServerHooks.fixStackTrace(error); } errorHandler(error, { context: loadContext, From 6708f3e82aa4cf39034018a749b235d3e28d7005 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Thu, 23 Nov 2023 13:08:03 +0900 Subject: [PATCH 5/8] chore: rename --- packages/remix-dev/vite/plugin.ts | 2 +- packages/remix-server-runtime/dev.ts | 2 +- packages/remix-server-runtime/server.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 4b0539234fd..7e9faf87084 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -682,7 +682,7 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { url ); }, - fixStackTrace: vite.ssrFixStacktrace, + fixStacktrace: vite.ssrFixStacktrace, }); // We cache the pluginConfig here to make sure we're only invalidating virtual modules when necessary. diff --git a/packages/remix-server-runtime/dev.ts b/packages/remix-server-runtime/dev.ts index 0b7bfc2d50a..e6a789833f0 100644 --- a/packages/remix-server-runtime/dev.ts +++ b/packages/remix-server-runtime/dev.ts @@ -31,7 +31,7 @@ type DevServerHooks = { build: ServerBuild, pathname: string ) => Promise; - fixStackTrace: (error: Error) => void; + fixStacktrace: (error: Error) => void; }; const globalDevServerHooksKey = "__remix_devServerHooks"; diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index dc71d03ae60..a81a3a98e62 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -97,7 +97,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let handleError = (error: unknown) => { let devServerHooks = getDevServerHooks(); if (devServerHooks && error instanceof Error) { - devServerHooks.fixStackTrace(error); + devServerHooks.fixStacktrace(error); } errorHandler(error, { context: loadContext, From 7b0dd6a3d740ea299c69a66cc275f4ab02e09fa8 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 24 Nov 2023 11:14:26 +1100 Subject: [PATCH 6/8] update changeset --- .changeset/mean-knives-beam.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/mean-knives-beam.md b/.changeset/mean-knives-beam.md index e737ca86739..461c0e72ddf 100644 --- a/.changeset/mean-knives-beam.md +++ b/.changeset/mean-knives-beam.md @@ -3,4 +3,4 @@ "@remix-run/server-runtime": patch --- -Fix server error stacktrace +Pass request handler errors to `vite.ssrFixStacktrace` in Vite dev to ensure stack traces correctly map to the original source code From 6ded334a2d1931c4da24b3ee044826b7d6e04f45 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 24 Nov 2023 11:15:21 +1100 Subject: [PATCH 7/8] rename hook to processRequestError --- packages/remix-dev/vite/plugin.ts | 12 +++++++++--- packages/remix-server-runtime/dev.ts | 4 ++-- packages/remix-server-runtime/server.ts | 8 ++++---- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts index 7e9faf87084..2edb79fc0be 100644 --- a/packages/remix-dev/vite/plugin.ts +++ b/packages/remix-dev/vite/plugin.ts @@ -669,9 +669,9 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { setTimeout(showUnstableWarning, 50); }); - // Give the request handler access to the critical CSS in dev to avoid a - // flash of unstyled content since Vite injects CSS file contents via JS setDevServerHooks({ + // Give the request handler access to the critical CSS in dev to avoid a + // flash of unstyled content since Vite injects CSS file contents via JS getCriticalCss: async (build, url) => { invariant(cachedPluginConfig); return getStylesForUrl( @@ -682,7 +682,13 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => { url ); }, - fixStacktrace: vite.ssrFixStacktrace, + // If an error is caught within the request handler, let Vite fix the + // stack trace so it maps back to the actual source code + processRequestError: (error) => { + if (error instanceof Error) { + vite.ssrFixStacktrace(error); + } + }, }); // We cache the pluginConfig here to make sure we're only invalidating virtual modules when necessary. diff --git a/packages/remix-server-runtime/dev.ts b/packages/remix-server-runtime/dev.ts index e6a789833f0..712389a1c1f 100644 --- a/packages/remix-server-runtime/dev.ts +++ b/packages/remix-server-runtime/dev.ts @@ -27,11 +27,11 @@ export function logDevReady(build: ServerBuild) { } type DevServerHooks = { - getCriticalCss: ( + getCriticalCss?: ( build: ServerBuild, pathname: string ) => Promise; - fixStacktrace: (error: Error) => void; + processRequestError?: (error: unknown) => void; }; const globalDevServerHooksKey = "__remix_devServerHooks"; diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index a81a3a98e62..a0e1900df58 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -95,10 +95,10 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( let matches = matchServerRoutes(routes, url.pathname); let handleError = (error: unknown) => { - let devServerHooks = getDevServerHooks(); - if (devServerHooks && error instanceof Error) { - devServerHooks.fixStacktrace(error); + if (mode === ServerMode.Development) { + getDevServerHooks()?.processRequestError?.(error); } + errorHandler(error, { context: loadContext, params: matches && matches.length > 0 ? matches[0].params : {}, @@ -142,7 +142,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = ( } else { let criticalCss = mode === ServerMode.Development - ? await getDevServerHooks()?.getCriticalCss(_build, url.pathname) + ? await getDevServerHooks()?.getCriticalCss?.(_build, url.pathname) : undefined; response = await handleDocumentRequestRR( From 09dcf0aa885ed35b9e97e07bcaea0768d2d17e0f Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 24 Nov 2023 11:16:30 +1100 Subject: [PATCH 8/8] tweak test name --- integration/vite-dev-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/vite-dev-test.ts b/integration/vite-dev-test.ts index 493c6ca642f..cf2251736cf 100644 --- a/integration/vite-dev-test.ts +++ b/integration/vite-dev-test.ts @@ -418,7 +418,7 @@ test.describe("Vite dev", () => { expect(pageErrors).toEqual([]); }); - test("server error stacktrace", async ({ page }) => { + test("request errors map to original source code", async ({ page }) => { let pageErrors: unknown[] = []; page.on("pageerror", (error) => pageErrors.push(error));