Skip to content

Commit

Permalink
Dont log single fetch thrown redirect response stubs via handleError (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed May 6, 2024
1 parent bd6a306 commit 3d8eeaf
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 13 deletions.
5 changes: 5 additions & 0 deletions .changeset/rare-dodos-push.md
@@ -0,0 +1,5 @@
---
"@remix-run/server-runtime": patch
---

Don't log thrown response stubs via `handleError` in Single Fetch
75 changes: 75 additions & 0 deletions integration/single-fetch-test.ts
Expand Up @@ -1667,6 +1667,81 @@ test.describe("single-fetch", () => {
);
});

test("does not log thrown redirect response stubs via handleError", async () => {
let fixture = await createFixture({
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/redirect.tsx": js`
export function action({ response }) {
response.status = 301;
response.headers.set("Location", "/data");
throw response;
}
export function loader({ response }) {
response.status = 301;
response.headers.set("Location", "/data");
throw response;
}
export default function Component() {
return <h1>Should not see me</h1>;
}
`,
},
});

let errorLogs = [];
console.error = (e) => errorLogs.push(e);
await fixture.requestDocument("/redirect");
await fixture.requestSingleFetchData("/redirect.data");
await fixture.requestSingleFetchData("/redirect.data", {
method: "post",
body: null,
});
expect(errorLogs.length).toBe(0);
});

test("does not log thrown non-redirect response stubs via handleError", async () => {
let fixture = await createFixture({
config: {
future: {
unstable_singleFetch: true,
},
},
files: {
...files,
"app/routes/redirect.tsx": js`
export function action({ response }) {
response.status = 400;
throw response;
}
export function loader({ response }) {
response.status = 400;
throw response;
}
export default function Component() {
return <h1>Should not see me</h1>;
}
`,
},
});

let errorLogs = [];
console.error = (e) => errorLogs.push(e);
await fixture.requestDocument("/redirect");
expect(errorLogs.length).toBe(1); // ErrorBoundary render logs this
await fixture.requestSingleFetchData("/redirect.data");
await fixture.requestSingleFetchData("/redirect.data", {
method: "post",
body: null,
});
expect(errorLogs.length).toBe(1);
});

test.describe("client loaders", () => {
test("when no routes have client loaders", async ({ page }) => {
let fixture = await createFixture(
Expand Down
23 changes: 12 additions & 11 deletions packages/remix-server-runtime/server.ts
Expand Up @@ -40,6 +40,7 @@ import {
getSingleFetchDataStrategy,
getSingleFetchRedirect,
getSingleFetchResourceRouteDataStrategy,
isResponseStub,
mergeResponseStubs,
singleFetchAction,
singleFetchLoaders,
Expand Down Expand Up @@ -408,17 +409,6 @@ async function handleDocumentRequest(
return context;
}

// Sanitize errors outside of development environments
if (context.errors) {
Object.values(context.errors).forEach((err) => {
// @ts-expect-error This is "private" from users but intended for internal use
if (!isRouteErrorResponse(err) || err.error) {
handleError(err);
}
});
context.errors = sanitizeErrors(context.errors, serverMode);
}

let statusCode: number;
let headers: Headers;
if (build.future.unstable_singleFetch) {
Expand All @@ -437,6 +427,17 @@ async function handleDocumentRequest(
headers = getDocumentHeaders(build, context);
}

// Sanitize errors outside of development environments
if (context.errors) {
Object.values(context.errors).forEach((err) => {
// @ts-expect-error `err.error` is "private" from users but intended for internal use
if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) {
handleError(err);
}
});
context.errors = sanitizeErrors(context.errors, serverMode);
}

// Server UI state to send to the client.
// - When single fetch is enabled, this is streamed down via `serverHandoffStream`
// - Otherwise it's stringified into `serverHandoffString`
Expand Down
4 changes: 2 additions & 2 deletions packages/remix-server-runtime/single-fetch.ts
Expand Up @@ -184,7 +184,7 @@ export async function singleFetchAction(
if (context.errors) {
Object.values(context.errors).forEach((err) => {
// @ts-expect-error This is "private" from users but intended for internal use
if (!isRouteErrorResponse(err) || err.error) {
if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) {
handleError(err);
}
});
Expand Down Expand Up @@ -273,7 +273,7 @@ export async function singleFetchLoaders(
if (context.errors) {
Object.values(context.errors).forEach((err) => {
// @ts-expect-error This is "private" from users but intended for internal use
if (!isRouteErrorResponse(err) || err.error) {
if ((!isRouteErrorResponse(err) || err.error) && !isResponseStub(err)) {
handleError(err);
}
});
Expand Down

0 comments on commit 3d8eeaf

Please sign in to comment.