Skip to content

Commit

Permalink
fix(remix): Do not capture thrown redirect responses.
Browse files Browse the repository at this point in the history
  • Loading branch information
onurtemizkan committed Dec 19, 2023
1 parent 6d32228 commit a83d167
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 5 deletions.
9 changes: 4 additions & 5 deletions packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,13 @@ export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs
export async function captureRemixServerException(err: unknown, name: string, request: Request): Promise<void> {
// Skip capturing if the thrown error is not a 5xx response
// https://remix.run/docs/en/v1/api/conventions#throwing-responses-in-loaders
if (IS_REMIX_V2) {
if (isRouteErrorResponse(err) && err.status < 500) {
return;
}
} else if (isResponse(err) && err.status < 500) {
if (IS_REMIX_V2 && isRouteErrorResponse(err) && err.status < 500) {
return;
}

if (isResponse(err) && err.status < 500) {
return;
}
// Skip capturing if the request is aborted as Remix docs suggest
// Ref: https://remix.run/docs/en/main/file-conventions/entry.server#handleerror
if (request.signal.aborted) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from '../../common/routes/throw-redirect';
export { default } from '../../common/routes/throw-redirect';
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from '../../common/routes/throw-redirect';
export { default } from '../../common/routes/throw-redirect';
11 changes: 11 additions & 0 deletions packages/remix/test/integration/common/routes/throw-redirect.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { LoaderFunction, redirect } from '@remix-run/node';
import { useLoaderData } from '@remix-run/react';

export const loader: LoaderFunction = async () => {
throw redirect('/');
};

export default function ThrowRedirect() {
const data = useLoaderData();
return <div>{data}</div>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { expect, test } from '@playwright/test';
import { countEnvelopes } from './utils/helpers';

test('should not report thrown redirect response on client side.', async ({ page }) => {
const count = await countEnvelopes(page, { url: '/throw-redirect', envelopeType: 'event' });

expect(count).toBe(0);
});
12 changes: 12 additions & 0 deletions packages/remix/test/integration/test/server/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,4 +241,16 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
],
});
});

it('does not capture thrown redirect responses', async () => {
const env = await RemixTestEnv.init(adapter);
const url = `${env.url}/throw-redirect`;

const envelopesCount = await env.countEnvelopes({
url,
envelopeType: ['event'],
});

expect(envelopesCount).toBe(0);
});
});

0 comments on commit a83d167

Please sign in to comment.