From 9d0d6e37a4f4add7efc4f66ccc6ca2fdb8c413b7 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Tue, 18 Oct 2022 15:24:35 -0700
Subject: [PATCH 01/30] initial work for document requests

---
 .../__tests__/server-test.ts                  | 179 +++++++++++-------
 packages/remix-server-runtime/headers.ts      |  36 ++++
 packages/remix-server-runtime/server.ts       | 140 +++++++++++++-
 3 files changed, 282 insertions(+), 73 deletions(-)

diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index 391eacb2b04..abda5b1a1e1 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -3,7 +3,8 @@ import { ServerMode } from "../mode";
 import type { ServerBuild } from "../build";
 import { mockServerBuild } from "./utils";
 
-const DATA_CALL_MULTIPIER = process.env.ENABLE_REMIX_ROUTER ? 2 : 1;
+const ENABLE_REMIX_ROUTER = !!process.env.ENABLE_REMIX_ROUTER;
+const DATA_CALL_MULTIPIER = ENABLE_REMIX_ROUTER ? 2 : 1;
 
 function spyConsole() {
   // https://github.com/facebook/react/issues/7047
@@ -757,10 +758,12 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(404);
       expect(rootLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(404);
@@ -785,10 +788,12 @@ describe("shared server runtime", () => {
       let result = await handler(request);
       expect(result.status).toBe(404);
       expect(rootLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(404);
@@ -822,12 +827,14 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(rootLoader.mock.calls.length).toBe(1);
-      expect(indexLoader.mock.calls.length).toBe(1);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(indexLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -864,12 +871,14 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(rootLoader.mock.calls.length).toBe(1);
-      expect(indexLoader.mock.calls.length).toBe(1);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(indexLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -909,14 +918,16 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(testAction.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       // Should not call root loader since it is the boundary route
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -954,14 +965,16 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(indexAction.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       // Should not call root loader since it is the boundary route
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -1000,13 +1013,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(testAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -1047,13 +1062,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(indexAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch!.status).toBe(400);
@@ -1102,13 +1119,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(testAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch.data).toBe("action");
@@ -1159,13 +1178,19 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(400);
-      expect(indexAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      // TODO: Check this out. Seems to be a gap in not being able to let
+      // RR know that a thrown response we catch should be treated the same
+      // as an error in terms of what subsequent loaders are called after an
+      // action throws a response.
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.catch).toBeTruthy();
       expect(entryContext.appState.catch.data).toBe("action");
@@ -1203,12 +1228,14 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(rootLoader.mock.calls.length).toBe(1);
-      expect(indexLoader.mock.calls.length).toBe(1);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(indexLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("index");
@@ -1245,12 +1272,14 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(rootLoader.mock.calls.length).toBe(1);
-      expect(indexLoader.mock.calls.length).toBe(1);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(indexLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("index");
@@ -1290,14 +1319,16 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(testAction.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       // Should not call root loader since it is the boundary route
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("test");
@@ -1335,14 +1366,16 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(indexAction.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       // Should not call root loader since it is the boundary route
       expect(rootLoader.mock.calls.length).toBe(0);
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("index");
@@ -1381,13 +1414,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(testAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("test");
@@ -1428,13 +1463,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(indexAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("index");
@@ -1483,13 +1520,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(testAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(testAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(testLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("action");
@@ -1540,13 +1579,15 @@ describe("shared server runtime", () => {
 
       let result = await handler(request);
       expect(result.status).toBe(500);
-      expect(indexAction.mock.calls.length).toBe(1);
-      expect(rootLoader.mock.calls.length).toBe(1);
+      expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
+      expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(indexLoader.mock.calls.length).toBe(0);
-      expect(build.entry.module.default.mock.calls.length).toBe(1);
+      expect(build.entry.module.default.mock.calls.length).toBe(
+        1 * DATA_CALL_MULTIPIER
+      );
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(1);
+      expect(calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       let entryContext = calls[0][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("action");
@@ -1596,7 +1637,7 @@ describe("shared server runtime", () => {
       expect(indexLoader.mock.calls.length).toBe(0);
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(2);
+      expect(calls.length).toBe(2 * DATA_CALL_MULTIPIER);
       let entryContext = calls[1][3];
       expect(entryContext.appState.error).toBeTruthy();
       expect(entryContext.appState.error.message).toBe("thrown");
@@ -1641,7 +1682,7 @@ describe("shared server runtime", () => {
       expect(indexLoader.mock.calls.length).toBe(0);
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(2);
+      expect(calls.length).toBe(2 * DATA_CALL_MULTIPIER);
     });
 
     test("returns more detailed message if handleDocumentRequest throws a second time in development mode", async () => {
@@ -1681,8 +1722,8 @@ describe("shared server runtime", () => {
       expect(indexLoader.mock.calls.length).toBe(0);
 
       let calls = build.entry.module.default.mock.calls;
-      expect(calls.length).toBe(2);
-      expect(spy.console.mock.calls.length).toBe(1);
+      expect(calls.length).toBe(2 * DATA_CALL_MULTIPIER);
+      expect(spy.console.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
     });
   });
 });
diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts
index 19b494266ed..8ecb102143d 100644
--- a/packages/remix-server-runtime/headers.ts
+++ b/packages/remix-server-runtime/headers.ts
@@ -3,6 +3,7 @@ import { splitCookiesString } from "set-cookie-parser";
 import type { ServerBuild } from "./build";
 import type { ServerRoute } from "./routes";
 import type { RouteMatch } from "./routeMatching";
+import type { StaticHandlerContext } from "./router";
 
 export function getDocumentHeaders(
   build: ServerBuild,
@@ -35,6 +36,41 @@ export function getDocumentHeaders(
   }, new Headers());
 }
 
+export function getDocumentHeadersRR(
+  context: StaticHandlerContext,
+  matches: RouteMatch<ServerRoute>[]
+) {
+  let parentHeaders = new Headers();
+
+  for (let match of matches) {
+    let id = match.route.id;
+    let { headers } = match.route.module || {};
+    if (!headers) continue;
+
+    let loaderHeaders = context.loaderHeaders?.[id] || new Headers();
+    let actionHeaders = context.actionHeaders?.[id] || new Headers();
+
+    let newHeaders = new Headers(
+      typeof headers === "function"
+        ? headers({
+            loaderHeaders,
+            actionHeaders,
+            parentHeaders,
+          })
+        : headers
+    );
+
+    // Automatically preserve Set-Cookie headers that were set either by the
+    // loader or by a parent route.
+    prependCookies(actionHeaders, newHeaders);
+    prependCookies(loaderHeaders, newHeaders);
+    prependCookies(parentHeaders, newHeaders);
+    parentHeaders = newHeaders;
+  }
+
+  return parentHeaders;
+}
+
 function prependCookies(parentHeaders: Headers, childHeaders: Headers): void {
   let parentSetCookieString = parentHeaders.get("Set-Cookie");
 
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index f5ee5b200cd..0361e2d135e 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -4,11 +4,11 @@ import { unstable_createStaticHandler } from "./router";
 import type { AppLoadContext } from "./data";
 import { callRouteAction, callRouteLoader, extractData } from "./data";
 import type { AppState } from "./errors";
-import type { ServerBuild } from "./build";
+import type { ServerBuild, HandleDocumentRequestFunction } from "./build";
 import type { EntryContext } from "./entry";
 import { createEntryMatches, createEntryRouteModules } from "./entry";
 import { serializeError } from "./errors";
-import { getDocumentHeaders } from "./headers";
+import { getDocumentHeaders, getDocumentHeadersRR } from "./headers";
 import invariant from "./invariant";
 import { ServerMode, isServerMode } from "./mode";
 import type { RouteMatch } from "./routeMatching";
@@ -121,14 +121,33 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
 
       response = await responsePromise;
     } else {
-      response = await handleDocumentRequest({
+      let responsePromise = handleDocumentRequest({
         build,
         loadContext,
         matches,
-        request,
+        request:
+          // We need to clone the request here instead of the call to the new
+          // handler otherwise the first handler will lock the body for the other.
+          // Cloning here allows the new handler to be the stream reader and delegate
+          // chunks back to this cloned request.
+          ENABLE_REMIX_ROUTER ? request.clone() : request,
         routes,
         serverMode,
       });
+
+      if (ENABLE_REMIX_ROUTER) {
+        let [response, remixRouterResponse] = await Promise.all([
+          responsePromise,
+          handleDocumentRequestRR(serverMode, build, staticHandler!, request),
+        ]);
+
+        assertResponsesMatch(response, remixRouterResponse);
+
+        console.log("Returning Remix Router Document Request Response");
+        responsePromise = Promise.resolve(remixRouterResponse);
+      }
+
+      response = await responsePromise;
     }
 
     if (request.method === "HEAD") {
@@ -294,6 +313,119 @@ async function handleDataRequestRR(
   }
 }
 
+async function handleDocumentRequestRR(
+  serverMode: ServerMode,
+  build: ServerBuild,
+  staticHandler: StaticHandler,
+  request: Request
+) {
+  if (request.method === "HEAD") {
+    request = new Request(request, { method: "GET" });
+  }
+  let context = await staticHandler.query(request);
+
+  if (context instanceof Response) {
+    return context;
+  }
+
+  let appState: AppState = {
+    trackBoundaries: true,
+    trackCatchBoundaries: true,
+    catchBoundaryRouteId: null,
+    renderBoundaryRouteId: null,
+    loaderBoundaryRouteId: null,
+  };
+
+  for (let match of context.matches) {
+    let route = match.route as ServerRoute;
+    let id = route.id;
+    let error = context.errors?.[id];
+
+    if (error) {
+      appState.error = await serializeError(error);
+      appState.loaderBoundaryRouteId = id;
+      break;
+    }
+
+    let actionHeaders = context.actionHeaders?.[id];
+    let loaderHeaders = context.loaderHeaders?.[id];
+    if (
+      actionHeaders?.has("X-Remix-Catch") ||
+      loaderHeaders?.has("X-Remix-Catch")
+    ) {
+      appState.loaderBoundaryRouteId = id;
+      appState.catch = {
+        data: context.actionData?.[id] || context.loaderData?.[id],
+        status: context.statusCode,
+        // TODO: Populate this.
+        statusText: "",
+      };
+      break;
+    }
+  }
+
+  let renderableMatches = getRenderableMatches(
+    context.matches as unknown as RouteMatch<ServerRoute>[],
+    appState
+  );
+
+  if (!renderableMatches) {
+    renderableMatches = [];
+
+    let root = staticHandler.dataRoutes[0] as ServerRoute;
+    if (root?.module.CatchBoundary) {
+      appState.catchBoundaryRouteId = "root";
+      renderableMatches.push({
+        params: {},
+        pathname: "",
+        route: staticHandler.dataRoutes[0] as ServerRoute,
+      });
+    }
+  }
+
+  let headers = getDocumentHeadersRR(context, renderableMatches);
+
+  let serverHandoff: Pick<
+    EntryContext,
+    "actionData" | "appState" | "matches" | "routeData"
+  > = {
+    actionData: context.actionData || undefined,
+    appState,
+    matches: createEntryMatches(renderableMatches, build.assets.routes),
+    routeData: context.loaderData || {},
+  };
+
+  let entryContext: EntryContext = {
+    ...serverHandoff,
+    manifest: build.assets,
+    routeModules: createEntryRouteModules(build.routes),
+    serverHandoffString: createServerHandoffString(serverHandoff),
+  };
+
+  let handleDocumentRequestParameters: Parameters<HandleDocumentRequestFunction> =
+    [request, context.statusCode, headers, entryContext];
+
+  let handleDocumentRequestFunction = build.entry.module.default;
+  try {
+    return await handleDocumentRequestFunction(
+      ...handleDocumentRequestParameters
+    );
+  } catch (error) {
+    handleDocumentRequestParameters[1] = 500;
+    appState.trackBoundaries = false;
+    appState.error = await serializeError(error as Error);
+    entryContext.serverHandoffString = createServerHandoffString(serverHandoff);
+
+    try {
+      return await handleDocumentRequestFunction(
+        ...handleDocumentRequestParameters
+      );
+    } catch (error: any) {
+      return returnLastResortErrorResponse(error, serverMode);
+    }
+  }
+}
+
 async function handleDocumentRequest({
   build,
   loadContext,

From fccc93322c97cdcb60f909232498dd863123cb48 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 19 Oct 2022 12:52:48 -0400
Subject: [PATCH 02/30] Keep thrown responses on the throw path

---
 packages/remix-server-runtime/data.ts         | 14 +++++++++++++
 .../remix-server-runtime/router/router.ts     |  6 +-----
 packages/remix-server-runtime/routes.ts       |  2 ++
 packages/remix-server-runtime/server.ts       | 20 ++++---------------
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index 0a7d1803aac..205e71db667 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -24,12 +24,14 @@ export async function callRouteAction({
   action,
   params,
   request,
+  isRemixRouterRequest,
 }: {
   loadContext: AppLoadContext;
   routeId: string;
   action?: ActionFunction;
   params: DataFunctionArgs["params"];
   request: Request;
+  isRemixRouterRequest?: boolean;
 }) {
   if (!action) {
     let response = new Response(null, { status: 405 });
@@ -51,6 +53,11 @@ export async function callRouteAction({
 
     if (!isRedirectResponse(error)) {
       error.headers.set("X-Remix-Catch", "yes");
+      // This needs to be thrown so @remix-run/router knows to handle it as an error
+      // and not a successful returned response
+      if (isRemixRouterRequest) {
+        throw error;
+      }
     }
     result = error;
   }
@@ -71,12 +78,14 @@ export async function callRouteLoader({
   loader,
   params,
   request,
+  isRemixRouterRequest,
 }: {
   request: Request;
   routeId: string;
   loader?: LoaderFunction;
   params: DataFunctionArgs["params"];
   loadContext: AppLoadContext;
+  isRemixRouterRequest?: boolean;
 }) {
   if (!loader) {
     throw new Error(
@@ -100,6 +109,11 @@ export async function callRouteLoader({
 
     if (!isRedirectResponse(error)) {
       error.headers.set("X-Remix-Catch", "yes");
+      // This needs to be thrown so @remix-run/router knows to handle it as an error
+      // and not a successful returned response
+      if (isRemixRouterRequest) {
+        throw error;
+      }
     }
     result = error;
   }
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 565fc9aaf9b..2e2a420da0f 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -1899,17 +1899,13 @@ export function unstable_createStaticHandler(
     matches: AgnosticDataRouteMatch[],
     isRouteRequest: boolean
   ): Promise<Omit<StaticHandlerContext, "location"> | Response> {
-    invariant(
-      request.method !== "HEAD",
-      "query()/queryRoute() do not support HEAD requests"
-    );
     invariant(
       request.signal,
       "query()/queryRoute() requests must contain an AbortController signal"
     );
 
     try {
-      if (request.method !== "GET") {
+      if (request.method !== "GET" && request.method !== "HEAD") {
         let result = await submit(
           request,
           matches,
diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
index 7a8b1162517..82c407245c0 100644
--- a/packages/remix-server-runtime/routes.ts
+++ b/packages/remix-server-runtime/routes.ts
@@ -75,6 +75,7 @@ export function createStaticHandlerDataRoutes(
                 routeId: route.id,
                 loader: route.module.loader,
                 loadContext,
+                isRemixRouterRequest: true,
               })
           : undefined,
         action: route.module.action
@@ -84,6 +85,7 @@ export function createStaticHandlerDataRoutes(
                 routeId: route.id,
                 action: route.module.action,
                 loadContext,
+                isRemixRouterRequest: true,
               })
           : undefined,
         handle: route.module.handle,
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 0361e2d135e..2d8a0fc029d 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -286,19 +286,7 @@ async function handleDataRequestRR(
     return response;
   } catch (error) {
     if (error instanceof Response) {
-      // To match existing behavior of remix-thrown responses,
-      // we construct a new one here with a null body and just
-      // the required headers. No remix-throw that surface to
-      // this point will ever have a body. This contrasts with
-      // the new router implementation that has a body under
-      // some conditions.
-      return new Response(null, {
-        status: error.status,
-        statusText: error.statusText,
-        headers: {
-          "X-Remix-Catch": "yes",
-        },
-      });
+      return error;
     }
 
     if (serverMode !== ServerMode.Test) {
@@ -319,9 +307,6 @@ async function handleDocumentRequestRR(
   staticHandler: StaticHandler,
   request: Request
 ) {
-  if (request.method === "HEAD") {
-    request = new Request(request, { method: "GET" });
-  }
   let context = await staticHandler.query(request);
 
   if (context instanceof Response) {
@@ -767,6 +752,9 @@ async function handleResourceRequestRR(
     );
     return response;
   } catch (error) {
+    if (error instanceof Response) {
+      return error;
+    }
     return returnLastResortErrorResponse(error, serverMode);
   }
 }

From 138222f5b1303a54c3f45d6ed74bf7c8c2c9d84a Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 19 Oct 2022 16:27:33 -0400
Subject: [PATCH 03/30] get headers integration tests passing

---
 packages/remix-server-runtime/headers.ts | 37 ++++++++++--------------
 packages/remix-server-runtime/server.ts  |  2 +-
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/packages/remix-server-runtime/headers.ts b/packages/remix-server-runtime/headers.ts
index 8ecb102143d..bf05eea6635 100644
--- a/packages/remix-server-runtime/headers.ts
+++ b/packages/remix-server-runtime/headers.ts
@@ -37,38 +37,31 @@ export function getDocumentHeaders(
 }
 
 export function getDocumentHeadersRR(
+  build: ServerBuild,
   context: StaticHandlerContext,
   matches: RouteMatch<ServerRoute>[]
 ) {
-  let parentHeaders = new Headers();
-
-  for (let match of matches) {
-    let id = match.route.id;
-    let { headers } = match.route.module || {};
-    if (!headers) continue;
-
+  return matches.reduce((parentHeaders, match, index) => {
+    let { id } = match.route;
+    let routeModule = build.routes[id].module;
     let loaderHeaders = context.loaderHeaders?.[id] || new Headers();
     let actionHeaders = context.actionHeaders?.[id] || new Headers();
-
-    let newHeaders = new Headers(
-      typeof headers === "function"
-        ? headers({
-            loaderHeaders,
-            actionHeaders,
-            parentHeaders,
-          })
-        : headers
+    let headers = new Headers(
+      routeModule.headers
+        ? typeof routeModule.headers === "function"
+          ? routeModule.headers({ loaderHeaders, parentHeaders, actionHeaders })
+          : routeModule.headers
+        : undefined
     );
 
     // Automatically preserve Set-Cookie headers that were set either by the
     // loader or by a parent route.
-    prependCookies(actionHeaders, newHeaders);
-    prependCookies(loaderHeaders, newHeaders);
-    prependCookies(parentHeaders, newHeaders);
-    parentHeaders = newHeaders;
-  }
+    prependCookies(actionHeaders, headers);
+    prependCookies(loaderHeaders, headers);
+    prependCookies(parentHeaders, headers);
 
-  return parentHeaders;
+    return headers;
+  }, new Headers());
 }
 
 function prependCookies(parentHeaders: Headers, childHeaders: Headers): void {
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 2d8a0fc029d..2b7c4bf07ed 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -368,7 +368,7 @@ async function handleDocumentRequestRR(
     }
   }
 
-  let headers = getDocumentHeadersRR(context, renderableMatches);
+  let headers = getDocumentHeadersRR(build, context, renderableMatches);
 
   let serverHandoff: Pick<
     EntryContext,

From ebcc9370d499550fefdd8a9671cdd6608b789d68 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 19 Oct 2022 16:29:57 -0400
Subject: [PATCH 04/30] temp work on error/catch boundary distinction

---
 packages/remix-server-runtime/server.ts | 75 +++++++++++++++++++------
 1 file changed, 57 insertions(+), 18 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 2b7c4bf07ed..16afdbd765c 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -1,5 +1,6 @@
 // TODO: RRR - Change import to @remix-run/router
-import type { StaticHandler } from "./router";
+import type { StaticHandler, StaticHandlerContext } from "./router";
+import { isRouteErrorResponse } from "./router";
 import { unstable_createStaticHandler } from "./router";
 import type { AppLoadContext } from "./data";
 import { callRouteAction, callRouteLoader, extractData } from "./data";
@@ -301,6 +302,47 @@ async function handleDataRequestRR(
   }
 }
 
+function findParentBoundary(
+  routes: ServerRouteManifest,
+  routeId: string,
+  error: any
+): string {
+  let route = routes[routeId];
+  let isCatch = isRouteErrorResponse(error);
+  if (
+    (isCatch && route.module.CatchBoundary) ||
+    (!isCatch && route.module.ErrorBoundary) ||
+    !route.parentId
+  ) {
+    return routeId;
+  }
+
+  return findParentBoundary(routes, route.parentId, error);
+}
+
+// Re-generate a remix-friendly context.errors structure.  The Router only
+// handles generic errors and does not distinguish error versus catch.  We
+// may have a thrown response tagged to a route that only exports an
+// ErrorBoundary or vice versa.  So we adjust here and ensure that
+// data-loading errors are properly associated with routes that have the right
+// type of boundaries.
+export function differentiateCatchVersusErrorBoundaries(
+  build: ServerBuild,
+  context: StaticHandlerContext
+) {
+  // TODO: This may have issues if multiple things throw?  We may need to walk
+  // bottom up doing this tso higher errors take precedence?
+  if (context.errors) {
+    let errors: Record<string, any> = {};
+    for (let routeId of Object.keys(context.errors)) {
+      let error = context.errors[routeId];
+      let handlingRouteId = findParentBoundary(build.routes, routeId, error);
+      errors[handlingRouteId] = error;
+    }
+    context.errors = errors;
+  }
+}
+
 async function handleDocumentRequestRR(
   serverMode: ServerMode,
   build: ServerBuild,
@@ -313,6 +355,10 @@ async function handleDocumentRequestRR(
     return context;
   }
 
+  // This should properly restructure context.errors to the right Catch/Error
+  // Boundary.
+  differentiateCatchVersusErrorBoundaries(build, context);
+
   let appState: AppState = {
     trackBoundaries: true,
     trackCatchBoundaries: true,
@@ -321,30 +367,23 @@ async function handleDocumentRequestRR(
     loaderBoundaryRouteId: null,
   };
 
+  // Copy staticContext.errors to catch/error boundaries in appState
   for (let match of context.matches) {
     let route = match.route as ServerRoute;
     let id = route.id;
     let error = context.errors?.[id];
 
-    if (error) {
-      appState.error = await serializeError(error);
-      appState.loaderBoundaryRouteId = id;
+    if (!error) {
+      continue;
+    } else if (isRouteErrorResponse(error)) {
+      appState.catchBoundaryRouteId = id;
+      appState.trackCatchBoundaries = false;
+      appState.catch = error;
       break;
-    }
-
-    let actionHeaders = context.actionHeaders?.[id];
-    let loaderHeaders = context.loaderHeaders?.[id];
-    if (
-      actionHeaders?.has("X-Remix-Catch") ||
-      loaderHeaders?.has("X-Remix-Catch")
-    ) {
+    } else {
       appState.loaderBoundaryRouteId = id;
-      appState.catch = {
-        data: context.actionData?.[id] || context.loaderData?.[id],
-        status: context.statusCode,
-        // TODO: Populate this.
-        statusText: "",
-      };
+      appState.trackBoundaries = false;
+      appState.error = await serializeError(error);
       break;
     }
   }

From 0e84d2fa5c47007fcf942872de72730c02619d3a Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 19 Oct 2022 15:00:59 -0700
Subject: [PATCH 05/30] chore: update test to be more reliable add missing type
 import

---
 integration/transition-test.ts          | 3 +++
 packages/remix-server-runtime/server.ts | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/integration/transition-test.ts b/integration/transition-test.ts
index 20f020eeac1..63bd78d2eb9 100644
--- a/integration/transition-test.ts
+++ b/integration/transition-test.ts
@@ -213,6 +213,7 @@ test.describe("rendering", () => {
     await app.goto("/");
     let responses = app.collectDataResponses();
     await app.clickLink(`/${PAGE}`);
+    await page.waitForLoadState("networkidle");
 
     expect(
       responses.map((res) => new URL(res.url()).searchParams.get("_data"))
@@ -227,6 +228,7 @@ test.describe("rendering", () => {
     await app.goto(`/${PAGE}`);
     let responses = app.collectDataResponses();
     await app.clickLink(`/${PAGE}/${CHILD}`);
+    await page.waitForLoadState("networkidle");
 
     expect(
       responses.map((res) => new URL(res.url()).searchParams.get("_data"))
@@ -244,6 +246,7 @@ test.describe("rendering", () => {
 
     await app.clickLink(`/${REDIRECT}`);
     await page.waitForURL(/\/page/);
+    await page.waitForLoadState("networkidle");
 
     expect(
       responses
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 16afdbd765c..2779ae63635 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -14,7 +14,7 @@ import invariant from "./invariant";
 import { ServerMode, isServerMode } from "./mode";
 import type { RouteMatch } from "./routeMatching";
 import { matchServerRoutes } from "./routeMatching";
-import type { ServerRoute } from "./routes";
+import type { ServerRoute, ServerRouteManifest } from "./routes";
 import { createStaticHandlerDataRoutes, createRoutes } from "./routes";
 import { json, isRedirectResponse, isCatchResponse } from "./responses";
 import { createServerHandoffString } from "./serverHandoff";

From 52a6fc3b95060b9fe29864a5e4aef0d56f095a1b Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 19 Oct 2022 15:09:59 -0700
Subject: [PATCH 06/30] be more protective when accessing route module

---
 packages/remix-server-runtime/server.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 2779ae63635..9975d199f4b 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -310,8 +310,8 @@ function findParentBoundary(
   let route = routes[routeId];
   let isCatch = isRouteErrorResponse(error);
   if (
-    (isCatch && route.module.CatchBoundary) ||
-    (!isCatch && route.module.ErrorBoundary) ||
+    (isCatch && route.module?.CatchBoundary) ||
+    (!isCatch && route.module?.ErrorBoundary) ||
     !route.parentId
   ) {
     return routeId;
@@ -397,7 +397,7 @@ async function handleDocumentRequestRR(
     renderableMatches = [];
 
     let root = staticHandler.dataRoutes[0] as ServerRoute;
-    if (root?.module.CatchBoundary) {
+    if (root?.module?.CatchBoundary) {
       appState.catchBoundaryRouteId = "root";
       renderableMatches.push({
         params: {},

From 2b5140f658e76778f76a98ec9b469d9aba81b691 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 19 Oct 2022 15:15:04 -0700
Subject: [PATCH 07/30] be more protective when accessing route module

---
 packages/remix-server-runtime/server.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 9975d199f4b..d31370192e1 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -310,8 +310,8 @@ function findParentBoundary(
   let route = routes[routeId];
   let isCatch = isRouteErrorResponse(error);
   if (
-    (isCatch && route.module?.CatchBoundary) ||
-    (!isCatch && route.module?.ErrorBoundary) ||
+    (isCatch && route?.module?.CatchBoundary) ||
+    (!isCatch && route?.module?.ErrorBoundary) ||
     !route.parentId
   ) {
     return routeId;

From 7a6045f7dcae2c45a6038b23b3daa9869704ffe3 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 19 Oct 2022 15:22:05 -0700
Subject: [PATCH 08/30] revert

---
 packages/remix-server-runtime/server.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index d31370192e1..9975d199f4b 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -310,8 +310,8 @@ function findParentBoundary(
   let route = routes[routeId];
   let isCatch = isRouteErrorResponse(error);
   if (
-    (isCatch && route?.module?.CatchBoundary) ||
-    (!isCatch && route?.module?.ErrorBoundary) ||
+    (isCatch && route.module?.CatchBoundary) ||
+    (!isCatch && route.module?.ErrorBoundary) ||
     !route.parentId
   ) {
     return routeId;

From 4a8997ed9f6353d33ed4d6e8b8db3d8dcb7ba637 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Thu, 20 Oct 2022 11:50:36 -0400
Subject: [PATCH 09/30] Get catch boundary integration tests passing

---
 packages/remix-server-runtime/data.ts         |  5 +-
 .../remix-server-runtime/router/router.ts     | 64 ++++++++++++++-----
 packages/remix-server-runtime/server.ts       | 32 ++++++++--
 3 files changed, 78 insertions(+), 23 deletions(-)

diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index 205e71db667..084de46eb9b 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -34,7 +34,10 @@ export async function callRouteAction({
   isRemixRouterRequest?: boolean;
 }) {
   if (!action) {
-    let response = new Response(null, { status: 405 });
+    let response = new Response(null, {
+      status: 405,
+      statusText: "Method Not Allowed",
+    });
     response.headers.set("X-Remix-Catch", "yes");
     return response;
   }
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 2e2a420da0f..9c19d77bdd8 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -1767,6 +1767,9 @@ export function createRouter(init: RouterInit): Router {
 //#region createStaticHandler
 ////////////////////////////////////////////////////////////////////////////////
 
+const validActionMethods = new Set(["POST", "PUT", "PATCH", "DELETE"]);
+const validRequestMethods = new Set(["GET", "HEAD", ...validActionMethods]);
+
 export function unstable_createStaticHandler(
   routes: AgnosticRouteObject[]
 ): StaticHandler {
@@ -1803,7 +1806,25 @@ export function unstable_createStaticHandler(
     let location = createLocation("", createPath(url), null, "default");
     let matches = matchRoutes(dataRoutes, location);
 
-    if (!matches) {
+    if (!validRequestMethods.has(request.method)) {
+      let {
+        matches: methodNotAllowedMatches,
+        route,
+        error,
+      } = getMethodNotAllowedMatches(dataRoutes);
+      return {
+        location,
+        matches: methodNotAllowedMatches,
+        loaderData: {},
+        actionData: null,
+        errors: {
+          [route.id]: error,
+        },
+        statusCode: error.status,
+        loaderHeaders: {},
+        actionHeaders: {},
+      };
+    } else if (!matches) {
       let {
         matches: notFoundMatches,
         route,
@@ -1817,7 +1838,7 @@ export function unstable_createStaticHandler(
         errors: {
           [route.id]: error,
         },
-        statusCode: 404,
+        statusCode: error.status,
         loaderHeaders: {},
         actionHeaders: {},
       };
@@ -1856,7 +1877,12 @@ export function unstable_createStaticHandler(
     let location = createLocation("", createPath(url), null, "default");
     let matches = matchRoutes(dataRoutes, location);
 
-    if (!matches) {
+    if (!validRequestMethods.has(request.method)) {
+      throw createRouterErrorResponse(null, {
+        status: 405,
+        statusText: "Method Not Allowed",
+      });
+    } else if (!matches) {
       throw createRouterErrorResponse(null, {
         status: 404,
         statusText: "Not Found",
@@ -1905,7 +1931,7 @@ export function unstable_createStaticHandler(
     );
 
     try {
-      if (request.method !== "GET" && request.method !== "HEAD") {
+      if (validActionMethods.has(request.method)) {
         let result = await submit(
           request,
           matches,
@@ -1952,7 +1978,7 @@ export function unstable_createStaticHandler(
     if (!actionMatch.route.action) {
       let href = createHref(new URL(request.url));
       if (isRouteRequest) {
-        throw createRouterErrorResponse(`No action found for [${href}]`, {
+        throw createRouterErrorResponse(null, {
           status: 405,
           statusText: "Method Not Allowed",
         });
@@ -2724,16 +2750,18 @@ function findNearestBoundary(
   );
 }
 
-function getNotFoundMatches(routes: AgnosticDataRouteObject[]): {
+function getShortCircuitMatches(
+  routes: AgnosticDataRouteObject[],
+  status: number,
+  statusText: string
+): {
   matches: AgnosticDataRouteMatch[];
   route: AgnosticDataRouteObject;
   error: ErrorResponse;
 } {
   // Prefer a root layout route if present, otherwise shim in a route object
-  let route = routes.find(
-    (r) => r.index || r.path === "" || r.path === "/"
-  ) || {
-    id: "__shim-404-route__",
+  let route = routes.find((r) => r.index || !r.path || r.path === "/") || {
+    id: `__shim-${status}-route__`,
   };
 
   return {
@@ -2746,10 +2774,18 @@ function getNotFoundMatches(routes: AgnosticDataRouteObject[]): {
       },
     ],
     route,
-    error: new ErrorResponse(404, "Not Found", null),
+    error: new ErrorResponse(status, statusText, null),
   };
 }
 
+function getNotFoundMatches(routes: AgnosticDataRouteObject[]) {
+  return getShortCircuitMatches(routes, 404, "Not Found");
+}
+
+function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) {
+  return getShortCircuitMatches(routes, 405, "Method Not Allowed");
+}
+
 function getMethodNotAllowedResult(path: Location | string): ErrorResult {
   let href = typeof path === "string" ? path : createHref(path);
   console.warn(
@@ -2759,11 +2795,7 @@ function getMethodNotAllowedResult(path: Location | string): ErrorResult {
   );
   return {
     type: ResultType.error,
-    error: new ErrorResponse(
-      405,
-      "Method Not Allowed",
-      `No action found for [${href}]`
-    ),
+    error: new ErrorResponse(405, "Method Not Allowed", ""),
   };
 }
 
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 9975d199f4b..b48a1f2b16d 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -287,6 +287,16 @@ async function handleDataRequestRR(
     return response;
   } catch (error) {
     if (error instanceof Response) {
+      // Our callRouteLoader/callRouteAction methods ensure we always have
+      // X-Remix-Catch on responses thrown from loaders and actions.  However,
+      // if we never make it to the loader (such as a 404 or 405) then we
+      // Remix Router puts it's own custom header on, which we swap here for
+      // X-Remix-Catch
+      if (error.headers.get("X-Remix-Router-Error") === "yes") {
+        error.headers.delete("X-Remix-Router-Error");
+        error.headers.set("X-Remix-Catch", "yes");
+      }
+
       return error;
     }
 
@@ -307,14 +317,18 @@ function findParentBoundary(
   routeId: string,
   error: any
 ): string {
-  let route = routes[routeId];
+  // Fall back to the root route if we don't match any routes, since Remix
+  // has default error/catch boundary handling.  This handles the case where
+  // react-router doesn't have a matching "root" route to assign the error to
+  // so it returns context.errors = { __shim-404-route__: ErrorResponse }
+  let route = routes[routeId] || routes["root"];
   let isCatch = isRouteErrorResponse(error);
   if (
-    (isCatch && route.module?.CatchBoundary) ||
-    (!isCatch && route.module?.ErrorBoundary) ||
+    (isCatch && route.module.CatchBoundary) ||
+    (!isCatch && route.module.ErrorBoundary) ||
     !route.parentId
   ) {
-    return routeId;
+    return route.id;
   }
 
   return findParentBoundary(routes, route.parentId, error);
@@ -378,7 +392,12 @@ async function handleDocumentRequestRR(
     } else if (isRouteErrorResponse(error)) {
       appState.catchBoundaryRouteId = id;
       appState.trackCatchBoundaries = false;
-      appState.catch = error;
+      appState.catch = {
+        // Order is important for response matching assertions!
+        data: error.data,
+        status: error.status,
+        statusText: error.statusText,
+      };
       break;
     } else {
       appState.loaderBoundaryRouteId = id;
@@ -527,8 +546,9 @@ async function handleDocumentRequest({
         );
         appState.trackCatchBoundaries = false;
         appState.catch = {
-          ...actionStatus,
           data: await extractData(actionResponse),
+          status: actionStatus.status,
+          statusText: actionStatus.statusText,
         };
       } else {
         actionData = {

From af9d4505ab722152aec674dd54a593376d55af0b Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Thu, 20 Oct 2022 17:29:36 -0400
Subject: [PATCH 10/30] Get error boundary tests passing

---
 packages/remix-server-runtime/server.ts | 61 ++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index b48a1f2b16d..d4218fedc64 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -400,7 +400,15 @@ async function handleDocumentRequestRR(
       };
       break;
     } else {
-      appState.loaderBoundaryRouteId = id;
+      // Only assign the boundary id if this module has a boundary.  This
+      // should be true in almost all cases, except for cases in which no
+      // boundaries exist and the router "assigns" the error to the root route
+      // for lack of a better place to put it.  If the root doesn't have an
+      // error boundary, then we leave loaderBoundaryId null to bubble to the
+      // default boundary at render time
+      if (build.routes[id]?.module.ErrorBoundary) {
+        appState.loaderBoundaryRouteId = id;
+      }
       appState.trackBoundaries = false;
       appState.error = await serializeError(error);
       break;
@@ -1010,7 +1018,23 @@ async function assertResponsesMatch(_a: Response, _b: Response) {
     assert(
       a,
       b,
-      (r) => r.text(),
+      async (r) => {
+        let text = await r.text();
+
+        // Strip stack trace from default error boundary HTML
+        if (
+          text.includes(">Application Error</h1>") &&
+          text.includes("💿 Hey developer👋")
+        ) {
+          return stripStackTraceFromDefaultErrorBoundary(text);
+        }
+
+        if (text.includes("<script>window.__remixContext")) {
+          return stripStackTraceFromRemixContext(text);
+        }
+
+        return text;
+      },
       "Non-JSON response body did not match!\nResponse 1:\n" +
         (await a.clone().text()) +
         "\nResponse 2:\n" +
@@ -1019,6 +1043,39 @@ async function assertResponsesMatch(_a: Response, _b: Response) {
   }
 }
 
+function stripStackTraceFromDefaultErrorBoundary(text: string) {
+  let openPreStart = text.indexOf("<pre ");
+  let openPreEnd = text.indexOf('">', openPreStart + 1);
+  let closePre = text.indexOf("</pre>", openPreEnd + 1);
+  let error = text.substring(openPreEnd + 2, closePre);
+  let stackStart = error.indexOf("\n");
+  let errorWithoutStack = error.substring(0, stackStart);
+  return (
+    text.replace(error, "ERROR REMOVED") + "\n\nError:\n" + errorWithoutStack
+  );
+}
+
+function stripStackTraceFromRemixContext(text: string) {
+  let scriptStart = text.indexOf("<script>window.__remixContext");
+  let scriptEnd = text.indexOf("</script>", scriptStart + 1);
+  let scriptContents = text.substring(scriptStart, scriptEnd + 9);
+  let remixContext = JSON.parse(
+    scriptContents
+      .replace("<script>window.__remixContext = ", "")
+      .replace(";</script>", "")
+  );
+  // If we didn't have an error - assert the full document
+  if (!remixContext?.appState?.error?.stack) {
+    return text;
+  }
+
+  // Otherwise remove the stack trace and assert the text/JSON as a
+  // concatenated string
+  remixContext.appState.error.stack = "yes";
+  text = text.replace(scriptContents, "CONTEXT REMOVED");
+  return text + "\n\nContext:\n" + JSON.stringify(remixContext);
+}
+
 function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) {
   if (serverMode !== ServerMode.Test) {
     console.error(error);

From ad09bef60b443aa6b00c4eeacbde71701df3be96 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Thu, 20 Oct 2022 20:59:34 -0700
Subject: [PATCH 11/30] test: mdx test ignores non-existing asset request test:
 stop compiler test from logging

---
 integration/compiler-test.ts | 13 +++++++++++++
 integration/mdx-test.ts      | 11 +++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/integration/compiler-test.ts b/integration/compiler-test.ts
index 4e4cdb5710a..3c7515f6956 100644
--- a/integration/compiler-test.ts
+++ b/integration/compiler-test.ts
@@ -364,6 +364,19 @@ test.describe("compiler", () => {
   });
 
   test.describe("serverBareModulesPlugin", () => {
+    let ogConsole: typeof global.console;
+    test.beforeEach(() => {
+      ogConsole = global.console;
+      // @ts-ignore
+      global.console = {
+        log() {},
+        warn() {},
+        error() {},
+      };
+    });
+    test.afterEach(() => {
+      global.console = ogConsole;
+    });
     test("warns when a module isn't installed", async () => {
       let buildOutput: string;
       let buildStdio = new PassThrough();
diff --git a/integration/mdx-test.ts b/integration/mdx-test.ts
index c93b9bb71a4..0263f1bc2fd 100644
--- a/integration/mdx-test.ts
+++ b/integration/mdx-test.ts
@@ -17,7 +17,6 @@ test.describe("mdx", () => {
     fixture = await createFixture({
       files: {
         "app/root.jsx": js`
-        import { json } from "@remix-run/node";
         import { Links, Meta, Outlet, Scripts } from "@remix-run/react";
 
           export default function Root() {
@@ -37,10 +36,9 @@ test.describe("mdx", () => {
         `,
 
         "app/routes/blog.jsx": js`
-          import { json } from "@remix-run/node";
           import { useMatches, Outlet } from "@remix-run/react";
 
-          export default function Index() {
+          export default function Blog() {
             const matches = useMatches();
             const mdxMatch = matches[matches.length - 1];
             return (
@@ -105,9 +103,14 @@ export function ComponentUsingData() {
     expect(await app.getHtml()).toMatch("This is some basic markdown!");
   });
 
-  test("supports links, meta, headers, handle, and loader", async ({
+  test.only("supports links, meta, headers, handle, and loader", async ({
     page,
   }) => {
+    // block requests for the non-existing app.css requested by test
+    page.route("**/app.css", (route) => {
+      route.abort();
+    });
+
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/blog/post");
     expect(await app.getHtml('meta[name="description"]')).toMatch(

From 2f44df4967fa4c39e2d5e7fd47fbd40f8c163a59 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Thu, 20 Oct 2022 21:13:54 -0700
Subject: [PATCH 12/30] fix: handle no root catch boundary test: revert mdx
 test to cover failure case

---
 integration/mdx-test.ts                 |  5 -----
 packages/remix-server-runtime/server.ts | 10 ++++++++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/integration/mdx-test.ts b/integration/mdx-test.ts
index 0263f1bc2fd..3d1f32f546e 100644
--- a/integration/mdx-test.ts
+++ b/integration/mdx-test.ts
@@ -106,11 +106,6 @@ export function ComponentUsingData() {
   test.only("supports links, meta, headers, handle, and loader", async ({
     page,
   }) => {
-    // block requests for the non-existing app.css requested by test
-    page.route("**/app.css", (route) => {
-      route.abort();
-    });
-
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/blog/post");
     expect(await app.getHtml('meta[name="description"]')).toMatch(
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index d4218fedc64..ff9dcff32b4 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -415,6 +415,16 @@ async function handleDocumentRequestRR(
     }
   }
 
+  // If we didn't find a catch boundary we default to the root route,
+  // and if it doesn't have a catch boundary, we fallback to the built-in
+  // catch boundary
+  if (
+    appState.catchBoundaryRouteId === "root" &&
+    !build.routes.root?.module?.CatchBoundary
+  ) {
+    appState.catchBoundaryRouteId = null;
+  }
+
   let renderableMatches = getRenderableMatches(
     context.matches as unknown as RouteMatch<ServerRoute>[],
     appState

From 8c12f0a659a5cc42b58342f8d11d99065a8d181a Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Thu, 20 Oct 2022 21:18:06 -0700
Subject: [PATCH 13/30] remove ".only"

---
 integration/mdx-test.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/integration/mdx-test.ts b/integration/mdx-test.ts
index 3d1f32f546e..8c0a6d635ae 100644
--- a/integration/mdx-test.ts
+++ b/integration/mdx-test.ts
@@ -103,7 +103,7 @@ export function ComponentUsingData() {
     expect(await app.getHtml()).toMatch("This is some basic markdown!");
   });
 
-  test.only("supports links, meta, headers, handle, and loader", async ({
+  test("supports links, meta, headers, handle, and loader", async ({
     page,
   }) => {
     let app = new PlaywrightFixture(appFixture, page);

From 8af6805aebf8e041d44334e3d434fbe7a61894ec Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 21 Oct 2022 16:51:38 -0400
Subject: [PATCH 14/30] Updates

---
 integration/error-boundary-test.ts      |  7 +++++++
 packages/remix-server-runtime/server.ts | 26 +++++++++----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index 92c94327e03..b8328842668 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -476,5 +476,12 @@ test.describe("ErrorBoundary", () => {
       await app.clickSubmitButton(NO_ROOT_BOUNDARY_ACTION_RETURN);
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
+
+    test.skip(
+      "returns a 500 x-remix-error on a data fetch to a path with no loader"
+    );
+    test.skip("returns a 405 x-remix-error on a data fetch with a bad method");
+    test.skip("returns a 404 x-remix-error on a data fetch to a bad path");
+    test.skip("returns a 403 x-remix-error on a data fetch to a bad routeId");
   });
 });
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index ff9dcff32b4..5f5bb675943 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -382,6 +382,12 @@ async function handleDocumentRequestRR(
   };
 
   // Copy staticContext.errors to catch/error boundaries in appState
+  // Note: Only assign the boundary id if this module has a boundary.  This
+  // should be true in almost all cases, except for cases in which no
+  // boundaries exist and the router "assigns" the catch/error to the root
+  // route for lack of a better place to put it.  If the root doesn't have a
+  // catch/error boundary, then we leave the boundaryId null to bubble to the
+  // default boundary at render time
   for (let match of context.matches) {
     let route = match.route as ServerRoute;
     let id = route.id;
@@ -390,7 +396,9 @@ async function handleDocumentRequestRR(
     if (!error) {
       continue;
     } else if (isRouteErrorResponse(error)) {
-      appState.catchBoundaryRouteId = id;
+      if (build.routes[id]?.module.CatchBoundary) {
+        appState.catchBoundaryRouteId = id;
+      }
       appState.trackCatchBoundaries = false;
       appState.catch = {
         // Order is important for response matching assertions!
@@ -400,12 +408,6 @@ async function handleDocumentRequestRR(
       };
       break;
     } else {
-      // Only assign the boundary id if this module has a boundary.  This
-      // should be true in almost all cases, except for cases in which no
-      // boundaries exist and the router "assigns" the error to the root route
-      // for lack of a better place to put it.  If the root doesn't have an
-      // error boundary, then we leave loaderBoundaryId null to bubble to the
-      // default boundary at render time
       if (build.routes[id]?.module.ErrorBoundary) {
         appState.loaderBoundaryRouteId = id;
       }
@@ -415,16 +417,6 @@ async function handleDocumentRequestRR(
     }
   }
 
-  // If we didn't find a catch boundary we default to the root route,
-  // and if it doesn't have a catch boundary, we fallback to the built-in
-  // catch boundary
-  if (
-    appState.catchBoundaryRouteId === "root" &&
-    !build.routes.root?.module?.CatchBoundary
-  ) {
-    appState.catchBoundaryRouteId = null;
-  }
-
   let renderableMatches = getRenderableMatches(
     context.matches as unknown as RouteMatch<ServerRoute>[],
     appState

From b25d64f81b7129ad1bc9dde0bfce08f6653126e6 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 21 Oct 2022 17:26:31 -0400
Subject: [PATCH 15/30] Copy over latest @remix-run/router files

---
 .../remix-server-runtime/router/history.ts    |   4 +-
 .../remix-server-runtime/router/router.ts     | 234 +++++++++++++-----
 packages/remix-server-runtime/router/utils.ts |  59 ++++-
 3 files changed, 232 insertions(+), 65 deletions(-)

diff --git a/packages/remix-server-runtime/router/history.ts b/packages/remix-server-runtime/router/history.ts
index 7cb1dc95dcf..6fce1725d31 100644
--- a/packages/remix-server-runtime/router/history.ts
+++ b/packages/remix-server-runtime/router/history.ts
@@ -572,7 +572,7 @@ function getUrlBasedHistory(
     }
 
     if (v5Compat && listener) {
-      listener({ action, location });
+      listener({ action, location: history.location });
     }
   }
 
@@ -586,7 +586,7 @@ function getUrlBasedHistory(
     globalHistory.replaceState(historyState, "", url);
 
     if (v5Compat && listener) {
-      listener({ action, location: location });
+      listener({ action, location: history.location });
     }
   }
 
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 9c19d77bdd8..f82242063fb 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -27,9 +27,12 @@ import {
   ErrorResponse,
   ResultType,
   convertRoutesToDataRoutes,
+  getPathContributingMatches,
   invariant,
   isRouteErrorResponse,
+  joinPaths,
   matchRoutes,
+  resolveTo,
 } from "./utils";
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -471,14 +474,25 @@ interface HandleLoadersResult extends ShortCircuitable {
 }
 
 /**
- * Tuple of [key, href, DataRouterMatch] for a revalidating fetcher.load()
+ * Tuple of [key, href, DataRouteMatch, DataRouteMatch[]] for a revalidating
+ * fetcher.load()
  */
-type RevalidatingFetcher = [string, string, AgnosticDataRouteMatch];
+type RevalidatingFetcher = [
+  string,
+  string,
+  AgnosticDataRouteMatch,
+  AgnosticDataRouteMatch[]
+];
 
 /**
- * Tuple of [href, DataRouteMatch] for an active fetcher.load()
+ * Tuple of [href, DataRouteMatch, DataRouteMatch[]] for an active
+ * fetcher.load()
  */
-type FetchLoadMatch = [string, AgnosticDataRouteMatch];
+type FetchLoadMatch = [
+  string,
+  AgnosticDataRouteMatch,
+  AgnosticDataRouteMatch[]
+];
 
 /**
  * Wrapper object to allow us to throw any response out from callLoaderOrAction
@@ -507,6 +521,12 @@ export const IDLE_FETCHER: FetcherStates["Idle"] = {
   formEncType: undefined,
   formData: undefined,
 };
+
+const isBrowser =
+  typeof window !== "undefined" &&
+  typeof window.document !== "undefined" &&
+  typeof window.document.createElement !== "undefined";
+const isServer = !isBrowser;
 //#endregion
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -745,6 +765,20 @@ export function createRouter(init: RouterInit): Router {
     let { path, submission, error } = normalizeNavigateOptions(to, opts);
 
     let location = createLocation(state.location, path, opts && opts.state);
+
+    // When using navigate as a PUSH/REPLACE we aren't reading an already-encoded
+    // URL from window.location, so we need to encode it here so the behavior
+    // remains the same as POP and non-data-router usages.  new URL() does all
+    // the same encoding we'd get from a history.pushState/window.location read
+    // without having to touch history
+    let url = createURL(createPath(location));
+    location = {
+      ...location,
+      pathname: url.pathname,
+      search: url.search,
+      hash: url.hash,
+    };
+
     let historyAction =
       (opts && opts.replace) === true || submission != null
         ? HistoryAction.Replace
@@ -951,7 +985,13 @@ export function createRouter(init: RouterInit): Router {
     if (!actionMatch.route.action) {
       result = getMethodNotAllowedResult(location);
     } else {
-      result = await callLoaderOrAction("action", request, actionMatch);
+      result = await callLoaderOrAction(
+        "action",
+        request,
+        actionMatch,
+        matches,
+        router.basename
+      );
 
       if (request.signal.aborted) {
         return { shortCircuited: true };
@@ -1093,6 +1133,7 @@ export function createRouter(init: RouterInit): Router {
     let { results, loaderResults, fetcherResults } =
       await callLoadersAndMaybeResolveData(
         state.matches,
+        matches,
         matchesToLoad,
         revalidatingFetchers,
         request
@@ -1162,7 +1203,7 @@ export function createRouter(init: RouterInit): Router {
     href: string,
     opts?: RouterFetchOptions
   ) {
-    if (typeof AbortController === "undefined") {
+    if (isServer) {
       throw new Error(
         "router.fetch() was called during the server render, but it shouldn't be. " +
           "You are likely calling a useFetcher() method in the body of your component. " +
@@ -1182,14 +1223,14 @@ export function createRouter(init: RouterInit): Router {
     let match = getTargetMatch(matches, path);
 
     if (submission) {
-      handleFetcherAction(key, routeId, path, match, submission);
+      handleFetcherAction(key, routeId, path, match, matches, submission);
       return;
     }
 
     // Store off the match so we can call it's shouldRevalidate on subsequent
     // revalidations
-    fetchLoadMatches.set(key, [path, match]);
-    handleFetcherLoader(key, routeId, path, match);
+    fetchLoadMatches.set(key, [path, match, matches]);
+    handleFetcherLoader(key, routeId, path, match, matches);
   }
 
   // Call the action for the matched fetcher.submit(), and then handle redirects,
@@ -1199,6 +1240,7 @@ export function createRouter(init: RouterInit): Router {
     routeId: string,
     path: string,
     match: AgnosticDataRouteMatch,
+    requestMatches: AgnosticDataRouteMatch[],
     submission: Submission
   ) {
     interruptActiveLoads();
@@ -1225,7 +1267,13 @@ export function createRouter(init: RouterInit): Router {
     let fetchRequest = createRequest(path, abortController.signal, submission);
     fetchControllers.set(key, abortController);
 
-    let actionResult = await callLoaderOrAction("action", fetchRequest, match);
+    let actionResult = await callLoaderOrAction(
+      "action",
+      fetchRequest,
+      match,
+      requestMatches,
+      router.basename
+    );
 
     if (fetchRequest.signal.aborted) {
       // We can delete this so long as we weren't aborted by ou our own fetcher
@@ -1327,6 +1375,7 @@ export function createRouter(init: RouterInit): Router {
     let { results, loaderResults, fetcherResults } =
       await callLoadersAndMaybeResolveData(
         state.matches,
+        matches,
         matchesToLoad,
         revalidatingFetchers,
         revalidationRequest
@@ -1407,7 +1456,8 @@ export function createRouter(init: RouterInit): Router {
     key: string,
     routeId: string,
     path: string,
-    match: AgnosticDataRouteMatch
+    match: AgnosticDataRouteMatch,
+    matches: AgnosticDataRouteMatch[]
   ) {
     let existingFetcher = state.fetchers.get(key);
     // Put this fetcher into it's loading state
@@ -1429,7 +1479,9 @@ export function createRouter(init: RouterInit): Router {
     let result: DataResult = await callLoaderOrAction(
       "loader",
       fetchRequest,
-      match
+      match,
+      matches,
+      router.basename
     );
 
     // Deferred isn't supported or fetcher loads, await everything and treat it
@@ -1527,6 +1579,7 @@ export function createRouter(init: RouterInit): Router {
 
     let redirectHistoryAction =
       replace === true ? HistoryAction.Replace : HistoryAction.Push;
+
     await startNavigation(redirectHistoryAction, navigation.location, {
       overrideNavigation: navigation,
     });
@@ -1534,6 +1587,7 @@ export function createRouter(init: RouterInit): Router {
 
   async function callLoadersAndMaybeResolveData(
     currentMatches: AgnosticDataRouteMatch[],
+    matches: AgnosticDataRouteMatch[],
     matchesToLoad: AgnosticDataRouteMatch[],
     fetchersToLoad: RevalidatingFetcher[],
     request: Request
@@ -1542,9 +1596,17 @@ export function createRouter(init: RouterInit): Router {
     // then slice off the results into separate arrays so we can handle them
     // accordingly
     let results = await Promise.all([
-      ...matchesToLoad.map((m) => callLoaderOrAction("loader", request, m)),
-      ...fetchersToLoad.map(([, href, match]) =>
-        callLoaderOrAction("loader", createRequest(href, request.signal), match)
+      ...matchesToLoad.map((match) =>
+        callLoaderOrAction("loader", request, match, matches, router.basename)
+      ),
+      ...fetchersToLoad.map(([, href, match, fetchMatches]) =>
+        callLoaderOrAction(
+          "loader",
+          createRequest(href, request.signal),
+          match,
+          fetchMatches,
+          router.basename
+        )
       ),
     ]);
     let loaderResults = results.slice(0, matchesToLoad.length);
@@ -1751,7 +1813,9 @@ export function createRouter(init: RouterInit): Router {
     navigate,
     fetch,
     revalidate,
-    createHref,
+    // Passthrough to history-aware createHref used by useHref so we get proper
+    // hash-aware URLs in DOM paths
+    createHref: (to: To) => init.history.createHref(to),
     getFetcher,
     deleteFetcher,
     dispose,
@@ -1844,7 +1908,7 @@ export function unstable_createStaticHandler(
       };
     }
 
-    let result = await queryImpl(request, location, matches, false);
+    let result = await queryImpl(request, location, matches);
     if (result instanceof Response) {
       return result;
     }
@@ -1900,7 +1964,7 @@ export function unstable_createStaticHandler(
       });
     }
 
-    let result = await queryImpl(request, location, [match], true);
+    let result = await queryImpl(request, location, matches, match);
     if (result instanceof Response) {
       return result;
     }
@@ -1923,7 +1987,7 @@ export function unstable_createStaticHandler(
     request: Request,
     location: Location,
     matches: AgnosticDataRouteMatch[],
-    isRouteRequest: boolean
+    routeMatch?: AgnosticDataRouteMatch
   ): Promise<Omit<StaticHandlerContext, "location"> | Response> {
     invariant(
       request.signal,
@@ -1935,13 +1999,13 @@ export function unstable_createStaticHandler(
         let result = await submit(
           request,
           matches,
-          getTargetMatch(matches, location),
-          isRouteRequest
+          routeMatch || getTargetMatch(matches, location),
+          routeMatch != null
         );
         return result;
       }
 
-      let result = await loadRouteData(request, matches, isRouteRequest);
+      let result = await loadRouteData(request, matches, routeMatch);
       return result instanceof Response
         ? result
         : {
@@ -1976,7 +2040,7 @@ export function unstable_createStaticHandler(
   ): Promise<Omit<StaticHandlerContext, "location"> | Response> {
     let result: DataResult;
     if (!actionMatch.route.action) {
-      let href = createHref(new URL(request.url));
+      let href = createServerHref(new URL(request.url));
       if (isRouteRequest) {
         throw createRouterErrorResponse(null, {
           status: 405,
@@ -1989,6 +2053,8 @@ export function unstable_createStaticHandler(
         "action",
         request,
         actionMatch,
+        matches,
+        undefined, // Basename not currently supported in static handlers
         true,
         isRouteRequest
       );
@@ -2001,7 +2067,7 @@ export function unstable_createStaticHandler(
 
     if (isRedirectResult(result)) {
       // Uhhhh - this should never happen, we should always throw these from
-      // calLoaderOrAction, but the type narrowing here keeps TS happy and we
+      // callLoaderOrAction, but the type narrowing here keeps TS happy and we
       // can get back on the "throw all redirect responses" train here should
       // this ever happen :/
       throw new Response(null, {
@@ -2053,7 +2119,7 @@ export function unstable_createStaticHandler(
       // Store off the pending error - we use it to determine which loaders
       // to call and will commit it when we complete the navigation
       let boundaryMatch = findNearestBoundary(matches, actionMatch.route.id);
-      let context = await loadRouteData(request, matches, isRouteRequest, {
+      let context = await loadRouteData(request, matches, undefined, {
         [boundaryMatch.route.id]: result.error,
       });
 
@@ -2070,7 +2136,7 @@ export function unstable_createStaticHandler(
       };
     }
 
-    let context = await loadRouteData(request, matches, isRouteRequest);
+    let context = await loadRouteData(request, matches);
 
     return {
       ...context,
@@ -2088,16 +2154,20 @@ export function unstable_createStaticHandler(
   async function loadRouteData(
     request: Request,
     matches: AgnosticDataRouteMatch[],
-    isRouteRequest: boolean,
+    routeMatch?: AgnosticDataRouteMatch,
     pendingActionError?: RouteData
   ): Promise<
     | Omit<StaticHandlerContext, "location" | "actionData" | "actionHeaders">
     | Response
   > {
-    let matchesToLoad = getLoaderMatchesUntilBoundary(
-      matches,
-      Object.keys(pendingActionError || {})[0]
-    ).filter((m) => m.route.loader);
+    let isRouteRequest = routeMatch != null;
+    let requestMatches = routeMatch
+      ? [routeMatch]
+      : getLoaderMatchesUntilBoundary(
+          matches,
+          Object.keys(pendingActionError || {})[0]
+        );
+    let matchesToLoad = requestMatches.filter((m) => m.route.loader);
 
     // Short circuit if we have no loaders to run
     if (matchesToLoad.length === 0) {
@@ -2111,8 +2181,16 @@ export function unstable_createStaticHandler(
     }
 
     let results = await Promise.all([
-      ...matchesToLoad.map((m) =>
-        callLoaderOrAction("loader", request, m, true, isRouteRequest)
+      ...matchesToLoad.map((match) =>
+        callLoaderOrAction(
+          "loader",
+          request,
+          match,
+          matches,
+          undefined, // Basename not currently supported in static handlers
+          true,
+          isRouteRequest
+        )
       ),
     ]);
 
@@ -2212,7 +2290,7 @@ function normalizeNavigateOptions(
       path,
       submission: {
         formMethod: opts.formMethod,
-        formAction: createHref(parsePath(path)),
+        formAction: createServerHref(parsePath(path)),
         formEncType:
           (opts && opts.formEncType) || "application/x-www-form-urlencoded",
         formData: opts.formData,
@@ -2327,10 +2405,10 @@ function getMatchesToLoad(
   // Pick fetcher.loads that need to be revalidated
   let revalidatingFetchers: RevalidatingFetcher[] = [];
   fetchLoadMatches &&
-    fetchLoadMatches.forEach(([href, match], key) => {
+    fetchLoadMatches.forEach(([href, match, fetchMatches], key) => {
       // This fetcher was cancelled from a prior action submission - force reload
       if (cancelledFetcherLoads.includes(key)) {
-        revalidatingFetchers.push([key, href, match]);
+        revalidatingFetchers.push([key, href, match, fetchMatches]);
       } else if (isRevalidationRequired) {
         let shouldRevalidate = shouldRevalidateLoader(
           href,
@@ -2342,7 +2420,7 @@ function getMatchesToLoad(
           actionResult
         );
         if (shouldRevalidate) {
-          revalidatingFetchers.push([key, href, match]);
+          revalidatingFetchers.push([key, href, match, fetchMatches]);
         }
       }
     });
@@ -2436,7 +2514,9 @@ async function callLoaderOrAction(
   type: "loader" | "action",
   request: Request,
   match: AgnosticDataRouteMatch,
-  skipRedirects: boolean = false,
+  matches: AgnosticDataRouteMatch[],
+  basename: string | undefined,
+  isStaticRequest: boolean = false,
   isRouteRequest: boolean = false
 ): Promise<DataResult> {
   let resultType;
@@ -2467,28 +2547,46 @@ async function callLoaderOrAction(
   }
 
   if (result instanceof Response) {
-    // Process redirects
     let status = result.status;
-    let location = result.headers.get("Location");
 
-    // For SSR single-route requests, we want to hand Responses back directly
-    // without unwrapping.  Wer do this with the QueryRouteResponse wrapper
-    // interface so we can know whether it was returned or thrown
-    if (isRouteRequest) {
-      // eslint-disable-next-line no-throw-literal
-      throw {
-        type: resultType || ResultType.data,
-        response: result,
-      };
-    }
+    // Process redirects
+    if (status >= 300 && status <= 399) {
+      let location = result.headers.get("Location");
+      invariant(
+        location,
+        "Redirects returned/thrown from loaders/actions must have a Location header"
+      );
+
+      // Support relative routing in redirects
+      let activeMatches = matches.slice(0, matches.indexOf(match) + 1);
+      let routePathnames = getPathContributingMatches(activeMatches).map(
+        (match) => match.pathnameBase
+      );
+      let requestPath = createURL(request.url).pathname;
+      let resolvedLocation = resolveTo(location, routePathnames, requestPath);
+      invariant(
+        createPath(resolvedLocation),
+        `Unable to resolve redirect location: ${result.headers.get("Location")}`
+      );
+
+      // Prepend the basename to the redirect location if we have one
+      if (basename) {
+        let path = resolvedLocation.pathname;
+        resolvedLocation.pathname =
+          path === "/" ? basename : joinPaths([basename, path]);
+      }
 
-    if (status >= 300 && status <= 399 && location != null) {
-      // Don't process redirects in the router during SSR document requests.
+      location = createPath(resolvedLocation);
+
+      // Don't process redirects in the router during static requests requests.
       // Instead, throw the Response and let the server handle it with an HTTP
-      // redirect
-      if (skipRedirects) {
+      // redirect.  We also update the Location header in place in this flow so
+      // basename and relative routing is taken into account
+      if (isStaticRequest) {
+        result.headers.set("Location", location);
         throw result;
       }
+
       return {
         type: ResultType.redirect,
         status,
@@ -2497,6 +2595,17 @@ async function callLoaderOrAction(
       };
     }
 
+    // For SSR single-route requests, we want to hand Responses back directly
+    // without unwrapping.  We do this with the QueryRouteResponse wrapper
+    // interface so we can know whether it was returned or thrown
+    if (isRouteRequest) {
+      // eslint-disable-next-line no-throw-literal
+      throw {
+        type: resultType || ResultType.data,
+        response: result,
+      };
+    }
+
     let data: any;
     let contentType = result.headers.get("Content-Type");
     if (contentType && contentType.startsWith("application/json")) {
@@ -2787,7 +2896,7 @@ function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) {
 }
 
 function getMethodNotAllowedResult(path: Location | string): ErrorResult {
-  let href = typeof path === "string" ? path : createHref(path);
+  let href = typeof path === "string" ? path : createServerHref(path);
   console.warn(
     "You're trying to submit to a route that does not have an action.  To " +
       "fix this, please add an `action` function to the route for " +
@@ -2810,7 +2919,7 @@ function findRedirect(results: DataResult[]): RedirectResult | undefined {
 }
 
 // Create an href to represent a "server" URL without the hash
-function createHref(location: Partial<Path> | Location | URL) {
+function createServerHref(location: Partial<Path> | Location | URL) {
   return (location.pathname || "") + (location.search || "");
 }
 
@@ -2941,11 +3050,15 @@ function getTargetMatch(
     typeof location === "string" ? parsePath(location).search : location.search;
   if (
     matches[matches.length - 1].route.index &&
-    !hasNakedIndexQuery(search || "")
+    hasNakedIndexQuery(search || "")
   ) {
-    return matches.slice(-2)[0];
+    // Return the leaf index route when index is present
+    return matches[matches.length - 1];
   }
-  return matches.slice(-1)[0];
+  // Otherwise grab the deepest "path contributing" match (ignoring index and
+  // pathless layout routes)
+  let pathMatches = getPathContributingMatches(matches);
+  return pathMatches[pathMatches.length - 1];
 }
 
 function createURL(location: Location | string): URL {
@@ -2953,7 +3066,8 @@ function createURL(location: Location | string): URL {
     typeof window !== "undefined" && typeof window.location !== "undefined"
       ? window.location.origin
       : "unknown://unknown";
-  let href = typeof location === "string" ? location : createHref(location);
+  let href =
+    typeof location === "string" ? location : createServerHref(location);
   return new URL(href, base);
 }
 //#endregion
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index cb34165ac8b..3bedd7a71aa 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -331,7 +331,13 @@ export function matchRoutes<
 
   let matches = null;
   for (let i = 0; matches == null && i < branches.length; ++i) {
-    matches = matchRouteBranch<string, RouteObjectType>(branches[i], pathname);
+    matches = matchRouteBranch<string, RouteObjectType>(
+      branches[i],
+      // incoming pathnames are always encoded from either window.location or
+      // from route.navigate, but we want to match against the unencoded paths
+      // in the route definitions
+      safelyDecodeURI(pathname)
+    );
   }
 
   return matches;
@@ -535,7 +541,7 @@ export function generatePath<Path extends string>(
       return params[key]!;
     })
     .replace(/(\/?)\*/, (_, prefix, __, str) => {
-      let star = "*" as PathParam<Path>;
+      const star = "*" as PathParam<Path>;
 
       if (params[star] == null) {
         // If no splat was provided, trim the trailing slash _unless_ it's
@@ -704,6 +710,21 @@ function compilePath(
   return [matcher, paramNames];
 }
 
+function safelyDecodeURI(value: string) {
+  try {
+    return decodeURI(value);
+  } catch (error) {
+    warning(
+      false,
+      `The URL path "${value}" could not be decoded because it is is a ` +
+        `malformed URL segment. This is probably due to a bad percent ` +
+        `encoding (${error}).`
+    );
+
+    return value;
+  }
+}
+
 function safelyDecodeURIComponent(value: string, paramName: string) {
   try {
     return decodeURIComponent(value);
@@ -837,6 +858,38 @@ function getInvalidPathError(
   );
 }
 
+/**
+ * @private
+ *
+ * When processing relative navigation we want to ignore ancestor routes that
+ * do not contribute to the path, such that index/pathless layout routes don't
+ * interfere.
+ *
+ * For example, when moving a route element into an index route and/or a
+ * pathless layout route, relative link behavior contained within should stay
+ * the same.  Both of the following examples should link back to the root:
+ *
+ *   <Route path="/">
+ *     <Route path="accounts" element={<Link to=".."}>
+ *   </Route>
+ *
+ *   <Route path="/">
+ *     <Route path="accounts">
+ *       <Route element={<AccountsLayout />}>       // <-- Does not contribute
+ *         <Route index element={<Link to=".."} />  // <-- Does not contribute
+ *       </Route
+ *     </Route>
+ *   </Route>
+ */
+export function getPathContributingMatches<
+  T extends AgnosticRouteMatch = AgnosticRouteMatch
+>(matches: T[]) {
+  return matches.filter(
+    (match, index) =>
+      index === 0 || (match.route.path && match.route.path.length > 0)
+  );
+}
+
 /**
  * @private
  */
@@ -1074,7 +1127,7 @@ export class DeferredData {
       this.unlistenAbortSignal();
     }
 
-    let subscriber = this.subscriber;
+    const subscriber = this.subscriber;
     if (error) {
       Object.defineProperty(promise, "_error", { get: () => error });
       subscriber && subscriber(false);

From 32af5b6e4c377609f9606af9bcd1bb58b7ac1e48 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 26 Oct 2022 11:49:31 -0400
Subject: [PATCH 16/30] Bring over latest router files

---
 .../remix-server-runtime/router/history.ts    | 36 +++++++++++++++++++
 packages/remix-server-runtime/router/index.ts | 11 ++++--
 .../remix-server-runtime/router/router.ts     | 36 ++++++-------------
 packages/remix-server-runtime/router/utils.ts |  9 +++--
 4 files changed, 61 insertions(+), 31 deletions(-)

diff --git a/packages/remix-server-runtime/router/history.ts b/packages/remix-server-runtime/router/history.ts
index 6fce1725d31..e63e83026b1 100644
--- a/packages/remix-server-runtime/router/history.ts
+++ b/packages/remix-server-runtime/router/history.ts
@@ -127,6 +127,15 @@ export interface History {
    */
   createHref(to: To): string;
 
+  /**
+   * Encode a location the same way window.history would do (no-op for memory
+   * history) so we ensure our PUSH/REPLAC e navigations for data routers
+   * behave the same as POP
+   *
+   * @param location The incoming location from router.navigate()
+   */
+  encodeLocation(location: Location): Location;
+
   /**
    * Pushes a new location onto the history stack, increasing its length by one.
    * If there were any entries in the stack after the current one, they are
@@ -261,6 +270,9 @@ export function createMemoryHistory(
     createHref(to) {
       return typeof to === "string" ? to : createPath(to);
     },
+    encodeLocation(location) {
+      return location;
+    },
     push(to, state) {
       action = Action.Push;
       let nextLocation = createMemoryLocation(to, state);
@@ -529,6 +541,20 @@ export function parsePath(path: string): Partial<Path> {
   return parsedPath;
 }
 
+export function createURL(location: Location | string): URL {
+  // window.location.origin is "null" (the literal string value) in Firefox
+  // under certain conditions, notably when serving from a local HTML file
+  // See https://bugzilla.mozilla.org/show_bug.cgi?id=878297
+  let base =
+    typeof window !== "undefined" &&
+    typeof window.location !== "undefined" &&
+    window.location.origin !== "null"
+      ? window.location.origin
+      : "unknown://unknown";
+  let href = typeof location === "string" ? location : createPath(location);
+  return new URL(href, base);
+}
+
 export interface UrlHistory extends History {}
 
 export type UrlHistoryOptions = {
@@ -612,6 +638,16 @@ function getUrlBasedHistory(
     createHref(to) {
       return createHref(window, to);
     },
+    encodeLocation(location) {
+      // Encode a Location the same way window.location would
+      let url = createURL(createPath(location));
+      return {
+        ...location,
+        pathname: url.pathname,
+        search: url.search,
+        hash: url.hash,
+      };
+    },
     push,
     replace,
     go(n) {
diff --git a/packages/remix-server-runtime/router/index.ts b/packages/remix-server-runtime/router/index.ts
index cc57bff8489..b643dd55030 100644
--- a/packages/remix-server-runtime/router/index.ts
+++ b/packages/remix-server-runtime/router/index.ts
@@ -1,12 +1,16 @@
 // @ts-nocheck
 
-import { convertRoutesToDataRoutes } from "./utils";
+import { convertRoutesToDataRoutes, getPathContributingMatches } from "./utils";
 
 export type {
   ActionFunction,
   ActionFunctionArgs,
+  AgnosticDataIndexRouteObject,
+  AgnosticDataNonIndexRouteObject,
   AgnosticDataRouteMatch,
   AgnosticDataRouteObject,
+  AgnosticIndexRouteObject,
+  AgnosticNonIndexRouteObject,
   AgnosticRouteMatch,
   AgnosticRouteObject,
   TrackedPromise,
@@ -77,4 +81,7 @@ export * from "./router";
 ///////////////////////////////////////////////////////////////////////////////
 
 /** @internal */
-export { convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes };
+export {
+  convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes,
+  getPathContributingMatches as UNSAFE_getPathContributingMatches,
+};
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index f82242063fb..28a4ae764e3 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -1,10 +1,11 @@
 // @ts-nocheck
 // eslint-disable
-import type { History, Location, Path, To } from "./history";
+import type { History, Location, To } from "./history";
 import {
   Action as HistoryAction,
   createLocation,
   createPath,
+  createURL,
   parsePath,
 } from "./history";
 import type {
@@ -771,13 +772,7 @@ export function createRouter(init: RouterInit): Router {
     // remains the same as POP and non-data-router usages.  new URL() does all
     // the same encoding we'd get from a history.pushState/window.location read
     // without having to touch history
-    let url = createURL(createPath(location));
-    location = {
-      ...location,
-      pathname: url.pathname,
-      search: url.search,
-      hash: url.hash,
-    };
+    location = init.history.encodeLocation(location);
 
     let historyAction =
       (opts && opts.replace) === true || submission != null
@@ -2040,14 +2035,13 @@ export function unstable_createStaticHandler(
   ): Promise<Omit<StaticHandlerContext, "location"> | Response> {
     let result: DataResult;
     if (!actionMatch.route.action) {
-      let href = createServerHref(new URL(request.url));
       if (isRouteRequest) {
         throw createRouterErrorResponse(null, {
           status: 405,
           statusText: "Method Not Allowed",
         });
       }
-      result = getMethodNotAllowedResult(href);
+      result = getMethodNotAllowedResult(request.url);
     } else {
       result = await callLoaderOrAction(
         "action",
@@ -2290,7 +2284,7 @@ function normalizeNavigateOptions(
       path,
       submission: {
         formMethod: opts.formMethod,
-        formAction: createServerHref(parsePath(path)),
+        formAction: stripHashFromPath(path),
         formEncType:
           (opts && opts.formEncType) || "application/x-www-form-urlencoded",
         formData: opts.formData,
@@ -2646,7 +2640,7 @@ function createRequest(
   signal: AbortSignal,
   submission?: Submission
 ): Request {
-  let url = createURL(location).toString();
+  let url = createURL(stripHashFromPath(location)).toString();
   let init: RequestInit = { signal };
 
   if (submission) {
@@ -2896,7 +2890,7 @@ function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) {
 }
 
 function getMethodNotAllowedResult(path: Location | string): ErrorResult {
-  let href = typeof path === "string" ? path : createServerHref(path);
+  let href = typeof path === "string" ? path : createPath(path);
   console.warn(
     "You're trying to submit to a route that does not have an action.  To " +
       "fix this, please add an `action` function to the route for " +
@@ -2918,9 +2912,9 @@ function findRedirect(results: DataResult[]): RedirectResult | undefined {
   }
 }
 
-// Create an href to represent a "server" URL without the hash
-function createServerHref(location: Partial<Path> | Location | URL) {
-  return (location.pathname || "") + (location.search || "");
+function stripHashFromPath(path: To) {
+  let parsedPath = typeof path === "string" ? parsePath(path) : path;
+  return createPath({ ...parsedPath, hash: "" });
 }
 
 function isHashChangeOnly(a: Location, b: Location): boolean {
@@ -3060,14 +3054,4 @@ function getTargetMatch(
   let pathMatches = getPathContributingMatches(matches);
   return pathMatches[pathMatches.length - 1];
 }
-
-function createURL(location: Location | string): URL {
-  let base =
-    typeof window !== "undefined" && typeof window.location !== "undefined"
-      ? window.location.origin
-      : "unknown://unknown";
-  let href =
-    typeof location === "string" ? location : createServerHref(location);
-  return new URL(href, base);
-}
 //#endregion
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index 3bedd7a71aa..a13eb46d69d 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -333,9 +333,12 @@ export function matchRoutes<
   for (let i = 0; matches == null && i < branches.length; ++i) {
     matches = matchRouteBranch<string, RouteObjectType>(
       branches[i],
-      // incoming pathnames are always encoded from either window.location or
-      // from route.navigate, but we want to match against the unencoded paths
-      // in the route definitions
+      // Incoming pathnames are generally encoded from either window.location
+      // or from router.navigate, but we want to match against the unencoded
+      // paths in the route definitions.  Memory router locations won't be
+      // encoded here but there also shouldn't be anything to decode so this
+      // should be a safe operation.  This avoids needing matchRoutes to be
+      // history-aware.
       safelyDecodeURI(pathname)
     );
   }

From 01e2277f725be51234045c13f58626297230e92b Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Wed, 26 Oct 2022 13:10:19 -0400
Subject: [PATCH 17/30] Add data request error tests

---
 integration/error-data-request-test.ts  | 193 ++++++++++++++++++++++++
 packages/remix-server-runtime/server.ts |  47 ++++++
 2 files changed, 240 insertions(+)
 create mode 100644 integration/error-data-request-test.ts

diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts
new file mode 100644
index 00000000000..69328487024
--- /dev/null
+++ b/integration/error-data-request-test.ts
@@ -0,0 +1,193 @@
+import { test, expect } from "@playwright/test";
+
+import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+import type { Fixture, AppFixture } from "./helpers/create-fixture";
+
+test.describe("ErrorBoundary", () => {
+  let fixture: Fixture;
+  let appFixture: AppFixture;
+  let _consoleError: any;
+
+  test.beforeAll(async () => {
+    _consoleError = console.error;
+    console.error = () => {};
+
+    fixture = await createFixture({
+      files: {
+        "app/root.jsx": js`
+          import { Links, Meta, Outlet, Scripts } from "@remix-run/react";
+
+        export default function Root() {
+          return (
+            <html lang="en">
+              <head>
+                <Meta />
+                <Links />
+              </head>
+              <body>
+                <Outlet />
+                <Scripts />
+              </body>
+            </html>
+          );
+        }
+        `,
+
+        "app/routes/index.jsx": js`
+          import { Link, Form } from "@remix-run/react";
+
+          export default function () {
+            return <h1>Index</h1>
+          }
+        `,
+
+        [`app/routes/loader-throw-error.jsx`]: js`
+          export async function loader() {
+            throw Error("BLARGH");
+          }
+
+          export default function () {
+              return <h1>Hello</h1>
+          }
+        `,
+
+        [`app/routes/loader-return-json.jsx`]: js`
+          import { json } from "@remix-run/server-runtime";
+
+          export async function loader() {
+            return json({ ok: true });
+          }
+
+          export default function () {
+              return <h1>Hello</h1>
+          }
+        `,
+
+        [`app/routes/action-throw-error.jsx`]: js`
+          export async function action() {
+            throw Error("YOOOOOOOO WHAT ARE YOU DOING");
+          }
+
+          export default function () {
+            return <h1>Goodbye</h1>;
+          }
+        `,
+
+        [`app/routes/action-return-json.jsx`]: js`
+          import { json } from "@remix-run/server-runtime";
+
+          export async function action() {
+            return json({ ok: true });
+          }
+
+          export default function () {
+            return <h1>Hi!</h1>
+          }
+        `,
+      },
+    });
+    appFixture = await createAppFixture(fixture);
+  });
+
+  test.afterAll(async () => {
+    console.error = _consoleError;
+    await appFixture.close();
+  });
+
+  /**
+   * TODO
+   * end state
+   *  - thrown 4xx responses go to catch
+   *    - user thrown and router thrown
+   *  - thrown 5xx responses go to catch
+   *    - user thrown go to catch
+   *    - router thrown go to catch - we don't have any of these use cases currently
+   *  - thrown non responses go to error
+   *
+   * - Make test cases align to above
+   * - Change remix code as needed
+   * - Can we remove X-Remix-Router-Error header entirely?
+   * - Fork off a callRouteLoaderRR and callRouteActionRR function for simplicity
+   *
+   * Eventually in user error boundaries (remix v2)
+   *  - thrown ANYTHING goes to errorElement
+   *    if (useRouteError() instanceof Response) {
+   *      <CatchBoundary>
+   *    } else {
+   *      <ErrorBoundary>
+   *    }
+   */
+
+  // test.skip(
+  //   "returns a 500 x-remix-error on a data fetch to a path with no loader"
+  // );
+  // test.skip("returns a 405 x-remix-error on a data fetch with a bad method");
+  // test.skip("returns a 404 x-remix-error on a data fetch to a bad path");
+  // test.skip("returns a 403 x-remix-error on a data fetch to a bad routeId");
+
+  test("returns a 500 x-remix-error on a data fetch to a path with no loader", async () => {
+    let response = await fixture.requestData("/", "routes/index");
+    expect(response.status).toBe(500);
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch("Unexpected Server Error");
+  });
+
+  test("returns a 405 x-remix-catch on a data fetch POST to a path with no action", async () => {
+    let response = await fixture.requestData("/", "routes/index", {
+      method: "POST",
+    });
+    expect(response.status).toBe(405);
+    expect(response.headers.get("X-Remix-Catch")).toBe("yes");
+    expect(await response.text()).toBe("");
+  });
+
+  test("returns a 405 x-remix-error on a data fetch with a bad method", async () => {
+    let response = await fixture.requestData(
+      `/loader-return-json`,
+      "routes/loader-return-json",
+      {
+        method: "OPTIONS",
+      }
+    );
+    expect(response.status).toBe(405);
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch(
+      'Invalid request method \\"OPTIONS\\"'
+    );
+  });
+
+  // test("returns a 405 x-remix-error on a data fetch POST with a bad method", async () => {
+  //   let response = await fixture.requestData(
+  //     `/${NO_ROOT_BOUNDARY_ACTION}`,
+  //     NO_ROOT_BOUNDARY_ACTION,
+  //     {
+  //       method: "POST",
+  //     }
+  //   );
+  //   expect(response.status).toBe(405);
+  //   expect(response.headers.get("X-Remix-Catch")).toBe("yes");
+  // });
+
+  // test("returns a 403 x-remix-error on a data fetch GET to a bad path", async () => {
+  //   // just headers content-type mismatch but differs from POST below
+  //   let response = await fixture.requestData(
+  //     "/",
+  //     `routes${NO_ROOT_BOUNDARY_LOADER}`
+  //   );
+  //   expect(response.status).toBe(403);
+  //   expect(response.headers.get("X-Remix-Error")).toBe("yes");
+  // });
+
+  // test("returns a 405 x-remix-catch on a data fetch POST to a bad path", async () => {
+  //   let response = await fixture.requestData(
+  //     "/",
+  //     `routes${NO_ROOT_BOUNDARY_LOADER}`,
+  //     {
+  //       method: "POST",
+  //     }
+  //   );
+  //   expect(response.status).toBe(405);
+  //   // This is a catch today and differs from above
+  //   expect(response.headers.get("X-Remix-Catch")).toBe("yes");
+  // });
+});
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 5f5bb675943..46db8373630 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -264,6 +264,30 @@ async function handleDataRequestRR(
   routeId: string,
   request: Request
 ) {
+  // let url = new URL(request.url);
+  // Match existing no-match behavior
+  // if (!matches) {
+  //   return errorBoundaryError(
+  //     new Error(`No route matches URL "${url.pathname}"`),
+  //     404
+  //   );
+  // }
+
+  // let match = matches.find((match) => match.route.id === routeId);
+
+  // if (isActionRequest(request) && !match?.route?.module?.action) {
+  //   return new Response(null, {
+  //     status: 405,
+  //     statusText: "Method Not Allowed",
+  //     headers: { "X-Remix-Catch": "yes" },
+  //   });
+  // } else if (!match) {
+  //   return errorBoundaryError(
+  //     new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
+  //     403
+  //   );
+  // }
+
   try {
     let response = await staticHandler.queryRoute(request, routeId);
 
@@ -297,6 +321,26 @@ async function handleDataRequestRR(
         error.headers.set("X-Remix-Catch", "yes");
       }
 
+      // let isInternalError = error.headers.get("X-Remix-Router-Error") === "yes";
+      // error.headers.delete("X-Remix-Router-Error");
+
+      // if (!isInternalError) {
+      //   error.headers.set("X-Remix-Error", "yes");
+      // } else if (
+      //   error.status === 404 &&
+      //   (request.method === "GET" || request.method === "HEAD")
+      // ) {
+      //   let url = new URL(request.url);
+      //   return errorBoundaryError(
+      //     new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
+      //     404
+      //   );
+      // } else if (error.status === 404) {
+      //   error.headers.set("X-Remix-Catch", "yes");
+      // } else {
+      //   error.headers.set("X-Remix-Catch", "yes");
+      // }
+
       return error;
     }
 
@@ -806,6 +850,8 @@ async function handleDocumentRequest({
   }
 }
 
+// TODO: Check on error in Remix if a loader returns undefined
+
 async function handleResourceRequestRR(
   serverMode: ServerMode,
   staticHandler: StaticHandler,
@@ -813,6 +859,7 @@ async function handleResourceRequestRR(
   request: Request
 ) {
   try {
+    // TODO remove routeId here and no need to match above for new flow
     let response = await staticHandler.queryRoute(request, routeId);
     // Remix should always be returning responses from loaders and actions
     invariant(

From 077e9b5f23850ef7c4674acea65cc7ab2077266e Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Thu, 27 Oct 2022 13:39:49 -0400
Subject: [PATCH 18/30] Updates for catch.error boundary data responses

---
 integration/error-data-request-test.ts        | 134 ++++++++----------
 packages/remix-server-runtime/data.ts         |  56 ++++++--
 packages/remix-server-runtime/router/index.ts |   3 +
 .../remix-server-runtime/router/router.ts     |  78 ++++++----
 packages/remix-server-runtime/router/utils.ts |  18 +++
 packages/remix-server-runtime/routes.ts       |  14 +-
 packages/remix-server-runtime/server.ts       | 132 ++++++-----------
 7 files changed, 222 insertions(+), 213 deletions(-)

diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts
index 69328487024..cacd7f4cfbe 100644
--- a/integration/error-data-request-test.ts
+++ b/integration/error-data-request-test.ts
@@ -3,14 +3,19 @@ import { test, expect } from "@playwright/test";
 import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
 import type { Fixture, AppFixture } from "./helpers/create-fixture";
 
+const ENABLE_REMIX_ROUTER = !!process.env.ENABLE_REMIX_ROUTER;
+
 test.describe("ErrorBoundary", () => {
   let fixture: Fixture;
   let appFixture: AppFixture;
   let _consoleError: any;
+  let errorLogs: string[];
 
   test.beforeAll(async () => {
     _consoleError = console.error;
-    console.error = () => {};
+    console.error = (whatever) => {
+      errorLogs.push(String(whatever));
+    };
 
     fixture = await createFixture({
       files: {
@@ -89,56 +94,42 @@ test.describe("ErrorBoundary", () => {
     appFixture = await createAppFixture(fixture);
   });
 
+  test.beforeEach(async () => {
+    errorLogs = [];
+  });
+
   test.afterAll(async () => {
     console.error = _consoleError;
     await appFixture.close();
   });
 
-  /**
-   * TODO
-   * end state
-   *  - thrown 4xx responses go to catch
-   *    - user thrown and router thrown
-   *  - thrown 5xx responses go to catch
-   *    - user thrown go to catch
-   *    - router thrown go to catch - we don't have any of these use cases currently
-   *  - thrown non responses go to error
-   *
-   * - Make test cases align to above
-   * - Change remix code as needed
-   * - Can we remove X-Remix-Router-Error header entirely?
-   * - Fork off a callRouteLoaderRR and callRouteActionRR function for simplicity
-   *
-   * Eventually in user error boundaries (remix v2)
-   *  - thrown ANYTHING goes to errorElement
-   *    if (useRouteError() instanceof Response) {
-   *      <CatchBoundary>
-   *    } else {
-   *      <ErrorBoundary>
-   *    }
-   */
-
-  // test.skip(
-  //   "returns a 500 x-remix-error on a data fetch to a path with no loader"
-  // );
-  // test.skip("returns a 405 x-remix-error on a data fetch with a bad method");
-  // test.skip("returns a 404 x-remix-error on a data fetch to a bad path");
-  // test.skip("returns a 403 x-remix-error on a data fetch to a bad routeId");
-
-  test("returns a 500 x-remix-error on a data fetch to a path with no loader", async () => {
+  function assertConsoleError(str: string) {
+    expect(errorLogs[0]).toEqual(str);
+    if (ENABLE_REMIX_ROUTER) {
+      expect(errorLogs[1]).toEqual(str);
+    }
+  }
+
+  test("returns a 405 x-remix-error on a data fetch to a path with no loader", async () => {
     let response = await fixture.requestData("/", "routes/index");
-    expect(response.status).toBe(500);
+    expect(response.status).toBe(405);
     expect(response.headers.get("X-Remix-Error")).toBe("yes");
     expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError(
+      'Error: You made a GET request to test://test/?_data=routes%2Findex but did not provide a `loader` for route "routes/index", so there is no way to handle the request.'
+    );
   });
 
-  test("returns a 405 x-remix-catch on a data fetch POST to a path with no action", async () => {
-    let response = await fixture.requestData("/", "routes/index", {
+  test("returns a 405 x-remix-error on a data fetch POST to a path with no action", async () => {
+    let response = await fixture.requestData("/?index", "routes/index", {
       method: "POST",
     });
     expect(response.status).toBe(405);
-    expect(response.headers.get("X-Remix-Catch")).toBe("yes");
-    expect(await response.text()).toBe("");
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError(
+      'Error: You made a POST request to test://test/?index=&_data=routes%2Findex but did not provide an `action` for route "routes/index", so there is no way to handle the request.'
+    );
   });
 
   test("returns a 405 x-remix-error on a data fetch with a bad method", async () => {
@@ -151,43 +142,38 @@ test.describe("ErrorBoundary", () => {
     );
     expect(response.status).toBe(405);
     expect(response.headers.get("X-Remix-Error")).toBe("yes");
-    expect(await response.text()).toMatch(
-      'Invalid request method \\"OPTIONS\\"'
+    expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError('Error: Invalid request method "OPTIONS"');
+  });
+
+  test("returns a 403 x-remix-error on a data fetch GET to a bad path", async () => {
+    // just headers content-type mismatch but differs from POST below
+    let response = await fixture.requestData("/", "routes/loader-return-json");
+    expect(response.status).toBe(403);
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError(
+      'Error: Route "routes/loader-return-json" does not match URL "/"'
+    );
+  });
+
+  test("returns a 403 x-remix-error on a data fetch POST to a bad path", async () => {
+    let response = await fixture.requestData("/", "routes/loader-return-json", {
+      method: "POST",
+    });
+    expect(response.status).toBe(403);
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError(
+      'Error: Route "routes/loader-return-json" does not match URL "/"'
     );
   });
 
-  // test("returns a 405 x-remix-error on a data fetch POST with a bad method", async () => {
-  //   let response = await fixture.requestData(
-  //     `/${NO_ROOT_BOUNDARY_ACTION}`,
-  //     NO_ROOT_BOUNDARY_ACTION,
-  //     {
-  //       method: "POST",
-  //     }
-  //   );
-  //   expect(response.status).toBe(405);
-  //   expect(response.headers.get("X-Remix-Catch")).toBe("yes");
-  // });
-
-  // test("returns a 403 x-remix-error on a data fetch GET to a bad path", async () => {
-  //   // just headers content-type mismatch but differs from POST below
-  //   let response = await fixture.requestData(
-  //     "/",
-  //     `routes${NO_ROOT_BOUNDARY_LOADER}`
-  //   );
-  //   expect(response.status).toBe(403);
-  //   expect(response.headers.get("X-Remix-Error")).toBe("yes");
-  // });
-
-  // test("returns a 405 x-remix-catch on a data fetch POST to a bad path", async () => {
-  //   let response = await fixture.requestData(
-  //     "/",
-  //     `routes${NO_ROOT_BOUNDARY_LOADER}`,
-  //     {
-  //       method: "POST",
-  //     }
-  //   );
-  //   expect(response.status).toBe(405);
-  //   // This is a catch today and differs from above
-  //   expect(response.headers.get("X-Remix-Catch")).toBe("yes");
-  // });
+  test("returns a 404 x-remix-error on a data fetch to a path with no matches", async () => {
+    let response = await fixture.requestData("/i/match/nothing", "routes/junk");
+    expect(response.status).toBe(404);
+    expect(response.headers.get("X-Remix-Error")).toBe("yes");
+    expect(await response.text()).toMatch("Unexpected Server Error");
+    assertConsoleError('Error: No route matches URL "/i/match/nothing"');
+  });
 });
diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index 084de46eb9b..b7ec7864828 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -1,3 +1,4 @@
+import { ErrorWithStatus } from "./router";
 import { json, isResponse, isRedirectResponse } from "./responses";
 import type {
   ActionFunction,
@@ -34,12 +35,10 @@ export async function callRouteAction({
   isRemixRouterRequest?: boolean;
 }) {
   if (!action) {
-    let response = new Response(null, {
-      status: 405,
-      statusText: "Method Not Allowed",
-    });
-    response.headers.set("X-Remix-Catch", "yes");
-    return response;
+    let msg =
+      `You made a ${request.method} request to ${request.url} but did not provide ` +
+      `an \`action\` for route "${routeId}", so there is no way to handle the request.`;
+    throw new ErrorWithStatus(msg, 405);
   }
 
   let result;
@@ -91,11 +90,10 @@ export async function callRouteLoader({
   isRemixRouterRequest?: boolean;
 }) {
   if (!loader) {
-    throw new Error(
+    let msg =
       `You made a ${request.method} request to ${request.url} but did not provide ` +
-        `a default component or \`loader\` for route "${routeId}", ` +
-        `so there is no way to handle the request.`
-    );
+      `a \`loader\` for route "${routeId}", so there is no way to handle the request.`;
+    throw new ErrorWithStatus(msg, 405);
   }
 
   let result;
@@ -131,6 +129,44 @@ export async function callRouteLoader({
   return isResponse(result) ? result : json(result);
 }
 
+export async function callRouteActionRR({
+  loadContext,
+  action,
+  params,
+  request,
+}: {
+  request: Request;
+  action: ActionFunction;
+  params: DataFunctionArgs["params"];
+  loadContext: AppLoadContext;
+}) {
+  let result = await action({
+    request: stripDataParam(stripIndexParam(request)),
+    context: loadContext,
+    params,
+  });
+  return isResponse(result) ? result : json(result);
+}
+
+export async function callRouteLoaderRR({
+  loadContext,
+  loader,
+  params,
+  request,
+}: {
+  request: Request;
+  loader: LoaderFunction;
+  params: DataFunctionArgs["params"];
+  loadContext: AppLoadContext;
+}) {
+  let result = await loader({
+    request: stripDataParam(stripIndexParam(request)),
+    context: loadContext,
+    params,
+  });
+  return isResponse(result) ? result : json(result);
+}
+
 function stripIndexParam(request: Request) {
   let url = new URL(request.url);
   let indexValues = url.searchParams.getAll("index");
diff --git a/packages/remix-server-runtime/router/index.ts b/packages/remix-server-runtime/router/index.ts
index b643dd55030..a673cee1969 100644
--- a/packages/remix-server-runtime/router/index.ts
+++ b/packages/remix-server-runtime/router/index.ts
@@ -30,11 +30,14 @@ export type {
 
 export {
   AbortedDeferredError,
+  // TODO: Is this supposed to be exported?
   ErrorResponse,
+  ErrorWithStatus,
   defer,
   generatePath,
   getToPathname,
   invariant,
+  isErrorWithStatus,
   isRouteErrorResponse,
   joinPaths,
   json,
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 28a4ae764e3..3b581c66b01 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -26,6 +26,7 @@ import type {
 import {
   DeferredData,
   ErrorResponse,
+  ErrorWithStatus,
   ResultType,
   convertRoutesToDataRoutes,
   getPathContributingMatches,
@@ -1922,6 +1923,8 @@ export function unstable_createStaticHandler(
    * from the action/loader, but it may be a primitive or other value as well -
    * and in such cases the calling context should handle that accordingly.
    *
+   * TODO: Add note about thrown ErrorWithStatus
+   *
    * We do respect the throw/return differentiation, so if an action/loader
    * throws, then this method will throw the value.  This is important so we
    * can do proper boundary identification in Remix where a thrown Response
@@ -1937,26 +1940,32 @@ export function unstable_createStaticHandler(
     let matches = matchRoutes(dataRoutes, location);
 
     if (!validRequestMethods.has(request.method)) {
-      throw createRouterErrorResponse(null, {
-        status: 405,
-        statusText: "Method Not Allowed",
-      });
+      throw new ErrorWithStatus(
+        `Invalid request method "${request.method}"`,
+        405
+      );
     } else if (!matches) {
-      throw createRouterErrorResponse(null, {
-        status: 404,
-        statusText: "Not Found",
-      });
+      throw new ErrorWithStatus(
+        `No route matches URL "${location.pathname}"`,
+        404
+      );
     }
 
     let match = routeId
       ? matches.find((m) => m.route.id === routeId)
       : getTargetMatch(matches, location);
 
-    if (!match) {
-      throw createRouterErrorResponse(null, {
-        status: 404,
-        statusText: "Not Found",
-      });
+    if (routeId && !match) {
+      throw new ErrorWithStatus(
+        `Route "${routeId}" does not match URL "${location.pathname}"`,
+        403
+      );
+    } else if (!match) {
+      // This should never hit I don't think?
+      throw new ErrorWithStatus(
+        `No route matches URL "${createPath(location)}"`,
+        404
+      );
     }
 
     let result = await queryImpl(request, location, matches, match);
@@ -2034,12 +2043,15 @@ export function unstable_createStaticHandler(
     isRouteRequest: boolean
   ): Promise<Omit<StaticHandlerContext, "location"> | Response> {
     let result: DataResult;
+
     if (!actionMatch.route.action) {
       if (isRouteRequest) {
-        throw createRouterErrorResponse(null, {
-          status: 405,
-          statusText: "Method Not Allowed",
-        });
+        throw new ErrorWithStatus(
+          `You made a ${request.method} request to ${request.url} but ` +
+            `did not provide an \`action\` for route "${actionMatch.route.id}", ` +
+            `so there is no way to handle the request.`,
+          405
+        );
       }
       result = getMethodNotAllowedResult(request.url);
     } else {
@@ -2155,6 +2167,17 @@ export function unstable_createStaticHandler(
     | Response
   > {
     let isRouteRequest = routeMatch != null;
+
+    // Short circuit if we have no loaders to run (queryRoute())
+    if (isRouteRequest && !routeMatch?.route.loader) {
+      throw new ErrorWithStatus(
+        `You made a ${request.method} request to ${request.url} but ` +
+          `did not provide a \`loader\` for route "${routeMatch?.route.id}", ` +
+          `so there is no way to handle the request.`,
+        405
+      );
+    }
+
     let requestMatches = routeMatch
       ? [routeMatch]
       : getLoaderMatchesUntilBoundary(
@@ -2163,7 +2186,7 @@ export function unstable_createStaticHandler(
         );
     let matchesToLoad = requestMatches.filter((m) => m.route.loader);
 
-    // Short circuit if we have no loaders to run
+    // Short circuit if we have no loaders to run (query())
     if (matchesToLoad.length === 0) {
       return {
         matches,
@@ -2215,19 +2238,6 @@ export function unstable_createStaticHandler(
     };
   }
 
-  function createRouterErrorResponse(
-    body: BodyInit | null | undefined,
-    init: ResponseInit
-  ) {
-    return new Response(body, {
-      ...init,
-      headers: {
-        ...init.headers,
-        "X-Remix-Router-Error": "yes",
-      },
-    });
-  }
-
   return {
     dataRoutes,
     query,
@@ -2609,6 +2619,12 @@ async function callLoaderOrAction(
     }
 
     if (resultType === ResultType.error) {
+      // TODO: Check if this is breaking for remix.  If the unwrapped data is
+      // of the format { message: string, stack: string } convert it back to
+      // new Error() here.  This might be a non-issue since the Remix client-side
+      // loader implementation might do the unwrapping and converson and throw
+      // new Error in which case we don't come this code path since it's not a
+      // thrown Response.
       return {
         type: resultType,
         error: new ErrorResponse(status, result.statusText, data),
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index a13eb46d69d..47d7af1415c 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -1260,3 +1260,21 @@ export class ErrorResponse {
 export function isRouteErrorResponse(e: any): e is ErrorResponse {
   return e instanceof ErrorResponse;
 }
+
+/**
+ * @private
+ * Utility class we use to hold thrown Errors that are status-code-aware
+ */
+export class ErrorWithStatus extends Error {
+  constructor(public msg: string, public status: number) {
+    super(msg);
+  }
+}
+
+/**
+ * Check if the given error is an ErrorResponse generated from a 4xx/5xx
+ * Response throw from an action/loader
+ */
+export function isErrorWithStatus(e: any): e is ErrorWithStatus {
+  return e instanceof ErrorWithStatus;
+}
diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
index 82c407245c0..d4edd6dd692 100644
--- a/packages/remix-server-runtime/routes.ts
+++ b/packages/remix-server-runtime/routes.ts
@@ -4,7 +4,7 @@ import type {
   ActionFunctionArgs,
   LoaderFunctionArgs,
 } from "./router";
-import type { AppLoadContext } from "./data";
+import { AppLoadContext, callRouteActionRR, callRouteLoaderRR } from "./data";
 import { callRouteAction, callRouteLoader } from "./data";
 import type { ServerRouteModule } from "./routeModules";
 
@@ -70,22 +70,18 @@ export function createStaticHandlerDataRoutes(
         path: route.path,
         loader: route.module.loader
           ? (args: LoaderFunctionArgs) =>
-              callRouteLoader({
+              callRouteLoaderRR({
                 ...args,
-                routeId: route.id,
-                loader: route.module.loader,
+                loader: route.module.loader!,
                 loadContext,
-                isRemixRouterRequest: true,
               })
           : undefined,
         action: route.module.action
           ? (args: ActionFunctionArgs) =>
-              callRouteAction({
+              callRouteActionRR({
                 ...args,
-                routeId: route.id,
-                action: route.module.action,
+                action: route.module.action!,
                 loadContext,
-                isRemixRouterRequest: true,
               })
           : undefined,
         handle: route.module.handle,
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 46db8373630..ce47fda40ec 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -1,6 +1,10 @@
 // TODO: RRR - Change import to @remix-run/router
 import type { StaticHandler, StaticHandlerContext } from "./router";
-import { isRouteErrorResponse } from "./router";
+import {
+  ErrorWithStatus,
+  isErrorWithStatus,
+  isRouteErrorResponse,
+} from "./router";
 import { unstable_createStaticHandler } from "./router";
 import type { AppLoadContext } from "./data";
 import { callRouteAction, callRouteLoader, extractData } from "./data";
@@ -174,26 +178,39 @@ async function handleDataRequest({
   request: Request;
   serverMode: ServerMode;
 }): Promise<Response> {
-  if (!isValidRequestMethod(request)) {
-    return errorBoundaryError(
-      new Error(`Invalid request method "${request.method}"`),
-      405
-    );
-  }
-
-  let url = new URL(request.url);
-
-  if (!matches) {
-    return errorBoundaryError(
-      new Error(`No route matches URL "${url.pathname}"`),
-      404
-    );
-  }
-
   let response: Response;
   let match: RouteMatch<ServerRoute>;
+
   try {
+    if (!isValidRequestMethod(request)) {
+      throw new ErrorWithStatus(
+        `Invalid request method "${request.method}"`,
+        405
+      );
+    }
+
+    let url = new URL(request.url);
+
+    if (!matches) {
+      throw new ErrorWithStatus(`No route matches URL "${url.pathname}"`, 404);
+    }
+
+    let routeId = url.searchParams.get("_data");
+    if (!routeId) {
+      throw new ErrorWithStatus(`Missing route id in ?_data`, 403);
+    }
+
+    let tempMatch = matches.find((match) => match.route.id === routeId);
+    if (!tempMatch) {
+      throw new ErrorWithStatus(
+        `Route "${routeId}" does not match URL "${url.pathname}"`,
+        403
+      );
+    }
+    match = tempMatch;
+
     if (isActionRequest(request)) {
+      // TODO: Can this go away??
       match = getRequestMatch(url, matches);
 
       response = await callRouteAction({
@@ -204,20 +221,6 @@ async function handleDataRequest({
         request: request,
       });
     } else {
-      let routeId = url.searchParams.get("_data");
-      if (!routeId) {
-        return errorBoundaryError(new Error(`Missing route id in ?_data`), 403);
-      }
-
-      let tempMatch = matches.find((match) => match.route.id === routeId);
-      if (!tempMatch) {
-        return errorBoundaryError(
-          new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
-          403
-        );
-      }
-      match = tempMatch;
-
       response = await callRouteLoader({
         loadContext,
         loader: match.route.module.loader,
@@ -250,11 +253,12 @@ async function handleDataRequest({
       console.error(error);
     }
 
+    let status = isErrorWithStatus(error) ? error.status : 500;
     if (serverMode === ServerMode.Development && error instanceof Error) {
-      return errorBoundaryError(error, 500);
+      return errorBoundaryError(error, status);
     }
 
-    return errorBoundaryError(new Error("Unexpected Server Error"), 500);
+    return errorBoundaryError(new Error("Unexpected Server Error"), status);
   }
 }
 
@@ -264,30 +268,6 @@ async function handleDataRequestRR(
   routeId: string,
   request: Request
 ) {
-  // let url = new URL(request.url);
-  // Match existing no-match behavior
-  // if (!matches) {
-  //   return errorBoundaryError(
-  //     new Error(`No route matches URL "${url.pathname}"`),
-  //     404
-  //   );
-  // }
-
-  // let match = matches.find((match) => match.route.id === routeId);
-
-  // if (isActionRequest(request) && !match?.route?.module?.action) {
-  //   return new Response(null, {
-  //     status: 405,
-  //     statusText: "Method Not Allowed",
-  //     headers: { "X-Remix-Catch": "yes" },
-  //   });
-  // } else if (!match) {
-  //   return errorBoundaryError(
-  //     new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
-  //     403
-  //   );
-  // }
-
   try {
     let response = await staticHandler.queryRoute(request, routeId);
 
@@ -311,36 +291,7 @@ async function handleDataRequestRR(
     return response;
   } catch (error) {
     if (error instanceof Response) {
-      // Our callRouteLoader/callRouteAction methods ensure we always have
-      // X-Remix-Catch on responses thrown from loaders and actions.  However,
-      // if we never make it to the loader (such as a 404 or 405) then we
-      // Remix Router puts it's own custom header on, which we swap here for
-      // X-Remix-Catch
-      if (error.headers.get("X-Remix-Router-Error") === "yes") {
-        error.headers.delete("X-Remix-Router-Error");
-        error.headers.set("X-Remix-Catch", "yes");
-      }
-
-      // let isInternalError = error.headers.get("X-Remix-Router-Error") === "yes";
-      // error.headers.delete("X-Remix-Router-Error");
-
-      // if (!isInternalError) {
-      //   error.headers.set("X-Remix-Error", "yes");
-      // } else if (
-      //   error.status === 404 &&
-      //   (request.method === "GET" || request.method === "HEAD")
-      // ) {
-      //   let url = new URL(request.url);
-      //   return errorBoundaryError(
-      //     new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
-      //     404
-      //   );
-      // } else if (error.status === 404) {
-      //   error.headers.set("X-Remix-Catch", "yes");
-      // } else {
-      //   error.headers.set("X-Remix-Catch", "yes");
-      // }
-
+      error.headers.set("X-Remix-Catch", "yes");
       return error;
     }
 
@@ -348,11 +299,12 @@ async function handleDataRequestRR(
       console.error(error);
     }
 
+    let status = isErrorWithStatus(error) ? error.status : 500;
     if (serverMode === ServerMode.Development && error instanceof Error) {
-      return errorBoundaryError(error, 500);
+      return errorBoundaryError(error, status);
     }
 
-    return errorBoundaryError(new Error("Unexpected Server Error"), 500);
+    return errorBoundaryError(new Error("Unexpected Server Error"), status);
   }
 }
 
@@ -559,6 +511,8 @@ async function handleDocumentRequest({
       statusText: "Method Not Allowed",
     };
   } else if (!matches) {
+    // TODO: Double check that we have integration tests asserting that this
+    // is a 404 catch boundary
     appState.trackCatchBoundaries = false;
     appState.catch = {
       data: null,

From 194ea13a9b1ba4d08debbab843535797d75b685a Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Thu, 27 Oct 2022 13:34:26 -0700
Subject: [PATCH 19/30] fix: ensure route modules are loaded regardless of
 error state

- reorder tests to match cases
- handle thrown errors with status
---
 integration/catch-boundary-test.ts      |  52 ----------
 integration/error-boundary-test.ts      | 132 +++++++++++++++++++++---
 packages/remix-react/routes.tsx         |  84 ++++++++-------
 packages/remix-server-runtime/routes.ts |   4 +-
 packages/remix-server-runtime/server.ts |   6 +-
 5 files changed, 172 insertions(+), 106 deletions(-)

diff --git a/integration/catch-boundary-test.ts b/integration/catch-boundary-test.ts
index 46da4bea157..a157a3847bd 100644
--- a/integration/catch-boundary-test.ts
+++ b/integration/catch-boundary-test.ts
@@ -334,58 +334,6 @@ test.describe("CatchBoundary", () => {
     await page.waitForSelector("#root-boundary");
   });
 
-  test("renders root boundary in document POST without action requests", async () => {
-    let res = await fixture.requestDocument(NO_BOUNDARY_NO_LOADER_OR_ACTION, {
-      method: "post",
-    });
-    expect(res.status).toBe(405);
-    expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
-  });
-
-  test("renders root boundary in action script transitions without action from other routes", async ({
-    page,
-  }) => {
-    let app = new PlaywrightFixture(appFixture, page);
-    await app.goto("/");
-    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    await page.waitForSelector("#root-boundary");
-  });
-
-  test("renders own boundary in document POST without action requests", async () => {
-    let res = await fixture.requestDocument(HAS_BOUNDARY_NO_LOADER_OR_ACTION, {
-      method: "post",
-    });
-    expect(res.status).toBe(405);
-    expect(await res.text()).toMatch(OWN_BOUNDARY_TEXT);
-  });
-
-  test("renders own boundary in action script transitions without action from other routes", async ({
-    page,
-  }) => {
-    let app = new PlaywrightFixture(appFixture, page);
-    await app.goto("/");
-    await app.clickSubmitButton(HAS_BOUNDARY_NO_LOADER_OR_ACTION);
-    await page.waitForSelector("#boundary-no-loader-or-action");
-  });
-
-  test("renders own boundary in fetcher action submission without action from other routes", async ({
-    page,
-  }) => {
-    let app = new PlaywrightFixture(appFixture, page);
-    await app.goto("/fetcher-boundary");
-    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    await page.waitForSelector("#fetcher-boundary");
-  });
-
-  test("renders root boundary in fetcher action submission without action from other routes", async ({
-    page,
-  }) => {
-    let app = new PlaywrightFixture(appFixture, page);
-    await app.goto("/fetcher-no-boundary");
-    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
-    await page.waitForSelector("#root-boundary");
-  });
-
   test("uses correct catch boundary on server action errors", async ({
     page,
   }) => {
diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index b8328842668..bec5430ec9a 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -15,10 +15,12 @@ test.describe("ErrorBoundary", () => {
   let HAS_BOUNDARY_LOADER = "/yes/loader";
   let HAS_BOUNDARY_ACTION = "/yes/action";
   let HAS_BOUNDARY_RENDER = "/yes/render";
+  let HAS_BOUNDARY_NO_LOADER_OR_ACTION = "/yes/no-loader-or-action";
 
   let NO_BOUNDARY_ACTION = "/no/action";
   let NO_BOUNDARY_LOADER = "/no/loader";
   let NO_BOUNDARY_RENDER = "/no/render";
+  let NO_BOUNDARY_NO_LOADER_OR_ACTION = "/no/no-loader-or-action";
 
   let NOT_FOUND_HREF = "/not/found";
 
@@ -56,7 +58,7 @@ test.describe("ErrorBoundary", () => {
                 <head />
                 <body>
                   <main>
-                    <div>${ROOT_BOUNDARY_TEXT}</div>
+                    <div id="root-boundary">${ROOT_BOUNDARY_TEXT}</div>
                   </main>
                   <Scripts />
                 </body>
@@ -79,6 +81,12 @@ test.describe("ErrorBoundary", () => {
                   <button formAction="${NO_BOUNDARY_ACTION}" type="submit">
                     No Boundary
                   </button>
+                  <button formAction="${HAS_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit">
+                    Has Boundary No Loader or Action
+                  </button>
+                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit">
+                    No Boundary No Loader or Action
+                  </button>
                 </Form>
 
                 <Link to="${HAS_BOUNDARY_LOADER}">
@@ -172,6 +180,62 @@ test.describe("ErrorBoundary", () => {
           }
         `,
 
+        [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
+          export default function Index() {
+            return <div/>
+          }
+        `,
+
+        [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
+          export function ErrorBoundary() {
+            return <div id="boundary-no-loader-or-action">${OWN_BOUNDARY_TEXT}</div>
+          }
+          export default function Index() {
+            return <div/>
+          }
+        `,
+
+        [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
+          export default function Index() {
+            return <div/>
+          }
+        `,
+
+        "app/routes/fetcher-boundary.jsx": js`
+          import { useFetcher } from "@remix-run/react";
+          export function ErrorBoundary() {
+            return <p id="fetcher-boundary">${OWN_BOUNDARY_TEXT}</p>
+          }
+          export default function() {
+            let fetcher = useFetcher();
+
+            return (
+              <div>
+                <fetcher.Form method="post">
+                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit" />
+                </fetcher.Form>
+              </div>
+            )
+          }
+        `,
+
+        "app/routes/fetcher-no-boundary.jsx": js`
+          import { useFetcher } from "@remix-run/react";
+          export default function() {
+            let fetcher = useFetcher();
+
+            return (
+              <div>
+                <fetcher.Form method="post">
+                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit">
+                    No Loader or Action
+                  </button>
+                </fetcher.Form>
+              </div>
+            )
+          }
+        `,
+
         "app/routes/action.jsx": js`
           import { Outlet, useLoaderData } from "@remix-run/react";
 
@@ -235,12 +299,7 @@ test.describe("ErrorBoundary", () => {
     expect(await res.text()).toMatch(OWN_BOUNDARY_TEXT);
   });
 
-  // FIXME: this is broken, test renders the root boundary logging in `RemixRoute`
-  // test's because the route module hasn't been loaded, my gut tells me that we
-  // didn't load the route module but tried to render test's boundary, we need the
-  // module for that!  this will probably fix the twin test over in
-  // catch-boundary-test
-  test.skip("own boundary, action, client transition from other route", async ({
+  test("own boundary, action, client transition from other route", async ({
     page,
   }) => {
     let app = new PlaywrightFixture(appFixture, page);
@@ -353,6 +412,58 @@ test.describe("ErrorBoundary", () => {
     expect(await app.getHtml("#child-error")).toMatch("Broken!");
   });
 
+  test("renders own boundary in fetcher action submission without action from other routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/fetcher-boundary");
+    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
+    await page.waitForSelector("#fetcher-boundary");
+  });
+
+  test("renders root boundary in fetcher action submission without action from other routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/fetcher-no-boundary");
+    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
+    await page.waitForSelector("#root-boundary");
+  });
+
+  test("renders root boundary in document POST without action requests", async () => {
+    let res = await fixture.requestDocument(NO_BOUNDARY_NO_LOADER_OR_ACTION, {
+      method: "post",
+    });
+    expect(res.status).toBe(405);
+    expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
+  });
+
+  test("renders root boundary in action script transitions without action from other routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/");
+    await app.clickSubmitButton(NO_BOUNDARY_NO_LOADER_OR_ACTION);
+    await page.waitForSelector("#root-boundary");
+  });
+
+  test("renders own boundary in document POST without action requests", async () => {
+    let res = await fixture.requestDocument(HAS_BOUNDARY_NO_LOADER_OR_ACTION, {
+      method: "post",
+    });
+    expect(res.status).toBe(405);
+    expect(await res.text()).toMatch(OWN_BOUNDARY_TEXT);
+  });
+
+  test("renders own boundary in action script transitions without action from other routes", async ({
+    page,
+  }) => {
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/");
+    await app.clickSubmitButton(HAS_BOUNDARY_NO_LOADER_OR_ACTION);
+    await page.waitForSelector("#boundary-no-loader-or-action");
+  });
+
   test.describe("if no error boundary exists in the app", () => {
     let NO_ROOT_BOUNDARY_LOADER = "/loader-bad";
     let NO_ROOT_BOUNDARY_ACTION = "/action-bad";
@@ -476,12 +587,5 @@ test.describe("ErrorBoundary", () => {
       await app.clickSubmitButton(NO_ROOT_BOUNDARY_ACTION_RETURN);
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
-
-    test.skip(
-      "returns a 500 x-remix-error on a data fetch to a path with no loader"
-    );
-    test.skip("returns a 405 x-remix-error on a data fetch with a bad method");
-    test.skip("returns a 404 x-remix-error on a data fetch to a bad path");
-    test.skip("returns a 403 x-remix-error on a data fetch to a bad routeId");
   });
 });
diff --git a/packages/remix-react/routes.tsx b/packages/remix-react/routes.tsx
index 5873e13bf17..af9361dd94c 100644
--- a/packages/remix-react/routes.tsx
+++ b/packages/remix-react/routes.tsx
@@ -148,25 +148,30 @@ async function loadRouteModuleWithBlockingLinks(
 function createLoader(route: EntryRoute, routeModules: RouteModules) {
   let loader: ClientRoute["loader"] = async ({ url, signal, submission }) => {
     if (route.hasLoader) {
-      let [result] = await Promise.all([
-        fetchData(url, route.id, signal, submission),
-        loadRouteModuleWithBlockingLinks(route, routeModules),
-      ]);
+      let routeModulePromise = loadRouteModuleWithBlockingLinks(
+        route,
+        routeModules
+      );
+      try {
+        let result = await fetchData(url, route.id, signal, submission);
 
-      if (result instanceof Error) throw result;
+        if (result instanceof Error) throw result;
 
-      let redirect = await checkRedirect(result);
-      if (redirect) return redirect;
+        let redirect = await checkRedirect(result);
+        if (redirect) return redirect;
 
-      if (isCatchResponse(result)) {
-        throw new CatchValue(
-          result.status,
-          result.statusText,
-          await extractData(result)
-        );
-      }
+        if (isCatchResponse(result)) {
+          throw new CatchValue(
+            result.status,
+            result.statusText,
+            await extractData(result)
+          );
+        }
 
-      return extractData(result);
+        return extractData(result);
+      } finally {
+        await routeModulePromise;
+      }
     } else {
       await loadRouteModuleWithBlockingLinks(route, routeModules);
     }
@@ -177,33 +182,40 @@ function createLoader(route: EntryRoute, routeModules: RouteModules) {
 
 function createAction(route: EntryRoute, routeModules: RouteModules) {
   let action: ClientRoute["action"] = async ({ url, signal, submission }) => {
-    if (!route.hasAction) {
-      console.error(
-        `Route "${route.id}" does not have an action, but you are trying ` +
-          `to submit to it. To fix this, please add an \`action\` function to the route`
-      );
-    }
+    let routeModulePromise = await loadRouteModuleWithBlockingLinks(
+      route,
+      routeModules
+    );
+
+    try {
+      if (!route.hasAction) {
+        console.error(
+          `Route "${route.id}" does not have an action, but you are trying ` +
+            `to submit to it. To fix this, please add an \`action\` function to the route`
+        );
+      }
 
-    let result = await fetchData(url, route.id, signal, submission);
+      let result = await fetchData(url, route.id, signal, submission);
 
-    if (result instanceof Error) {
-      throw result;
-    }
+      if (result instanceof Error) {
+        throw result;
+      }
 
-    let redirect = await checkRedirect(result);
-    if (redirect) return redirect;
+      let redirect = await checkRedirect(result);
+      if (redirect) return redirect;
 
-    await loadRouteModuleWithBlockingLinks(route, routeModules);
+      if (isCatchResponse(result)) {
+        throw new CatchValue(
+          result.status,
+          result.statusText,
+          await extractData(result)
+        );
+      }
 
-    if (isCatchResponse(result)) {
-      throw new CatchValue(
-        result.status,
-        result.statusText,
-        await extractData(result)
-      );
+      return extractData(result);
+    } finally {
+      await routeModulePromise;
     }
-
-    return extractData(result);
   };
 
   return action;
diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
index d4edd6dd692..b512746466f 100644
--- a/packages/remix-server-runtime/routes.ts
+++ b/packages/remix-server-runtime/routes.ts
@@ -4,8 +4,8 @@ import type {
   ActionFunctionArgs,
   LoaderFunctionArgs,
 } from "./router";
-import { AppLoadContext, callRouteActionRR, callRouteLoaderRR } from "./data";
-import { callRouteAction, callRouteLoader } from "./data";
+import { type AppLoadContext } from "./data";
+import { callRouteActionRR, callRouteLoaderRR } from "./data";
 import type { ServerRouteModule } from "./routeModules";
 
 export interface RouteManifest<Route> {
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index ce47fda40ec..87ef0a51510 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -521,6 +521,7 @@ async function handleDocumentRequest({
     };
   }
 
+  let errorStatus = 500;
   let actionStatus: { status: number; statusText: string } | undefined;
   let actionData: Record<string, unknown> | undefined;
   let actionMatch: RouteMatch<ServerRoute> | undefined;
@@ -564,6 +565,7 @@ async function handleDocumentRequest({
         };
       }
     } catch (error: any) {
+      errorStatus = isErrorWithStatus(error) ? error.status : 500;
       appState.loaderBoundaryRouteId = getDeepestRouteIdWithBoundary(
         matches,
         "ErrorBoundary"
@@ -671,7 +673,7 @@ async function handleDocumentRequest({
     }
 
     if (error) {
-      loaderStatusCodes.push(500);
+      loaderStatusCodes.push(isErrorWithStatus(error) ? error.status : 500);
       appState.trackBoundaries = false;
       appState.error = await serializeError(error);
 
@@ -740,7 +742,7 @@ async function handleDocumentRequest({
       : loaderStatusCodes.find((status) => status !== 200);
 
   let responseStatusCode = appState.error
-    ? 500
+    ? errorStatus
     : typeof notOkResponse === "number"
     ? notOkResponse
     : appState.catch

From 8d7c86aa88c4046e06b8e620803b1e80ba68e827 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Fri, 28 Oct 2022 17:44:16 -0400
Subject: [PATCH 20/30] =?UTF-8?q?ALL=20GREEN=20BABY=20=E2=9C=85?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 integration/catch-boundary-test.ts            |   6 -
 integration/error-boundary-test.ts            |   6 +
 integration/error-data-request-test.ts        |   4 +-
 packages/remix-server-runtime/data.ts         |  32 ++-
 packages/remix-server-runtime/router/index.ts |   3 -
 .../remix-server-runtime/router/router.ts     | 226 ++++++++++--------
 packages/remix-server-runtime/router/utils.ts |  37 ++-
 packages/remix-server-runtime/server.ts       | 148 +++++++++---
 8 files changed, 285 insertions(+), 177 deletions(-)

diff --git a/integration/catch-boundary-test.ts b/integration/catch-boundary-test.ts
index a157a3847bd..ec38c2eeed3 100644
--- a/integration/catch-boundary-test.ts
+++ b/integration/catch-boundary-test.ts
@@ -250,12 +250,6 @@ test.describe("CatchBoundary", () => {
     await page.waitForSelector("#root-boundary");
   });
 
-  test("invalid request methods", async () => {
-    let res = await fixture.requestDocument("/", { method: "OPTIONS" });
-    expect(res.status).toBe(405);
-    expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
-  });
-
   test("own boundary, action, document request", async () => {
     let params = new URLSearchParams();
     let res = await fixture.postDocument(HAS_BOUNDARY_ACTION, params);
diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index bec5430ec9a..5d10a2fd3e8 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -292,6 +292,12 @@ test.describe("ErrorBoundary", () => {
     await appFixture.close();
   });
 
+  test("invalid request methods", async () => {
+    let res = await fixture.requestDocument("/", { method: "OPTIONS" });
+    expect(res.status).toBe(405);
+    expect(await res.text()).toMatch(ROOT_BOUNDARY_TEXT);
+  });
+
   test("own boundary, action, document request", async () => {
     let params = new URLSearchParams();
     let res = await fixture.postDocument(HAS_BOUNDARY_ACTION, params);
diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts
index cacd7f4cfbe..dc0a7d9f1f7 100644
--- a/integration/error-data-request-test.ts
+++ b/integration/error-data-request-test.ts
@@ -116,7 +116,7 @@ test.describe("ErrorBoundary", () => {
     expect(response.headers.get("X-Remix-Error")).toBe("yes");
     expect(await response.text()).toMatch("Unexpected Server Error");
     assertConsoleError(
-      'Error: You made a GET request to test://test/?_data=routes%2Findex but did not provide a `loader` for route "routes/index", so there is no way to handle the request.'
+      'Error: You made a GET request to "/" but did not provide a `loader` for route "routes/index", so there is no way to handle the request.'
     );
   });
 
@@ -128,7 +128,7 @@ test.describe("ErrorBoundary", () => {
     expect(response.headers.get("X-Remix-Error")).toBe("yes");
     expect(await response.text()).toMatch("Unexpected Server Error");
     assertConsoleError(
-      'Error: You made a POST request to test://test/?index=&_data=routes%2Findex but did not provide an `action` for route "routes/index", so there is no way to handle the request.'
+      'Error: You made a POST request to "/" but did not provide an `action` for route "routes/index", so there is no way to handle the request.'
     );
   });
 
diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index b7ec7864828..2b52a4a54a4 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -1,10 +1,10 @@
-import { ErrorWithStatus } from "./router";
 import { json, isResponse, isRedirectResponse } from "./responses";
 import type {
   ActionFunction,
   DataFunctionArgs,
   LoaderFunction,
 } from "./routeModules";
+import { ErrorResponse } from "./router";
 
 /**
  * An object of unknown type for route loaders and actions provided by the
@@ -35,10 +35,11 @@ export async function callRouteAction({
   isRemixRouterRequest?: boolean;
 }) {
   if (!action) {
+    let pathname = new URL(request.url).pathname;
     let msg =
-      `You made a ${request.method} request to ${request.url} but did not provide ` +
+      `You made a ${request.method} request to "${pathname}" but did not provide ` +
       `an \`action\` for route "${routeId}", so there is no way to handle the request.`;
-    throw new ErrorWithStatus(msg, 405);
+    throw new ErrorResponse(405, "Method Not Allowed", new Error(msg), true);
   }
 
   let result;
@@ -90,10 +91,11 @@ export async function callRouteLoader({
   isRemixRouterRequest?: boolean;
 }) {
   if (!loader) {
+    let pathname = new URL(request.url).pathname;
     let msg =
-      `You made a ${request.method} request to ${request.url} but did not provide ` +
+      `You made a ${request.method} request to "${pathname}" but did not provide ` +
       `a \`loader\` for route "${routeId}", so there is no way to handle the request.`;
-    throw new ErrorWithStatus(msg, 405);
+    throw new ErrorResponse(405, "Method Not Allowed", new Error(msg), true);
   }
 
   let result;
@@ -134,17 +136,27 @@ export async function callRouteActionRR({
   action,
   params,
   request,
+  routeId,
 }: {
   request: Request;
   action: ActionFunction;
   params: DataFunctionArgs["params"];
   loadContext: AppLoadContext;
+  routeId: string;
 }) {
   let result = await action({
     request: stripDataParam(stripIndexParam(request)),
     context: loadContext,
     params,
   });
+
+  if (result === undefined) {
+    throw new Error(
+      `You defined an action for route "${routeId}" but didn't return ` +
+        `anything from your \`action\` function. Please return a value or \`null\`.`
+    );
+  }
+
   return isResponse(result) ? result : json(result);
 }
 
@@ -153,17 +165,27 @@ export async function callRouteLoaderRR({
   loader,
   params,
   request,
+  routeId,
 }: {
   request: Request;
   loader: LoaderFunction;
   params: DataFunctionArgs["params"];
   loadContext: AppLoadContext;
+  routeId: string;
 }) {
   let result = await loader({
     request: stripDataParam(stripIndexParam(request)),
     context: loadContext,
     params,
   });
+
+  if (result === undefined) {
+    throw new Error(
+      `You defined a loader for route "${routeId}" but didn't return ` +
+        `anything from your \`loader\` function. Please return a value or \`null\`.`
+    );
+  }
+
   return isResponse(result) ? result : json(result);
 }
 
diff --git a/packages/remix-server-runtime/router/index.ts b/packages/remix-server-runtime/router/index.ts
index a673cee1969..b643dd55030 100644
--- a/packages/remix-server-runtime/router/index.ts
+++ b/packages/remix-server-runtime/router/index.ts
@@ -30,14 +30,11 @@ export type {
 
 export {
   AbortedDeferredError,
-  // TODO: Is this supposed to be exported?
   ErrorResponse,
-  ErrorWithStatus,
   defer,
   generatePath,
   getToPathname,
   invariant,
-  isErrorWithStatus,
   isRouteErrorResponse,
   joinPaths,
   json,
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 3b581c66b01..c906eb157c3 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -26,7 +26,6 @@ import type {
 import {
   DeferredData,
   ErrorResponse,
-  ErrorWithStatus,
   ResultType,
   convertRoutesToDataRoutes,
   getPathContributingMatches,
@@ -571,7 +570,10 @@ export function createRouter(init: RouterInit): Router {
   if (initialMatches == null) {
     // If we do not match a user-provided-route, fall back to the root
     // to allow the error boundary to take over
-    let { matches, route, error } = getNotFoundMatches(dataRoutes);
+    let error = getInternalRouterError(404, {
+      pathname: init.history.location.pathname,
+    });
+    let { matches, route } = getShortCircuitMatches(dataRoutes);
     initialMatches = matches;
     initialErrors = { [route.id]: error };
   }
@@ -861,11 +863,9 @@ export function createRouter(init: RouterInit): Router {
 
     // Short circuit with a 404 on the root error boundary if we match nothing
     if (!matches) {
-      let {
-        matches: notFoundMatches,
-        route,
-        error,
-      } = getNotFoundMatches(dataRoutes);
+      let error = getInternalRouterError(404, { pathname: location.pathname });
+      let { matches: notFoundMatches, route } =
+        getShortCircuitMatches(dataRoutes);
       // Cancel all pending deferred on 404s since we don't keep any routes
       cancelActiveDeferreds();
       completeNavigation(location, {
@@ -979,7 +979,14 @@ export function createRouter(init: RouterInit): Router {
     let actionMatch = getTargetMatch(matches, location);
 
     if (!actionMatch.route.action) {
-      result = getMethodNotAllowedResult(location);
+      result = {
+        type: ResultType.error,
+        error: getInternalRouterError(405, {
+          method: request.method,
+          pathname: location.pathname,
+          routeId: actionMatch.route.id,
+        }),
+      };
     } else {
       result = await callLoaderOrAction(
         "action",
@@ -1211,7 +1218,11 @@ export function createRouter(init: RouterInit): Router {
 
     let matches = matchRoutes(dataRoutes, href, init.basename);
     if (!matches) {
-      setFetcherError(key, routeId, new ErrorResponse(404, "Not Found", null));
+      setFetcherError(
+        key,
+        routeId,
+        getInternalRouterError(404, { pathname: href })
+      );
       return;
     }
 
@@ -1243,7 +1254,11 @@ export function createRouter(init: RouterInit): Router {
     fetchLoadMatches.delete(key);
 
     if (!match.route.action) {
-      let { error } = getMethodNotAllowedResult(path);
+      let error = getInternalRouterError(405, {
+        method: submission.formMethod,
+        pathname: path,
+        routeId: routeId,
+      });
       setFetcherError(key, routeId, error);
       return;
     }
@@ -1867,11 +1882,9 @@ export function unstable_createStaticHandler(
     let matches = matchRoutes(dataRoutes, location);
 
     if (!validRequestMethods.has(request.method)) {
-      let {
-        matches: methodNotAllowedMatches,
-        route,
-        error,
-      } = getMethodNotAllowedMatches(dataRoutes);
+      let error = getInternalRouterError(405, { method: request.method });
+      let { matches: methodNotAllowedMatches, route } =
+        getShortCircuitMatches(dataRoutes);
       return {
         location,
         matches: methodNotAllowedMatches,
@@ -1885,11 +1898,9 @@ export function unstable_createStaticHandler(
         actionHeaders: {},
       };
     } else if (!matches) {
-      let {
-        matches: notFoundMatches,
-        route,
-        error,
-      } = getNotFoundMatches(dataRoutes);
+      let error = getInternalRouterError(404, { pathname: location.pathname });
+      let { matches: notFoundMatches, route } =
+        getShortCircuitMatches(dataRoutes);
       return {
         location,
         matches: notFoundMatches,
@@ -1923,16 +1934,17 @@ export function unstable_createStaticHandler(
    * from the action/loader, but it may be a primitive or other value as well -
    * and in such cases the calling context should handle that accordingly.
    *
-   * TODO: Add note about thrown ErrorWithStatus
-   *
    * We do respect the throw/return differentiation, so if an action/loader
    * throws, then this method will throw the value.  This is important so we
    * can do proper boundary identification in Remix where a thrown Response
    * must go to the Catch Boundary but a returned Response is happy-path.
    *
-   * One thing to note is that any Router-initiated thrown Response (such as a
-   * 404 or 405) will have a custom X-Remix-Router-Error: "yes" header on it
-   * in order to differentiate from responses thrown from user actions/loaders.
+   * One thing to note is that any Router-initiated Errors that make sense
+   * to associate with a status code will be thrown as an ErrorResponse
+   * instance which include the raw Error, such that the calling context can
+   * serialize the error as they see fit while including the proper response
+   * code.  Examples here are 404 and 405 errors that occur prior to reaching
+   * any user-defined loaders.
    */
   async function queryRoute(request: Request, routeId?: string): Promise<any> {
     let url = new URL(request.url);
@@ -1940,15 +1952,9 @@ export function unstable_createStaticHandler(
     let matches = matchRoutes(dataRoutes, location);
 
     if (!validRequestMethods.has(request.method)) {
-      throw new ErrorWithStatus(
-        `Invalid request method "${request.method}"`,
-        405
-      );
+      throw getInternalRouterError(405, { method: request.method });
     } else if (!matches) {
-      throw new ErrorWithStatus(
-        `No route matches URL "${location.pathname}"`,
-        404
-      );
+      throw getInternalRouterError(404, { pathname: location.pathname });
     }
 
     let match = routeId
@@ -1956,16 +1962,13 @@ export function unstable_createStaticHandler(
       : getTargetMatch(matches, location);
 
     if (routeId && !match) {
-      throw new ErrorWithStatus(
-        `Route "${routeId}" does not match URL "${location.pathname}"`,
-        403
-      );
+      throw getInternalRouterError(403, {
+        pathname: location.pathname,
+        routeId,
+      });
     } else if (!match) {
       // This should never hit I don't think?
-      throw new ErrorWithStatus(
-        `No route matches URL "${createPath(location)}"`,
-        404
-      );
+      throw getInternalRouterError(404, { pathname: location.pathname });
     }
 
     let result = await queryImpl(request, location, matches, match);
@@ -2045,15 +2048,18 @@ export function unstable_createStaticHandler(
     let result: DataResult;
 
     if (!actionMatch.route.action) {
+      let error = getInternalRouterError(405, {
+        method: request.method,
+        pathname: createURL(request.url).pathname,
+        routeId: actionMatch.route.id,
+      });
       if (isRouteRequest) {
-        throw new ErrorWithStatus(
-          `You made a ${request.method} request to ${request.url} but ` +
-            `did not provide an \`action\` for route "${actionMatch.route.id}", ` +
-            `so there is no way to handle the request.`,
-          405
-        );
+        throw error;
       }
-      result = getMethodNotAllowedResult(request.url);
+      result = {
+        type: ResultType.error,
+        error,
+      };
     } else {
       result = await callLoaderOrAction(
         "action",
@@ -2092,20 +2098,7 @@ export function unstable_createStaticHandler(
       // Note: This should only be non-Response values if we get here, since
       // isRouteRequest should throw any Response received in callLoaderOrAction
       if (isErrorResult(result)) {
-        let boundaryMatch = findNearestBoundary(matches, actionMatch.route.id);
-        return {
-          matches: [actionMatch],
-          loaderData: {},
-          actionData: null,
-          errors: {
-            [boundaryMatch.route.id]: result.error,
-          },
-          // Note: statusCode + headers are unused here since queryRoute will
-          // return the raw Response or value
-          statusCode: 500,
-          loaderHeaders: {},
-          actionHeaders: {},
-        };
+        throw result.error;
       }
 
       return {
@@ -2170,12 +2163,11 @@ export function unstable_createStaticHandler(
 
     // Short circuit if we have no loaders to run (queryRoute())
     if (isRouteRequest && !routeMatch?.route.loader) {
-      throw new ErrorWithStatus(
-        `You made a ${request.method} request to ${request.url} but ` +
-          `did not provide a \`loader\` for route "${routeMatch?.route.id}", ` +
-          `so there is no way to handle the request.`,
-        405
-      );
+      throw getInternalRouterError(405, {
+        method: request.method,
+        pathname: createURL(request.url).pathname,
+        routeId: routeMatch?.route.id,
+      });
     }
 
     let requestMatches = routeMatch
@@ -2325,11 +2317,7 @@ function normalizeNavigateOptions(
   } catch (e) {
     return {
       path,
-      error: new ErrorResponse(
-        400,
-        "Bad Request",
-        "Cannot submit binary form data using GET"
-      ),
+      error: getInternalRouterError(400),
     };
   }
 
@@ -2543,6 +2531,13 @@ async function callLoaderOrAction(
       handler({ request, params: match.params }),
       abortPromise,
     ]);
+
+    invariant(
+      result !== undefined,
+      `You defined ${type === "action" ? "an action" : "a loader"} for route ` +
+        `"${match.route.id}" but didn't return anything from your \`${type}\` ` +
+        `function. Please return a value or \`null\`.`
+    );
   } catch (e) {
     resultType = ResultType.error;
     result = e;
@@ -2619,12 +2614,6 @@ async function callLoaderOrAction(
     }
 
     if (resultType === ResultType.error) {
-      // TODO: Check if this is breaking for remix.  If the unwrapped data is
-      // of the format { message: string, stack: string } convert it back to
-      // new Error() here.  This might be a non-issue since the Remix client-side
-      // loader implementation might do the unwrapping and converson and throw
-      // new Error in which case we don't come this code path since it's not a
-      // thrown Response.
       return {
         type: resultType,
         error: new ErrorResponse(status, result.statusText, data),
@@ -2869,18 +2858,13 @@ function findNearestBoundary(
   );
 }
 
-function getShortCircuitMatches(
-  routes: AgnosticDataRouteObject[],
-  status: number,
-  statusText: string
-): {
+function getShortCircuitMatches(routes: AgnosticDataRouteObject[]): {
   matches: AgnosticDataRouteMatch[];
   route: AgnosticDataRouteObject;
-  error: ErrorResponse;
 } {
   // Prefer a root layout route if present, otherwise shim in a route object
   let route = routes.find((r) => r.index || !r.path || r.path === "/") || {
-    id: `__shim-${status}-route__`,
+    id: `__shim-error-route__`,
   };
 
   return {
@@ -2893,29 +2877,63 @@ function getShortCircuitMatches(
       },
     ],
     route,
-    error: new ErrorResponse(status, statusText, null),
   };
 }
 
-function getNotFoundMatches(routes: AgnosticDataRouteObject[]) {
-  return getShortCircuitMatches(routes, 404, "Not Found");
-}
-
-function getMethodNotAllowedMatches(routes: AgnosticDataRouteObject[]) {
-  return getShortCircuitMatches(routes, 405, "Method Not Allowed");
-}
+function getInternalRouterError(
+  status: number,
+  {
+    pathname,
+    routeId,
+    method,
+    message,
+  }: {
+    pathname?: string;
+    routeId?: string;
+    method?: string;
+    message?: string;
+  } = {}
+) {
+  let statusText: string;
+  let errorMessage = message;
+
+  if (status === 400) {
+    statusText = "Bad Request";
+    errorMessage = "Cannot submit binary form data using GET";
+  } else if (status === 403) {
+    statusText = "Forbidden";
+    errorMessage = `Route "${routeId}" does not match URL "${pathname}"`;
+  } else if (status === 404) {
+    statusText = "Not Found";
+    errorMessage = `No route matches URL "${pathname}"`;
+  } else if (status === 405) {
+    statusText = "Method Not Allowed";
+    if (method && pathname && routeId) {
+      if (validActionMethods.has(method)) {
+        errorMessage =
+          `You made a ${method} request to "${pathname}" but ` +
+          `did not provide an \`action\` for route "${routeId}", ` +
+          `so there is no way to handle the request.`;
+      } else {
+        errorMessage =
+          `You made a ${method} request to "${pathname}" but ` +
+          `did not provide a \`loader\` for route "${routeId}", ` +
+          `so there is no way to handle the request.`;
+      }
+    } else {
+      errorMessage = `Invalid request method "${method}"`;
+    }
+  } else {
+    statusText = "Unknown Server Error";
+    errorMessage = "Unknown @remix-run/router error";
+  }
 
-function getMethodNotAllowedResult(path: Location | string): ErrorResult {
-  let href = typeof path === "string" ? path : createPath(path);
-  console.warn(
-    "You're trying to submit to a route that does not have an action.  To " +
-      "fix this, please add an `action` function to the route for " +
-      `[${href}]`
+  return new ErrorResponse(
+    status || 500,
+    statusText,
+    new Error(errorMessage),
+    true
   );
-  return {
-    type: ResultType.error,
-    error: new ErrorResponse(405, "Method Not Allowed", ""),
-  };
 }
 
 // Find any returned redirect errors, starting from the lowest match
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index 47d7af1415c..b6e0ad39011 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -1245,11 +1245,24 @@ export class ErrorResponse {
   status: number;
   statusText: string;
   data: any;
-
-  constructor(status: number, statusText: string | undefined, data: any) {
+  error?: Error;
+  internal: boolean;
+
+  constructor(
+    status: number,
+    statusText: string | undefined,
+    data: any,
+    internal = false
+  ) {
     this.status = status;
     this.statusText = statusText || "";
-    this.data = data;
+    this.internal = internal;
+    if (data instanceof Error) {
+      this.data = data.toString();
+      this.error = data;
+    } else {
+      this.data = data;
+    }
   }
 }
 
@@ -1260,21 +1273,3 @@ export class ErrorResponse {
 export function isRouteErrorResponse(e: any): e is ErrorResponse {
   return e instanceof ErrorResponse;
 }
-
-/**
- * @private
- * Utility class we use to hold thrown Errors that are status-code-aware
- */
-export class ErrorWithStatus extends Error {
-  constructor(public msg: string, public status: number) {
-    super(msg);
-  }
-}
-
-/**
- * Check if the given error is an ErrorResponse generated from a 4xx/5xx
- * Response throw from an action/loader
- */
-export function isErrorWithStatus(e: any): e is ErrorWithStatus {
-  return e instanceof ErrorWithStatus;
-}
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 87ef0a51510..9be5bdb00c7 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -1,10 +1,6 @@
 // TODO: RRR - Change import to @remix-run/router
-import type { StaticHandler, StaticHandlerContext } from "./router";
-import {
-  ErrorWithStatus,
-  isErrorWithStatus,
-  isRouteErrorResponse,
-} from "./router";
+import { ErrorResponse, StaticHandler, StaticHandlerContext } from "./router";
+import { isRouteErrorResponse } from "./router";
 import { unstable_createStaticHandler } from "./router";
 import type { AppLoadContext } from "./data";
 import { callRouteAction, callRouteLoader, extractData } from "./data";
@@ -183,28 +179,42 @@ async function handleDataRequest({
 
   try {
     if (!isValidRequestMethod(request)) {
-      throw new ErrorWithStatus(
-        `Invalid request method "${request.method}"`,
-        405
+      throw new ErrorResponse(
+        405,
+        "Method Not Allowed",
+        new Error(`Invalid request method "${request.method}"`),
+        true
       );
     }
 
     let url = new URL(request.url);
 
     if (!matches) {
-      throw new ErrorWithStatus(`No route matches URL "${url.pathname}"`, 404);
+      throw new ErrorResponse(
+        404,
+        "Not Found",
+        new Error(`No route matches URL "${url.pathname}"`),
+        true
+      );
     }
 
     let routeId = url.searchParams.get("_data");
     if (!routeId) {
-      throw new ErrorWithStatus(`Missing route id in ?_data`, 403);
+      throw new ErrorResponse(
+        403,
+        "Forbidden",
+        new Error(`Missing route id in ?_data`),
+        true
+      );
     }
 
     let tempMatch = matches.find((match) => match.route.id === routeId);
     if (!tempMatch) {
-      throw new ErrorWithStatus(
-        `Route "${routeId}" does not match URL "${url.pathname}"`,
-        403
+      throw new ErrorResponse(
+        403,
+        "Forbidden",
+        new Error(`Route "${routeId}" does not match URL "${url.pathname}"`),
+        true
       );
     }
     match = tempMatch;
@@ -249,13 +259,23 @@ async function handleDataRequest({
 
     return response;
   } catch (error: unknown) {
+    let status = 500;
+    let errorInstance = error;
+
+    if (isRouteErrorResponse(error)) {
+      status = error.status;
+      errorInstance = error.error || errorInstance;
+    }
+
     if (serverMode !== ServerMode.Test) {
-      console.error(error);
+      console.error(errorInstance);
     }
 
-    let status = isErrorWithStatus(error) ? error.status : 500;
-    if (serverMode === ServerMode.Development && error instanceof Error) {
-      return errorBoundaryError(error, status);
+    if (
+      serverMode === ServerMode.Development &&
+      errorInstance instanceof Error
+    ) {
+      return errorBoundaryError(errorInstance, status);
     }
 
     return errorBoundaryError(new Error("Unexpected Server Error"), status);
@@ -295,13 +315,23 @@ async function handleDataRequestRR(
       return error;
     }
 
+    let status = 500;
+    let errorInstance = error;
+
+    if (isRouteErrorResponse(error)) {
+      status = error.status;
+      errorInstance = error.error || errorInstance;
+    }
+
     if (serverMode !== ServerMode.Test) {
-      console.error(error);
+      console.error(errorInstance);
     }
 
-    let status = isErrorWithStatus(error) ? error.status : 500;
-    if (serverMode === ServerMode.Development && error instanceof Error) {
-      return errorBoundaryError(error, status);
+    if (
+      serverMode === ServerMode.Development &&
+      errorInstance instanceof Error
+    ) {
+      return errorBoundaryError(errorInstance, status);
     }
 
     return errorBoundaryError(new Error("Unexpected Server Error"), status);
@@ -318,7 +348,11 @@ function findParentBoundary(
   // react-router doesn't have a matching "root" route to assign the error to
   // so it returns context.errors = { __shim-404-route__: ErrorResponse }
   let route = routes[routeId] || routes["root"];
-  let isCatch = isRouteErrorResponse(error);
+  // Router-thrown ErrorResponses will have the error instance.  User-thrown
+  // Responses will not have an error. The one exception here is internal 404s
+  // which we handle the same as user-thrown 404s
+  let isCatch =
+    isRouteErrorResponse(error) && (!error.error || error.status === 404);
   if (
     (isCatch && route.module.CatchBoundary) ||
     (!isCatch && route.module.ErrorBoundary) ||
@@ -392,13 +426,37 @@ async function handleDocumentRequestRR(
     if (!error) {
       continue;
     } else if (isRouteErrorResponse(error)) {
+      // Internal Remix errors will throw an ErrorResponse with the actual error
+      if (error.error && error.status !== 404) {
+        if (build.routes[id]?.module.ErrorBoundary) {
+          appState.loaderBoundaryRouteId = id;
+        }
+        appState.trackBoundaries = false;
+        appState.error = await serializeError(error.error);
+
+        if (
+          error.status === 405 &&
+          error.error.message.includes("Invalid request method")
+        ) {
+          // Note: Emptying this out ensures our response matches the Remix-flow
+          // exactly, but functionally both end up using the root error boundary
+          // if it exists.  Might be able to clean this up eventually.
+          context.matches = [];
+        }
+        break;
+      }
+
+      // ErrorResponse's without an error were thrown by the user action/loader
       if (build.routes[id]?.module.CatchBoundary) {
         appState.catchBoundaryRouteId = id;
       }
       appState.trackCatchBoundaries = false;
       appState.catch = {
         // Order is important for response matching assertions!
-        data: error.data,
+        data:
+          error.error && error.status === 404
+            ? error.error.message
+            : error.data,
         status: error.status,
         statusText: error.statusText,
       };
@@ -502,26 +560,29 @@ async function handleDocumentRequest({
     catch: undefined,
   };
 
+  let errorStatus = 500;
+
   if (!isValidRequestMethod(request)) {
     matches = null;
-    appState.trackCatchBoundaries = false;
-    appState.catch = {
-      data: null,
-      status: 405,
-      statusText: "Method Not Allowed",
-    };
+    appState.loaderBoundaryRouteId = routes[0].module.ErrorBoundary
+      ? routes[0].id
+      : null;
+    appState.trackBoundaries = false;
+    errorStatus = 405;
+    appState.error = await serializeError(
+      new Error(`Invalid request method "${request.method}"`)
+    );
   } else if (!matches) {
     // TODO: Double check that we have integration tests asserting that this
     // is a 404 catch boundary
     appState.trackCatchBoundaries = false;
     appState.catch = {
-      data: null,
+      data: `No route matches URL "${new URL(request.url).pathname}"`,
       status: 404,
       statusText: "Not Found",
     };
   }
 
-  let errorStatus = 500;
   let actionStatus: { status: number; statusText: string } | undefined;
   let actionData: Record<string, unknown> | undefined;
   let actionMatch: RouteMatch<ServerRoute> | undefined;
@@ -565,13 +626,18 @@ async function handleDocumentRequest({
         };
       }
     } catch (error: any) {
-      errorStatus = isErrorWithStatus(error) ? error.status : 500;
+      let errorInstance = error;
+      if (isRouteErrorResponse(error)) {
+        errorStatus = error.status;
+        errorInstance = error.error || errorInstance;
+      }
+
       appState.loaderBoundaryRouteId = getDeepestRouteIdWithBoundary(
         matches,
         "ErrorBoundary"
       );
       appState.trackBoundaries = false;
-      appState.error = await serializeError(error);
+      appState.error = await serializeError(errorInstance);
 
       if (serverMode !== ServerMode.Test) {
         console.error(
@@ -673,9 +739,15 @@ async function handleDocumentRequest({
     }
 
     if (error) {
-      loaderStatusCodes.push(isErrorWithStatus(error) ? error.status : 500);
       appState.trackBoundaries = false;
-      appState.error = await serializeError(error);
+
+      if (isRouteErrorResponse(error) && error.error) {
+        loaderStatusCodes.push(error.status);
+        appState.error = await serializeError(error.error);
+      } else {
+        loaderStatusCodes.push(500);
+        appState.error = await serializeError(error);
+      }
 
       if (serverMode !== ServerMode.Test) {
         console.error(
@@ -825,6 +897,9 @@ async function handleResourceRequestRR(
     return response;
   } catch (error) {
     if (error instanceof Response) {
+      // Note: Not functionally required but ensures that our response headers
+      // match identically to what Remix returns
+      error.headers.set("X-Remix-Catch", "yes");
       return error;
     }
     return returnLastResortErrorResponse(error, serverMode);
@@ -974,6 +1049,7 @@ async function assert(
 ) {
   let aStr = JSON.stringify(await accessor(a));
   let bStr = JSON.stringify(await accessor(b));
+
   if (aStr !== bStr) {
     console.error(message);
     message += "\nResponse 1:\n" + aStr + "\nResponse 2:\n" + bStr;

From b93e0c7698fe79b70c61a30f7522ad7305572bc8 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Mon, 31 Oct 2022 17:26:50 -0400
Subject: [PATCH 21/30] Fix lint issues and one broken unit test

---
 packages/remix-server-runtime/__tests__/server-test.ts | 2 +-
 packages/remix-server-runtime/router/history.ts        | 1 +
 packages/remix-server-runtime/router/router.ts         | 2 +-
 packages/remix-server-runtime/router/utils.ts          | 2 +-
 packages/remix-server-runtime/server.ts                | 4 ++--
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index abda5b1a1e1..6bf75528317 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -410,7 +410,7 @@ describe("shared server runtime", () => {
       });
 
       let result = await handler(request);
-      expect(result.status).toBe(500);
+      expect(result.status).toBe(405);
       expect(result.headers.get("X-Remix-Error")).toBe("yes");
       expect((await result.json()).message).toBeTruthy();
     });
diff --git a/packages/remix-server-runtime/router/history.ts b/packages/remix-server-runtime/router/history.ts
index e63e83026b1..1113ac6cda4 100644
--- a/packages/remix-server-runtime/router/history.ts
+++ b/packages/remix-server-runtime/router/history.ts
@@ -1,4 +1,5 @@
 // @ts-nocheck
+/* eslint-disable */
 
 ////////////////////////////////////////////////////////////////////////////////
 //#region Types and Constants
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index c906eb157c3..befb71e0b1e 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -1,5 +1,5 @@
 // @ts-nocheck
-// eslint-disable
+/* eslint-disable */
 import type { History, Location, To } from "./history";
 import {
   Action as HistoryAction,
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index b6e0ad39011..fb633293f3e 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -1,5 +1,5 @@
 // @ts-nocheck
-// eslint-disable
+/* eslint-disable */
 import type { Location, Path, To } from "./history";
 import { parsePath } from "./history";
 
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 9be5bdb00c7..61d52f04fa4 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -1,6 +1,6 @@
 // TODO: RRR - Change import to @remix-run/router
-import { ErrorResponse, StaticHandler, StaticHandlerContext } from "./router";
-import { isRouteErrorResponse } from "./router";
+import type { StaticHandler, StaticHandlerContext } from "./router";
+import { ErrorResponse, isRouteErrorResponse } from "./router";
 import { unstable_createStaticHandler } from "./router";
 import type { AppLoadContext } from "./data";
 import { callRouteAction, callRouteLoader, extractData } from "./data";

From c1ae7a9ec000ec2ae14c4d089d6cbda77857875b Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Mon, 31 Oct 2022 17:40:59 -0400
Subject: [PATCH 22/30] Fix tsc build error

---
 packages/remix-server-runtime/routes.ts | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
index b512746466f..f4eed8f7b2a 100644
--- a/packages/remix-server-runtime/routes.ts
+++ b/packages/remix-server-runtime/routes.ts
@@ -71,17 +71,21 @@ export function createStaticHandlerDataRoutes(
         loader: route.module.loader
           ? (args: LoaderFunctionArgs) =>
               callRouteLoaderRR({
-                ...args,
-                loader: route.module.loader!,
                 loadContext,
+                loader: route.module.loader!,
+                params: args.params,
+                request: args.request,
+                routeId: route.id,
               })
           : undefined,
         action: route.module.action
           ? (args: ActionFunctionArgs) =>
               callRouteActionRR({
-                ...args,
-                action: route.module.action!,
                 loadContext,
+                action: route.module.action!,
+                params: args.params,
+                request: args.request,
+                routeId: route.id,
               })
           : undefined,
         handle: route.module.handle,

From 5d1c6d17b6e3a41ca5f27854ebeae50eeb9aa13a Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 1 Nov 2022 14:54:59 -0400
Subject: [PATCH 23/30] make fetcher tests more reliable

---
 integration/fetcher-test.ts | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/integration/fetcher-test.ts b/integration/fetcher-test.ts
index 39e6534a06b..b7916692dd1 100644
--- a/integration/fetcher-test.ts
+++ b/integration/fetcher-test.ts
@@ -168,7 +168,7 @@ test.describe("useFetcher", () => {
         }),
       ]);
 
-      expect(await app.getHtml("pre")).toMatch(LUNCH);
+      await page.waitForSelector(`pre:has-text("${LUNCH}")`);
     });
 
     test("Form can hit an action", async ({ page }) => {
@@ -181,7 +181,7 @@ test.describe("useFetcher", () => {
           method: "post",
         }),
       ]);
-      expect(await app.getHtml("pre")).toMatch(CHEESESTEAK);
+      await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
     });
   });
 
@@ -189,21 +189,21 @@ test.describe("useFetcher", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickElement("#fetcher-load");
-    expect(await app.getHtml("pre")).toMatch(LUNCH);
+    await page.waitForSelector(`pre:has-text("${LUNCH}")`);
   });
 
   test("submit can hit an action", async ({ page }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickElement("#fetcher-submit");
-    expect(await app.getHtml("pre")).toMatch(CHEESESTEAK);
+    await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
   });
 
   test("submit can hit an action only route", async ({ page }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/fetcher-action-only-call");
     await app.clickElement("#fetcher-submit");
-    expect(await app.getHtml("pre")).toMatch(CHEESESTEAK);
+    await page.waitForSelector(`pre:has-text("${CHEESESTEAK}")`);
   });
 
   test("fetchers handle ?index param correctly", async ({ page }) => {
@@ -211,25 +211,25 @@ test.describe("useFetcher", () => {
     await app.goto("/parent");
 
     await app.clickElement("#load-parent");
-    expect(await app.getHtml("pre")).toMatch(PARENT_LAYOUT_LOADER);
+    await page.waitForSelector(`pre:has-text("${PARENT_LAYOUT_LOADER}")`);
 
     await app.clickElement("#load-index");
-    expect(await app.getHtml("pre")).toMatch(PARENT_INDEX_LOADER);
+    await page.waitForSelector(`pre:has-text("${PARENT_INDEX_LOADER}")`);
 
     // fetcher.submit({}) defaults to GET for the current Route
     await app.clickElement("#submit-empty");
-    expect(await app.getHtml("pre")).toMatch(PARENT_INDEX_LOADER);
+    await page.waitForSelector(`pre:has-text("${PARENT_INDEX_LOADER}")`);
 
     await app.clickElement("#submit-parent-get");
-    expect(await app.getHtml("pre")).toMatch(PARENT_LAYOUT_LOADER);
+    await page.waitForSelector(`pre:has-text("${PARENT_LAYOUT_LOADER}")`);
 
     await app.clickElement("#submit-index-get");
-    expect(await app.getHtml("pre")).toMatch(PARENT_INDEX_LOADER);
+    await page.waitForSelector(`pre:has-text("${PARENT_INDEX_LOADER}")`);
 
     await app.clickElement("#submit-parent-post");
-    expect(await app.getHtml("pre")).toMatch(PARENT_LAYOUT_ACTION);
+    await page.waitForSelector(`pre:has-text("${PARENT_LAYOUT_ACTION}")`);
 
     await app.clickElement("#submit-index-post");
-    expect(await app.getHtml("pre")).toMatch(PARENT_INDEX_ACTION);
+    await page.waitForSelector(`pre:has-text("${PARENT_INDEX_ACTION}")`);
   });
 });

From e6e7f9dacf3e5874c5b9abda5c7f98c79016dc73 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 1 Nov 2022 17:14:40 -0400
Subject: [PATCH 24/30] =?UTF-8?q?=F0=9F=A7=B9=20code?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 integration/catch-boundary-test.ts            | 52 ------------
 integration/error-boundary-test.ts            | 80 +++++++++++++------
 .../__tests__/server-test.ts                  |  4 -
 packages/remix-server-runtime/data.ts         | 14 ----
 packages/remix-server-runtime/routes.ts       |  6 +-
 packages/remix-server-runtime/server.ts       | 51 ++++++------
 6 files changed, 83 insertions(+), 124 deletions(-)

diff --git a/integration/catch-boundary-test.ts b/integration/catch-boundary-test.ts
index ec38c2eeed3..82932111b7c 100644
--- a/integration/catch-boundary-test.ts
+++ b/integration/catch-boundary-test.ts
@@ -13,10 +13,8 @@ test.describe("CatchBoundary", () => {
 
   let HAS_BOUNDARY_LOADER = "/yes/loader";
   let HAS_BOUNDARY_ACTION = "/yes/action";
-  let HAS_BOUNDARY_NO_LOADER_OR_ACTION = "/yes/no-loader-or-action";
   let NO_BOUNDARY_ACTION = "/no/action";
   let NO_BOUNDARY_LOADER = "/no/loader";
-  let NO_BOUNDARY_NO_LOADER_OR_ACTION = "/no/no-loader-or-action";
 
   let NOT_FOUND_HREF = "/not/found";
 
@@ -64,8 +62,6 @@ test.describe("CatchBoundary", () => {
                 <Form method="post">
                   <button formAction="${HAS_BOUNDARY_ACTION}" type="submit" />
                   <button formAction="${NO_BOUNDARY_ACTION}" type="submit" />
-                  <button formAction="${HAS_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit" />
-                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit" />
                 </Form>
 
                 <Link to="${HAS_BOUNDARY_LOADER}">
@@ -79,39 +75,6 @@ test.describe("CatchBoundary", () => {
           }
         `,
 
-        "app/routes/fetcher-boundary.jsx": js`
-          import { useFetcher } from "@remix-run/react";
-          export function CatchBoundary() {
-            return <p id="fetcher-boundary">${OWN_BOUNDARY_TEXT}</p>
-          }
-          export default function() {
-            let fetcher = useFetcher();
-
-            return (
-              <div>
-                <fetcher.Form method="post">
-                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit" />
-                </fetcher.Form>
-              </div>
-            )
-          }
-        `,
-
-        "app/routes/fetcher-no-boundary.jsx": js`
-          import { useFetcher } from "@remix-run/react";
-          export default function() {
-            let fetcher = useFetcher();
-
-            return (
-              <div>
-                <fetcher.Form method="post">
-                  <button formAction="${NO_BOUNDARY_NO_LOADER_OR_ACTION}" type="submit" />
-                </fetcher.Form>
-              </div>
-            )
-          }
-        `,
-
         [`app/routes${HAS_BOUNDARY_ACTION}.jsx`]: js`
           import { Form } from "@remix-run/react";
           export async function action() {
@@ -147,21 +110,6 @@ test.describe("CatchBoundary", () => {
           }
         `,
 
-        [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
-          export function CatchBoundary() {
-            return <div id="boundary-no-loader-or-action">${OWN_BOUNDARY_TEXT}</div>
-          }
-          export default function Index() {
-            return <div/>
-          }
-        `,
-
-        [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
-          export default function Index() {
-            return <div/>
-          }
-        `,
-
         [`app/routes${HAS_BOUNDARY_LOADER}.jsx`]: js`
           export function loader() {
             throw new Response("", { status: 401 })
diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index 5d10a2fd3e8..04b9bea72bc 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -180,12 +180,6 @@ test.describe("ErrorBoundary", () => {
           }
         `,
 
-        [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
-          export default function Index() {
-            return <div/>
-          }
-        `,
-
         [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION}.jsx`]: js`
           export function ErrorBoundary() {
             return <div id="boundary-no-loader-or-action">${OWN_BOUNDARY_TEXT}</div>
@@ -473,6 +467,7 @@ test.describe("ErrorBoundary", () => {
   test.describe("if no error boundary exists in the app", () => {
     let NO_ROOT_BOUNDARY_LOADER = "/loader-bad";
     let NO_ROOT_BOUNDARY_ACTION = "/action-bad";
+    let NO_ROOT_BOUNDARY_LOADER_RETURN = "/loader-no-return";
     let NO_ROOT_BOUNDARY_ACTION_RETURN = "/action-no-return";
 
     test.beforeAll(async () => {
@@ -481,20 +476,20 @@ test.describe("ErrorBoundary", () => {
           "app/root.jsx": js`
             import { Links, Meta, Outlet, Scripts } from "@remix-run/react";
 
-          export default function Root() {
-            return (
-              <html lang="en">
-                <head>
-                  <Meta />
-                  <Links />
-                </head>
-                <body>
-                  <Outlet />
-                  <Scripts />
-                </body>
-              </html>
-            );
-          }
+            export default function Root() {
+              return (
+                <html lang="en">
+                  <head>
+                    <Meta />
+                    <Links />
+                  </head>
+                  <body>
+                    <Outlet />
+                    <Scripts />
+                  </body>
+                </html>
+              );
+            }
           `,
 
           "app/routes/index.jsx": js`
@@ -504,6 +499,7 @@ test.describe("ErrorBoundary", () => {
               return (
                 <div>
                   <h1>Home</h1>
+                  <Link to="${NO_ROOT_BOUNDARY_LOADER_RETURN}">Loader no return</Link>
                   <Form method="post">
                     <button formAction="${NO_ROOT_BOUNDARY_ACTION}" type="submit">
                       Action go boom
@@ -518,8 +514,6 @@ test.describe("ErrorBoundary", () => {
           `,
 
           [`app/routes${NO_ROOT_BOUNDARY_LOADER}.jsx`]: js`
-            import { Link, Form } from "@remix-run/react";
-
             export async function loader() {
               throw Error("BLARGH");
             }
@@ -534,8 +528,6 @@ test.describe("ErrorBoundary", () => {
           `,
 
           [`app/routes${NO_ROOT_BOUNDARY_ACTION}.jsx`]: js`
-            import { Link, Form } from "@remix-run/react";
-
             export async function action() {
               throw Error("YOOOOOOOO WHAT ARE YOU DOING");
             }
@@ -549,8 +541,23 @@ test.describe("ErrorBoundary", () => {
             }
           `,
 
+          [`app/routes${NO_ROOT_BOUNDARY_LOADER_RETURN}.jsx`]: js`
+            import { useLoaderData } from "@remix-run/react";
+
+            export async function loader() {}
+
+            export default function () {
+              let data = useLoaderData();
+              return (
+                <div>
+                  <h1>{data}</h1>
+                </div>
+              )
+            }
+          `,
+
           [`app/routes${NO_ROOT_BOUNDARY_ACTION_RETURN}.jsx`]: js`
-            import { Link, Form, useActionData } from "@remix-run/react";
+            import { useActionData } from "@remix-run/react";
 
             export async function action() {}
 
@@ -585,6 +592,29 @@ test.describe("ErrorBoundary", () => {
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
 
+    test("bubbles to internal boundary if loader doesn't return (document requests)", async () => {
+      let res = await fixture.requestDocument(NO_ROOT_BOUNDARY_LOADER_RETURN);
+      expect(res.status).toBe(500);
+      expect(await res.text()).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
+    });
+
+    test("bubbles to internal boundary if loader doesn't return", async ({
+      page,
+    }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/");
+      await app.clickLink(NO_ROOT_BOUNDARY_LOADER_RETURN);
+      expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
+    });
+
+    test("bubbles to internal boundary if action doesn't return (document requests)", async () => {
+      let res = await fixture.requestDocument(NO_ROOT_BOUNDARY_ACTION_RETURN, {
+        method: "post",
+      });
+      expect(res.status).toBe(500);
+      expect(await res.text()).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
+    });
+
     test("bubbles to internal boundary if action doesn't return", async ({
       page,
     }) => {
diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index 6bf75528317..75024ca1620 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -1180,10 +1180,6 @@ describe("shared server runtime", () => {
       expect(result.status).toBe(400);
       expect(indexAction.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
       expect(rootLoader.mock.calls.length).toBe(1 * DATA_CALL_MULTIPIER);
-      // TODO: Check this out. Seems to be a gap in not being able to let
-      // RR know that a thrown response we catch should be treated the same
-      // as an error in terms of what subsequent loaders are called after an
-      // action throws a response.
       expect(indexLoader.mock.calls.length).toBe(0);
       expect(build.entry.module.default.mock.calls.length).toBe(
         1 * DATA_CALL_MULTIPIER
diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index 2b52a4a54a4..5f62c0ec355 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -25,14 +25,12 @@ export async function callRouteAction({
   action,
   params,
   request,
-  isRemixRouterRequest,
 }: {
   loadContext: AppLoadContext;
   routeId: string;
   action?: ActionFunction;
   params: DataFunctionArgs["params"];
   request: Request;
-  isRemixRouterRequest?: boolean;
 }) {
   if (!action) {
     let pathname = new URL(request.url).pathname;
@@ -56,11 +54,6 @@ export async function callRouteAction({
 
     if (!isRedirectResponse(error)) {
       error.headers.set("X-Remix-Catch", "yes");
-      // This needs to be thrown so @remix-run/router knows to handle it as an error
-      // and not a successful returned response
-      if (isRemixRouterRequest) {
-        throw error;
-      }
     }
     result = error;
   }
@@ -81,14 +74,12 @@ export async function callRouteLoader({
   loader,
   params,
   request,
-  isRemixRouterRequest,
 }: {
   request: Request;
   routeId: string;
   loader?: LoaderFunction;
   params: DataFunctionArgs["params"];
   loadContext: AppLoadContext;
-  isRemixRouterRequest?: boolean;
 }) {
   if (!loader) {
     let pathname = new URL(request.url).pathname;
@@ -112,11 +103,6 @@ export async function callRouteLoader({
 
     if (!isRedirectResponse(error)) {
       error.headers.set("X-Remix-Catch", "yes");
-      // This needs to be thrown so @remix-run/router knows to handle it as an error
-      // and not a successful returned response
-      if (isRemixRouterRequest) {
-        throw error;
-      }
     }
     result = error;
   }
diff --git a/packages/remix-server-runtime/routes.ts b/packages/remix-server-runtime/routes.ts
index f4eed8f7b2a..e1bb2daf0d8 100644
--- a/packages/remix-server-runtime/routes.ts
+++ b/packages/remix-server-runtime/routes.ts
@@ -71,20 +71,18 @@ export function createStaticHandlerDataRoutes(
         loader: route.module.loader
           ? (args: LoaderFunctionArgs) =>
               callRouteLoaderRR({
+                ...args,
                 loadContext,
                 loader: route.module.loader!,
-                params: args.params,
-                request: args.request,
                 routeId: route.id,
               })
           : undefined,
         action: route.module.action
           ? (args: ActionFunctionArgs) =>
               callRouteActionRR({
+                ...args,
                 loadContext,
                 action: route.module.action!,
-                params: args.params,
-                request: args.request,
                 routeId: route.id,
               })
           : undefined,
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 213e53f18aa..c812a35b870 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -220,7 +220,6 @@ async function handleDataRequest({
     match = tempMatch;
 
     if (isActionRequest(request)) {
-      // TODO: Can this go away??
       match = getRequestMatch(url, matches);
 
       response = await callRouteAction({
@@ -346,7 +345,7 @@ function findParentBoundary(
   // Fall back to the root route if we don't match any routes, since Remix
   // has default error/catch boundary handling.  This handles the case where
   // react-router doesn't have a matching "root" route to assign the error to
-  // so it returns context.errors = { __shim-404-route__: ErrorResponse }
+  // so it returns context.errors = { __shim-error-route__: ErrorResponse }
   let route = routes[routeId] || routes["root"];
   // Router-thrown ErrorResponses will have the error instance.  User-thrown
   // Responses will not have an error. The one exception here is internal 404s
@@ -374,17 +373,17 @@ export function differentiateCatchVersusErrorBoundaries(
   build: ServerBuild,
   context: StaticHandlerContext
 ) {
-  // TODO: This may have issues if multiple things throw?  We may need to walk
-  // bottom up doing this tso higher errors take precedence?
-  if (context.errors) {
-    let errors: Record<string, any> = {};
-    for (let routeId of Object.keys(context.errors)) {
-      let error = context.errors[routeId];
-      let handlingRouteId = findParentBoundary(build.routes, routeId, error);
-      errors[handlingRouteId] = error;
-    }
-    context.errors = errors;
+  if (!context.errors) {
+    return;
+  }
+
+  let errors: Record<string, any> = {};
+  for (let routeId of Object.keys(context.errors)) {
+    let error = context.errors[routeId];
+    let handlingRouteId = findParentBoundary(build.routes, routeId, error);
+    errors[handlingRouteId] = error;
   }
+  context.errors = errors;
 }
 
 async function handleDocumentRequestRR(
@@ -399,8 +398,7 @@ async function handleDocumentRequestRR(
     return context;
   }
 
-  // This should properly restructure context.errors to the right Catch/Error
-  // Boundary.
+  // Restructure context.errors to the right Catch/Error Boundary
   differentiateCatchVersusErrorBoundaries(build, context);
 
   let appState: AppState = {
@@ -422,13 +420,18 @@ async function handleDocumentRequestRR(
     let route = match.route as ServerRoute;
     let id = route.id;
     let error = context.errors?.[id];
+    let hasCatchBoundary = build.routes[id]?.module.CatchBoundary != null;
+    let hasErrorBoundary = build.routes[id]?.module.ErrorBoundary != null;
 
     if (!error) {
       continue;
     } else if (isRouteErrorResponse(error)) {
-      // Internal Remix errors will throw an ErrorResponse with the actual error
-      if (error.error && error.status !== 404) {
-        if (build.routes[id]?.module.ErrorBoundary) {
+      // Internal Router errors will throw an ErrorResponse with the actual
+      // error instance, while user-thrown ErrorResponses will not have the
+      // instance.  We also exclude 404s so we can handle them as CatchBoundary
+      // errors so the user has a singular location for 404 UI
+      if (error.internal && error.error && error.status !== 404) {
+        if (hasErrorBoundary) {
           appState.loaderBoundaryRouteId = id;
         }
         appState.trackBoundaries = false;
@@ -447,7 +450,7 @@ async function handleDocumentRequestRR(
       }
 
       // ErrorResponse's without an error were thrown by the user action/loader
-      if (build.routes[id]?.module.CatchBoundary) {
+      if (hasCatchBoundary) {
         appState.catchBoundaryRouteId = id;
       }
       appState.trackCatchBoundaries = false;
@@ -462,7 +465,7 @@ async function handleDocumentRequestRR(
       };
       break;
     } else {
-      if (build.routes[id]?.module.ErrorBoundary) {
+      if (hasErrorBoundary) {
         appState.loaderBoundaryRouteId = id;
       }
       appState.trackBoundaries = false;
@@ -573,8 +576,6 @@ async function handleDocumentRequest({
       new Error(`Invalid request method "${request.method}"`)
     );
   } else if (!matches) {
-    // TODO: Double check that we have integration tests asserting that this
-    // is a 404 catch boundary
     appState.trackCatchBoundaries = false;
     appState.catch = {
       data: `No route matches URL "${new URL(request.url).pathname}"`,
@@ -878,8 +879,6 @@ async function handleDocumentRequest({
   }
 }
 
-// TODO: Check on error in Remix if a loader returns undefined
-
 async function handleResourceRequestRR(
   serverMode: ServerMode,
   staticHandler: StaticHandler,
@@ -887,9 +886,11 @@ async function handleResourceRequestRR(
   request: Request
 ) {
   try {
-    // TODO remove routeId here and no need to match above for new flow
+    // Note we keep the routeId here to align with the Remix handling of
+    // resource routes which doesn't take ?index into account and just takes
+    // the leaf match
     let response = await staticHandler.queryRoute(request, routeId);
-    // Remix should always be returning responses from loaders and actions
+    // callRouteLoader/callRouteAction always return responses
     invariant(
       response instanceof Response,
       "Expected a Response to be returned from queryRoute"

From 3829f5099b8a03aebdc0910afdcceaa0002f31a8 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 2 Nov 2022 11:01:13 -0700
Subject: [PATCH 25/30] updated to cover aborted request case

---
 packages/remix-server-runtime/server.ts | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index c812a35b870..409f15e1e58 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -71,7 +71,9 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
           handleDataRequestRR(serverMode, staticHandler!, routeId, request),
         ]);
 
-        assertResponsesMatch(response, remixRouterResponse);
+        if (!request.signal.aborted) {
+          assertResponsesMatch(response, remixRouterResponse);
+        }
 
         console.log("Returning Remix Router Data Request Response");
         responsePromise = Promise.resolve(remixRouterResponse);
@@ -114,7 +116,9 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
           ),
         ]);
 
-        assertResponsesMatch(response, remixRouterResponse);
+        if (!request.signal.aborted) {
+          assertResponsesMatch(response, remixRouterResponse);
+        }
 
         console.log("Returning Remix Router Resource Request Response");
         responsePromise = Promise.resolve(remixRouterResponse);
@@ -142,7 +146,9 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
           handleDocumentRequestRR(serverMode, build, staticHandler!, request),
         ]);
 
-        assertResponsesMatch(response, remixRouterResponse);
+        if (!request.signal.aborted) {
+          assertResponsesMatch(response, remixRouterResponse);
+        }
 
         console.log("Returning Remix Router Document Request Response");
         responsePromise = Promise.resolve(remixRouterResponse);
@@ -322,7 +328,7 @@ async function handleDataRequestRR(
       errorInstance = error.error || errorInstance;
     }
 
-    if (serverMode !== ServerMode.Test) {
+    if (serverMode !== ServerMode.Test && !request.signal.aborted) {
       console.error(errorInstance);
     }
 
@@ -392,7 +398,16 @@ async function handleDocumentRequestRR(
   staticHandler: StaticHandler,
   request: Request
 ) {
-  let context = await staticHandler.query(request);
+  let context;
+  try {
+    context = await staticHandler.query(request);
+  } catch (error) {
+    if (!request.signal.aborted && serverMode !== ServerMode.Test) {
+      console.error(error);
+    }
+
+    return new Response(null, { status: 500 });
+  }
 
   if (context instanceof Response) {
     return context;

From b944b7cea7e0722a24a85fabd4d97a2d70453305 Mon Sep 17 00:00:00 2001
From: Jacob Ebey <jacob.ebey@live.com>
Date: Wed, 9 Nov 2022 12:55:47 -0800
Subject: [PATCH 26/30] chore: remove assert flow (#4554)

chore: update tests to handle no assert flow
---
 integration/error-boundary-test.ts            |  11 +-
 integration/error-data-request-test.ts        |   5 -
 integration/transition-test.ts                |   4 +-
 .../__tests__/server-test.ts                  |   3 +-
 packages/remix-server-runtime/server.ts       | 348 ++++++++----------
 5 files changed, 161 insertions(+), 210 deletions(-)

diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts
index 04b9bea72bc..d90ec588280 100644
--- a/integration/error-boundary-test.ts
+++ b/integration/error-boundary-test.ts
@@ -112,7 +112,7 @@ test.describe("ErrorBoundary", () => {
             throw new Error("Kaboom!")
           }
           export function ErrorBoundary() {
-            return <p>${OWN_BOUNDARY_TEXT}</p>
+            return <p id="own-boundary">${OWN_BOUNDARY_TEXT}</p>
           }
           export default function () {
             return (
@@ -146,7 +146,7 @@ test.describe("ErrorBoundary", () => {
             throw new Error("Kaboom!")
           }
           export function ErrorBoundary() {
-            return <div>${OWN_BOUNDARY_TEXT}</div>
+            return <div id="own-boundary">${OWN_BOUNDARY_TEXT}</div>
           }
           export default function () {
             return <div/>
@@ -176,7 +176,7 @@ test.describe("ErrorBoundary", () => {
           }
 
           export function ErrorBoundary() {
-            return <div>${OWN_BOUNDARY_TEXT}</div>
+            return <div id="own-boundary">${OWN_BOUNDARY_TEXT}</div>
           }
         `,
 
@@ -305,6 +305,7 @@ test.describe("ErrorBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickSubmitButton(HAS_BOUNDARY_ACTION);
+    await page.waitForSelector("#own-boundary");
     expect(await app.getHtml("main")).toMatch(OWN_BOUNDARY_TEXT);
   });
 
@@ -314,6 +315,7 @@ test.describe("ErrorBoundary", () => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto(HAS_BOUNDARY_ACTION);
     await app.clickSubmitButton(HAS_BOUNDARY_ACTION);
+    await page.waitForSelector("#own-boundary");
     expect(await app.getHtml("main")).toMatch(OWN_BOUNDARY_TEXT);
   });
 
@@ -589,6 +591,7 @@ test.describe("ErrorBoundary", () => {
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await app.clickSubmitButton(NO_ROOT_BOUNDARY_ACTION);
+      await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`)
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
 
@@ -604,6 +607,7 @@ test.describe("ErrorBoundary", () => {
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await app.clickLink(NO_ROOT_BOUNDARY_LOADER_RETURN);
+      await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`)
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
 
@@ -621,6 +625,7 @@ test.describe("ErrorBoundary", () => {
       let app = new PlaywrightFixture(appFixture, page);
       await app.goto("/");
       await app.clickSubmitButton(NO_ROOT_BOUNDARY_ACTION_RETURN);
+      await page.waitForSelector(`text=${INTERNAL_ERROR_BOUNDARY_HEADING}`)
       expect(await app.getHtml("h1")).toMatch(INTERNAL_ERROR_BOUNDARY_HEADING);
     });
   });
diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts
index dc0a7d9f1f7..456eee1ecd4 100644
--- a/integration/error-data-request-test.ts
+++ b/integration/error-data-request-test.ts
@@ -3,8 +3,6 @@ import { test, expect } from "@playwright/test";
 import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
 import type { Fixture, AppFixture } from "./helpers/create-fixture";
 
-const ENABLE_REMIX_ROUTER = !!process.env.ENABLE_REMIX_ROUTER;
-
 test.describe("ErrorBoundary", () => {
   let fixture: Fixture;
   let appFixture: AppFixture;
@@ -105,9 +103,6 @@ test.describe("ErrorBoundary", () => {
 
   function assertConsoleError(str: string) {
     expect(errorLogs[0]).toEqual(str);
-    if (ENABLE_REMIX_ROUTER) {
-      expect(errorLogs[1]).toEqual(str);
-    }
   }
 
   test("returns a 405 x-remix-error on a data fetch to a path with no loader", async () => {
diff --git a/integration/transition-test.ts b/integration/transition-test.ts
index 63bd78d2eb9..6cf713b1c9e 100644
--- a/integration/transition-test.ts
+++ b/integration/transition-test.ts
@@ -216,8 +216,8 @@ test.describe("rendering", () => {
     await page.waitForLoadState("networkidle");
 
     expect(
-      responses.map((res) => new URL(res.url()).searchParams.get("_data"))
-    ).toEqual([`routes/${PAGE}`, `routes/${PAGE}/index`]);
+      responses.map((res) => new URL(res.url()).searchParams.get("_data")).sort()
+    ).toEqual([`routes/${PAGE}`, `routes/${PAGE}/index`].sort());
 
     await page.waitForSelector(`h2:has-text("${PAGE_TEXT}")`);
     await page.waitForSelector(`h3:has-text("${PAGE_INDEX_TEXT}")`);
diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index 75024ca1620..ae7c4d2ce51 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -3,8 +3,7 @@ import { ServerMode } from "../mode";
 import type { ServerBuild } from "../build";
 import { mockServerBuild } from "./utils";
 
-const ENABLE_REMIX_ROUTER = !!process.env.ENABLE_REMIX_ROUTER;
-const DATA_CALL_MULTIPIER = ENABLE_REMIX_ROUTER ? 2 : 1;
+const DATA_CALL_MULTIPIER = 1;
 
 function spyConsole() {
   // https://github.com/facebook/react/issues/7047
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index 409f15e1e58..790d8289099 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -52,35 +52,19 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
 
     let response: Response;
     if (url.searchParams.has("_data")) {
-      let responsePromise = handleDataRequest({
-        request:
-          // We need to clone the request here instead of the call to the new
-          // handler otherwise the first handler will lock the body for the other.
-          // Cloning here allows the new handler to be the stream reader and delegate
-          // chunks back to this cloned request.
-          ENABLE_REMIX_ROUTER ? request.clone() : request,
-        loadContext,
-        matches: matches!,
-        serverMode,
-      });
-
       let routeId = url.searchParams.get("_data")!;
-      if (ENABLE_REMIX_ROUTER) {
-        let [response, remixRouterResponse] = await Promise.all([
-          responsePromise,
-          handleDataRequestRR(serverMode, staticHandler!, routeId, request),
-        ]);
 
-        if (!request.signal.aborted) {
-          assertResponsesMatch(response, remixRouterResponse);
-        }
-
-        console.log("Returning Remix Router Data Request Response");
-        responsePromise = Promise.resolve(remixRouterResponse);
+      if (ENABLE_REMIX_ROUTER) {
+        response = await handleDataRequestRR(serverMode, staticHandler!, routeId, request);
+      } else {
+        response = await handleDataRequest({
+          request,
+          loadContext,
+          matches: matches!,
+          serverMode,
+        });
       }
 
-      response = await responsePromise;
-
       if (build.entry.module.handleDataRequest) {
         let match = matches!.find((match) => match.route.id == routeId)!;
         response = await build.entry.module.handleDataRequest(response, {
@@ -93,68 +77,34 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
       matches &&
       matches[matches.length - 1].route.module.default == null
     ) {
-      let responsePromise = handleResourceRequest({
-        request:
-          // We need to clone the request here instead of the call to the new
-          // handler otherwise the first handler will lock the body for the other.
-          // Cloning here allows the new handler to be the stream reader and delegate
-          // chunks back to this cloned request.
-          ENABLE_REMIX_ROUTER ? request.clone() : request,
-        loadContext,
-        matches,
-        serverMode,
-      });
-
       if (ENABLE_REMIX_ROUTER) {
-        let [response, remixRouterResponse] = await Promise.all([
-          responsePromise,
-          handleResourceRequestRR(
-            serverMode,
-            staticHandler!,
-            matches.slice(-1)[0].route.id,
-            request
-          ),
-        ]);
-
-        if (!request.signal.aborted) {
-          assertResponsesMatch(response, remixRouterResponse);
-        }
-
-        console.log("Returning Remix Router Resource Request Response");
-        responsePromise = Promise.resolve(remixRouterResponse);
+        response = await handleResourceRequestRR(
+          serverMode,
+          staticHandler!,
+          matches.slice(-1)[0].route.id,
+          request
+        );
+      } else {
+        response = await handleResourceRequest({
+          request,
+          loadContext,
+          matches,
+          serverMode,
+        });
       }
-
-      response = await responsePromise;
     } else {
-      let responsePromise = handleDocumentRequest({
-        build,
-        loadContext,
-        matches,
-        request:
-          // We need to clone the request here instead of the call to the new
-          // handler otherwise the first handler will lock the body for the other.
-          // Cloning here allows the new handler to be the stream reader and delegate
-          // chunks back to this cloned request.
-          ENABLE_REMIX_ROUTER ? request.clone() : request,
-        routes,
-        serverMode,
-      });
-
       if (ENABLE_REMIX_ROUTER) {
-        let [response, remixRouterResponse] = await Promise.all([
-          responsePromise,
-          handleDocumentRequestRR(serverMode, build, staticHandler!, request),
-        ]);
-
-        if (!request.signal.aborted) {
-          assertResponsesMatch(response, remixRouterResponse);
-        }
-
-        console.log("Returning Remix Router Document Request Response");
-        responsePromise = Promise.resolve(remixRouterResponse);
+        response = await handleDocumentRequestRR(serverMode, build, staticHandler!, request)
+      } else {
+        response = await handleDocumentRequest({
+          build,
+          loadContext,
+          matches,
+          request,
+          routes,
+          serverMode,
+        })
       }
-
-      response = await responsePromise;
     }
 
     if (request.method === "HEAD") {
@@ -1065,122 +1015,6 @@ function getRenderableMatches(
   return matches.slice(0, lastRenderableIndex + 1);
 }
 
-async function assert(
-  a: Response,
-  b: Response,
-  accessor: (r: Response) => object | Promise<object>,
-  message: string
-) {
-  let aStr = JSON.stringify(await accessor(a));
-  let bStr = JSON.stringify(await accessor(b));
-
-  if (aStr !== bStr) {
-    console.error(message);
-    message += "\nResponse 1:\n" + aStr + "\nResponse 2:\n" + bStr;
-    throw new Error(message);
-  }
-}
-
-async function assertResponsesMatch(_a: Response, _b: Response) {
-  let a = _a.clone();
-  let b = _b.clone();
-  assert(
-    a,
-    b,
-    (r) => Object.fromEntries(r.headers.entries()),
-    "Headers did not match!"
-  );
-
-  if (a.headers.get("Content-Type")?.startsWith("application/json")) {
-    if (a.headers.get("X-Remix-Error")) {
-      assert(
-        a,
-        b,
-        async (r) => {
-          let { stack, ...json } = await r.json();
-          return {
-            ...json,
-            stack: stack ? "yes" : "no",
-          };
-        },
-        "JSON error response body did not match!\n Response 1:\n" +
-          (await a.clone().text()) +
-          "\nResponse 2:\n" +
-          (await b.clone().text())
-      );
-    } else {
-      assert(
-        a,
-        b,
-        (r) => r.json(),
-        "JSON response body did not match!\nResponse 1:\n" +
-          (await a.clone().text()) +
-          "\nResponse 2:\n" +
-          (await b.clone().text())
-      );
-    }
-  } else {
-    assert(
-      a,
-      b,
-      async (r) => {
-        let text = await r.text();
-
-        // Strip stack trace from default error boundary HTML
-        if (
-          text.includes(">Application Error</h1>") &&
-          text.includes("💿 Hey developer👋")
-        ) {
-          return stripStackTraceFromDefaultErrorBoundary(text);
-        }
-
-        if (text.includes("<script>window.__remixContext")) {
-          return stripStackTraceFromRemixContext(text);
-        }
-
-        return text;
-      },
-      "Non-JSON response body did not match!\nResponse 1:\n" +
-        (await a.clone().text()) +
-        "\nResponse 2:\n" +
-        (await b.clone().text())
-    );
-  }
-}
-
-function stripStackTraceFromDefaultErrorBoundary(text: string) {
-  let openPreStart = text.indexOf("<pre ");
-  let openPreEnd = text.indexOf('">', openPreStart + 1);
-  let closePre = text.indexOf("</pre>", openPreEnd + 1);
-  let error = text.substring(openPreEnd + 2, closePre);
-  let stackStart = error.indexOf("\n");
-  let errorWithoutStack = error.substring(0, stackStart);
-  return (
-    text.replace(error, "ERROR REMOVED") + "\n\nError:\n" + errorWithoutStack
-  );
-}
-
-function stripStackTraceFromRemixContext(text: string) {
-  let scriptStart = text.indexOf("<script>window.__remixContext");
-  let scriptEnd = text.indexOf("</script>", scriptStart + 1);
-  let scriptContents = text.substring(scriptStart, scriptEnd + 9);
-  let remixContext = JSON.parse(
-    scriptContents
-      .replace("<script>window.__remixContext = ", "")
-      .replace(";</script>", "")
-  );
-  // If we didn't have an error - assert the full document
-  if (!remixContext?.appState?.error?.stack) {
-    return text;
-  }
-
-  // Otherwise remove the stack trace and assert the text/JSON as a
-  // concatenated string
-  remixContext.appState.error.stack = "yes";
-  text = text.replace(scriptContents, "CONTEXT REMOVED");
-  return text + "\n\nContext:\n" + JSON.stringify(remixContext);
-}
-
 function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) {
   if (serverMode !== ServerMode.Test) {
     console.error(error);
@@ -1200,3 +1034,121 @@ function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) {
     },
   });
 }
+
+
+// TODO: Remove before we "finalize" router migration
+// async function assert(
+//   a: Response,
+//   b: Response,
+//   accessor: (r: Response) => object | Promise<object>,
+//   message: string
+// ) {
+//   let aStr = JSON.stringify(await accessor(a));
+//   let bStr = JSON.stringify(await accessor(b));
+
+//   if (aStr !== bStr) {
+//     console.error(message);
+//     message += "\nResponse 1:\n" + aStr + "\nResponse 2:\n" + bStr;
+//     throw new Error(message);
+//   }
+// }
+
+// async function assertResponsesMatch(_a: Response, _b: Response) {
+//   let a = _a.clone();
+//   let b = _b.clone();
+//   assert(
+//     a,
+//     b,
+//     (r) => Object.fromEntries(r.headers.entries()),
+//     "Headers did not match!"
+//   );
+
+//   if (a.headers.get("Content-Type")?.startsWith("application/json")) {
+//     if (a.headers.get("X-Remix-Error")) {
+//       assert(
+//         a,
+//         b,
+//         async (r) => {
+//           let { stack, ...json } = await r.json();
+//           return {
+//             ...json,
+//             stack: stack ? "yes" : "no",
+//           };
+//         },
+//         "JSON error response body did not match!\n Response 1:\n" +
+//           (await a.clone().text()) +
+//           "\nResponse 2:\n" +
+//           (await b.clone().text())
+//       );
+//     } else {
+//       assert(
+//         a,
+//         b,
+//         (r) => r.json(),
+//         "JSON response body did not match!\nResponse 1:\n" +
+//           (await a.clone().text()) +
+//           "\nResponse 2:\n" +
+//           (await b.clone().text())
+//       );
+//     }
+//   } else {
+//     assert(
+//       a,
+//       b,
+//       async (r) => {
+//         let text = await r.text();
+
+//         // Strip stack trace from default error boundary HTML
+//         if (
+//           text.includes(">Application Error</h1>") &&
+//           text.includes("💿 Hey developer👋")
+//         ) {
+//           return stripStackTraceFromDefaultErrorBoundary(text);
+//         }
+
+//         if (text.includes("<script>window.__remixContext")) {
+//           return stripStackTraceFromRemixContext(text);
+//         }
+
+//         return text;
+//       },
+//       "Non-JSON response body did not match!\nResponse 1:\n" +
+//         (await a.clone().text()) +
+//         "\nResponse 2:\n" +
+//         (await b.clone().text())
+//     );
+//   }
+// }
+
+// function stripStackTraceFromDefaultErrorBoundary(text: string) {
+//   let openPreStart = text.indexOf("<pre ");
+//   let openPreEnd = text.indexOf('">', openPreStart + 1);
+//   let closePre = text.indexOf("</pre>", openPreEnd + 1);
+//   let error = text.substring(openPreEnd + 2, closePre);
+//   let stackStart = error.indexOf("\n");
+//   let errorWithoutStack = error.substring(0, stackStart);
+//   return (
+//     text.replace(error, "ERROR REMOVED") + "\n\nError:\n" + errorWithoutStack
+//   );
+// }
+
+// function stripStackTraceFromRemixContext(text: string) {
+//   let scriptStart = text.indexOf("<script>window.__remixContext");
+//   let scriptEnd = text.indexOf("</script>", scriptStart + 1);
+//   let scriptContents = text.substring(scriptStart, scriptEnd + 9);
+//   let remixContext = JSON.parse(
+//     scriptContents
+//       .replace("<script>window.__remixContext = ", "")
+//       .replace(";</script>", "")
+//   );
+//   // If we didn't have an error - assert the full document
+//   if (!remixContext?.appState?.error?.stack) {
+//     return text;
+//   }
+
+//   // Otherwise remove the stack trace and assert the text/JSON as a
+//   // concatenated string
+//   remixContext.appState.error.stack = "yes";
+//   text = text.replace(scriptContents, "CONTEXT REMOVED");
+//   return text + "\n\nContext:\n" + JSON.stringify(remixContext);
+// }

From 5db567ebb93accf83fe26c3f1955b2c31daa9e33 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 15 Nov 2022 16:38:25 -0500
Subject: [PATCH 27/30] Change missing loader from 405 -> 400 error

---
 integration/error-data-request-test.ts        |  4 +--
 .../__tests__/server-test.ts                  |  2 +-
 packages/remix-server-runtime/data.ts         |  2 +-
 .../remix-server-runtime/router/router.ts     | 26 +++++++++----------
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/integration/error-data-request-test.ts b/integration/error-data-request-test.ts
index 456eee1ecd4..537e282f656 100644
--- a/integration/error-data-request-test.ts
+++ b/integration/error-data-request-test.ts
@@ -105,9 +105,9 @@ test.describe("ErrorBoundary", () => {
     expect(errorLogs[0]).toEqual(str);
   }
 
-  test("returns a 405 x-remix-error on a data fetch to a path with no loader", async () => {
+  test("returns a 400 x-remix-error on a data fetch to a path with no loader", async () => {
     let response = await fixture.requestData("/", "routes/index");
-    expect(response.status).toBe(405);
+    expect(response.status).toBe(400);
     expect(response.headers.get("X-Remix-Error")).toBe("yes");
     expect(await response.text()).toMatch("Unexpected Server Error");
     assertConsoleError(
diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
index ae7c4d2ce51..da5d4cc7eb1 100644
--- a/packages/remix-server-runtime/__tests__/server-test.ts
+++ b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -409,7 +409,7 @@ describe("shared server runtime", () => {
       });
 
       let result = await handler(request);
-      expect(result.status).toBe(405);
+      expect(result.status).toBe(400);
       expect(result.headers.get("X-Remix-Error")).toBe("yes");
       expect((await result.json()).message).toBeTruthy();
     });
diff --git a/packages/remix-server-runtime/data.ts b/packages/remix-server-runtime/data.ts
index 5f62c0ec355..d5daab04aa3 100644
--- a/packages/remix-server-runtime/data.ts
+++ b/packages/remix-server-runtime/data.ts
@@ -86,7 +86,7 @@ export async function callRouteLoader({
     let msg =
       `You made a ${request.method} request to "${pathname}" but did not provide ` +
       `a \`loader\` for route "${routeId}", so there is no way to handle the request.`;
-    throw new ErrorResponse(405, "Method Not Allowed", new Error(msg), true);
+    throw new ErrorResponse(400, "Bad Request", new Error(msg), true);
   }
 
   let result;
diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index befb71e0b1e..7d4a4e3d238 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -2163,7 +2163,7 @@ export function unstable_createStaticHandler(
 
     // Short circuit if we have no loaders to run (queryRoute())
     if (isRouteRequest && !routeMatch?.route.loader) {
-      throw getInternalRouterError(405, {
+      throw getInternalRouterError(400, {
         method: request.method,
         pathname: createURL(request.url).pathname,
         routeId: routeMatch?.route.id,
@@ -2899,7 +2899,14 @@ function getInternalRouterError(
 
   if (status === 400) {
     statusText = "Bad Request";
-    errorMessage = "Cannot submit binary form data using GET";
+    if (method && pathname && routeId) {
+      errorMessage =
+        `You made a ${method} request to "${pathname}" but ` +
+        `did not provide a \`loader\` for route "${routeId}", ` +
+        `so there is no way to handle the request.`;
+    } else {
+      errorMessage = "Cannot submit binary form data using GET";
+    }
   } else if (status === 403) {
     statusText = "Forbidden";
     errorMessage = `Route "${routeId}" does not match URL "${pathname}"`;
@@ -2909,17 +2916,10 @@ function getInternalRouterError(
   } else if (status === 405) {
     statusText = "Method Not Allowed";
     if (method && pathname && routeId) {
-      if (validActionMethods.has(method)) {
-        errorMessage =
-          `You made a ${method} request to "${pathname}" but ` +
-          `did not provide an \`action\` for route "${routeId}", ` +
-          `so there is no way to handle the request.`;
-      } else {
-        errorMessage =
-          `You made a ${method} request to "${pathname}" but ` +
-          `did not provide a \`loader\` for route "${routeId}", ` +
-          `so there is no way to handle the request.`;
-      }
+      errorMessage =
+        `You made a ${method} request to "${pathname}" but ` +
+        `did not provide an \`action\` for route "${routeId}", ` +
+        `so there is no way to handle the request.`;
     } else {
       errorMessage = `Invalid request method "${method}"`;
     }

From 8a14ade17f3f3372ad26db47a492404e7b599fe9 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 15 Nov 2022 16:38:36 -0500
Subject: [PATCH 28/30] Bring over external redirect logic from react router

---
 .../remix-server-runtime/router/router.ts     | 52 ++++++++++++-------
 packages/remix-server-runtime/router/utils.ts |  1 +
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/packages/remix-server-runtime/router/router.ts b/packages/remix-server-runtime/router/router.ts
index 7d4a4e3d238..b379bf0e04e 100644
--- a/packages/remix-server-runtime/router/router.ts
+++ b/packages/remix-server-runtime/router/router.ts
@@ -1584,6 +1584,16 @@ export function createRouter(init: RouterInit): Router {
       navigation.location,
       "Expected a location on the redirect navigation"
     );
+
+    if (
+      redirect.external &&
+      typeof window !== "undefined" &&
+      typeof window.location !== "undefined"
+    ) {
+      window.location.replace(redirect.location);
+      return;
+    }
+
     // There's no need to abort on redirects, since we don't detect the
     // redirect until the action/loaders have settled
     pendingNavigationController = null;
@@ -2556,26 +2566,31 @@ async function callLoaderOrAction(
         "Redirects returned/thrown from loaders/actions must have a Location header"
       );
 
-      // Support relative routing in redirects
-      let activeMatches = matches.slice(0, matches.indexOf(match) + 1);
-      let routePathnames = getPathContributingMatches(activeMatches).map(
-        (match) => match.pathnameBase
-      );
-      let requestPath = createURL(request.url).pathname;
-      let resolvedLocation = resolveTo(location, routePathnames, requestPath);
-      invariant(
-        createPath(resolvedLocation),
-        `Unable to resolve redirect location: ${result.headers.get("Location")}`
-      );
+      // Check if this an external redirect that goes to a new origin
+      let external = createURL(location).origin !== createURL("/").origin;
 
-      // Prepend the basename to the redirect location if we have one
-      if (basename) {
-        let path = resolvedLocation.pathname;
-        resolvedLocation.pathname =
-          path === "/" ? basename : joinPaths([basename, path]);
-      }
+      // Support relative routing in internal redirects
+      if (!external) {
+        let activeMatches = matches.slice(0, matches.indexOf(match) + 1);
+        let routePathnames = getPathContributingMatches(activeMatches).map(
+          (match) => match.pathnameBase
+        );
+        let requestPath = createURL(request.url).pathname;
+        let resolvedLocation = resolveTo(location, routePathnames, requestPath);
+        invariant(
+          createPath(resolvedLocation),
+          `Unable to resolve redirect location: ${location}`
+        );
 
-      location = createPath(resolvedLocation);
+        // Prepend the basename to the redirect location if we have one
+        if (basename) {
+          let path = resolvedLocation.pathname;
+          resolvedLocation.pathname =
+            path === "/" ? basename : joinPaths([basename, path]);
+        }
+
+        location = createPath(resolvedLocation);
+      }
 
       // Don't process redirects in the router during static requests requests.
       // Instead, throw the Response and let the server handle it with an HTTP
@@ -2591,6 +2606,7 @@ async function callLoaderOrAction(
         status,
         location,
         revalidate: result.headers.get("X-Remix-Revalidate") !== null,
+        external,
       };
     }
 
diff --git a/packages/remix-server-runtime/router/utils.ts b/packages/remix-server-runtime/router/utils.ts
index fb633293f3e..a4f47751330 100644
--- a/packages/remix-server-runtime/router/utils.ts
+++ b/packages/remix-server-runtime/router/utils.ts
@@ -43,6 +43,7 @@ export interface RedirectResult {
   status: number;
   location: string;
   revalidate: boolean;
+  external: boolean;
 }
 
 /**

From 3ce0ac3ba04ba962e7783bda863c69b8f49b6a46 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 15 Nov 2022 16:48:05 -0500
Subject: [PATCH 29/30] add changeset

---
 .changeset/light-avocados-drop.md | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 .changeset/light-avocados-drop.md

diff --git a/.changeset/light-avocados-drop.md b/.changeset/light-avocados-drop.md
new file mode 100644
index 00000000000..8be5277aa0b
--- /dev/null
+++ b/.changeset/light-avocados-drop.md
@@ -0,0 +1,26 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+fix: Properly categorize internal framework-thrown error Responses as error boundary errors
+
+Previously there was some ambiguity around _"thrown Responses go to the `CatchBoundary`"_.
+The `CatchBoundary` exists to give the _user_ a place to handle non-happy path code flows
+such that they can throw Response instances from _their own code_ and handle them in a
+`CatchBoundary`.  However, there are a handful of framework-internal errors that make
+sense to have a non-500 status code, and the fact that these were being thrown as Responses
+was causing them to go into the CatchBoundary, even though they were not user-thrown.
+
+With this change, anything thrown by the framework itself (`Error` or `Response`) will
+go to the `ErrorBoundary`, and any user-thrown `Response` instances will go to the
+`CatchBoundary`.  Thereis  one exception to this rule, which is that framework-detected
+404's will continue to go to the `CatchBoundary` since users should have one single
+location to handle 404 displays.
+
+The primary affected use cases are scenarios such as:
+
+* HTTP `OPTIONS` requests (405 Unsupported Method )
+* `GET` requests to routes without loaders (400 Bad Request)
+* `POST` requests to routes without actions (405 Method Not Allowed)
+* Missing route id in `_data` parameters (403 Forbidden)
+* Non-matching route id included in `_data` parameters (403 Forbidden)

From ccae87fbcd74dd68774c1a4952ce37ca32d64454 Mon Sep 17 00:00:00 2001
From: Matt Brophy <matt@brophy.org>
Date: Tue, 15 Nov 2022 16:57:10 -0500
Subject: [PATCH 30/30] fix tsc error

---
 packages/remix-server-runtime/server.ts | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
index eaedbfdba2b..609631b649e 100644
--- a/packages/remix-server-runtime/server.ts
+++ b/packages/remix-server-runtime/server.ts
@@ -55,7 +55,12 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
       let routeId = url.searchParams.get("_data")!;
 
       if (ENABLE_REMIX_ROUTER) {
-        response = await handleDataRequestRR(serverMode, staticHandler!, routeId, request);
+        response = await handleDataRequestRR(
+          serverMode,
+          staticHandler!,
+          routeId,
+          request
+        );
       } else {
         response = await handleDataRequest({
           request,
@@ -94,7 +99,12 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
       }
     } else {
       if (ENABLE_REMIX_ROUTER) {
-        response = await handleDocumentRequestRR(serverMode, build, staticHandler!, request)
+        response = await handleDocumentRequestRR(
+          serverMode,
+          build,
+          staticHandler!,
+          request
+        );
       } else {
         response = await handleDocumentRequest({
           build,
@@ -103,7 +113,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
           request,
           routes,
           serverMode,
-        })
+        });
       }
     }
 
@@ -462,12 +472,13 @@ async function handleDocumentRequestRR(
 
   let serverHandoff: Pick<
     EntryContext,
-    "actionData" | "appState" | "matches" | "routeData"
+    "actionData" | "appState" | "matches" | "routeData" | "future"
   > = {
     actionData: context.actionData || undefined,
     appState,
     matches: createEntryMatches(renderableMatches, build.assets.routes),
     routeData: context.loaderData || {},
+    future: build.future,
   };
 
   let entryContext: EntryContext = {
@@ -1036,7 +1047,6 @@ function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) {
   });
 }
 
-
 // TODO: Remove before we "finalize" router migration
 // async function assert(
 //   a: Response,