fix(remix-dev/vite, server-runtime): pass Vite server errors to `vite…

….ssrFixStacktrace` (#8066) Co-authored-by: Mark Dalgleish <mark.john.dalgleish@gmail.com>
remix-run · Nov 24, 2023 · dda449a · dda449a
1 parent 3bf45f0
commit dda449a
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 5 deletions.
diff --git a/.changeset/mean-knives-beam.md b/.changeset/mean-knives-beam.md
@@ -0,0 +1,6 @@
+---
+"@remix-run/dev": patch
+"@remix-run/server-runtime": patch
+---
+
+Pass request handler errors to `vite.ssrFixStacktrace` in Vite dev to ensure stack traces correctly map to the original source code
diff --git 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 (
+              <div>
+                <ul>
+                  {["crash-loader", "crash-server-render"].map(
+                    (v) => (
+                      <li key={v}>
+                        <Link to={"/?" + v}>{v}</Link>
+                      </li>
+                    )
+                  )}
+                </ul>
+              </div>
+            );
+          }
+        `,
         "app/routes/known-route-exports.tsx": js`
           import { useMatches } from "@remix-run/react";
 
@@ -385,6 +418,29 @@ test.describe("Vite dev", () => {
     expect(pageErrors).toEqual([]);
   });
 
+  test("request errors map to original source code", 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"
+    );
+  });
+
   test("handle known route exports with HMR", async ({ page }) => {
     let pageErrors: unknown[] = [];
     page.on("pageerror", (error) => pageErrors.push(error));

diff --git 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,6 +682,13 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
               url
             );
           },
+          // 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
@@ -27,10 +27,11 @@ export function logDevReady(build: ServerBuild) {
 }
 
 type DevServerHooks = {
-  getCriticalCss: (
+  getCriticalCss?: (
     build: ServerBuild,
     pathname: string
   ) => Promise<string | undefined>;
+  processRequestError?: (error: unknown) => void;
 };
 
 const globalDevServerHooksKey = "__remix_devServerHooks";

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
@@ -94,12 +94,17 @@ 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 (mode === ServerMode.Development) {
+        getDevServerHooks()?.processRequestError?.(error);
+      }
+
       errorHandler(error, {
         context: loadContext,
         params: matches && matches.length > 0 ? matches[0].params : {},
         request,
       });
+    };
 
     let response: Response;
     if (url.searchParams.has("_data")) {
@@ -137,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(