From a83d1672e7a4bf2fb83b7cd13af168cdb3c67904 Mon Sep 17 00:00:00 2001
From: Onur Temizkan <onur@narval.co.uk>
Date: Tue, 19 Dec 2023 10:42:35 +0000
Subject: [PATCH] fix(remix): Do not capture thrown redirect responses.

---
 packages/remix/src/utils/instrumentServer.ts         |  9 ++++-----
 .../integration/app_v1/routes/throw-redirect.tsx     |  2 ++
 .../integration/app_v2/routes/throw-redirect.tsx     |  2 ++
 .../integration/common/routes/throw-redirect.tsx     | 11 +++++++++++
 .../integration/test/client/throw-redirect.test.ts   |  8 ++++++++
 .../test/integration/test/server/loader.test.ts      | 12 ++++++++++++
 6 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 packages/remix/test/integration/app_v1/routes/throw-redirect.tsx
 create mode 100644 packages/remix/test/integration/app_v2/routes/throw-redirect.tsx
 create mode 100644 packages/remix/test/integration/common/routes/throw-redirect.tsx
 create mode 100644 packages/remix/test/integration/test/client/throw-redirect.test.ts

diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts
index 816410fd75f9..ea87b961493c 100644
--- a/packages/remix/src/utils/instrumentServer.ts
+++ b/packages/remix/src/utils/instrumentServer.ts
@@ -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) {
diff --git a/packages/remix/test/integration/app_v1/routes/throw-redirect.tsx b/packages/remix/test/integration/app_v1/routes/throw-redirect.tsx
new file mode 100644
index 000000000000..4425f3432b58
--- /dev/null
+++ b/packages/remix/test/integration/app_v1/routes/throw-redirect.tsx
@@ -0,0 +1,2 @@
+export * from '../../common/routes/throw-redirect';
+export { default } from '../../common/routes/throw-redirect';
diff --git a/packages/remix/test/integration/app_v2/routes/throw-redirect.tsx b/packages/remix/test/integration/app_v2/routes/throw-redirect.tsx
new file mode 100644
index 000000000000..4425f3432b58
--- /dev/null
+++ b/packages/remix/test/integration/app_v2/routes/throw-redirect.tsx
@@ -0,0 +1,2 @@
+export * from '../../common/routes/throw-redirect';
+export { default } from '../../common/routes/throw-redirect';
diff --git a/packages/remix/test/integration/common/routes/throw-redirect.tsx b/packages/remix/test/integration/common/routes/throw-redirect.tsx
new file mode 100644
index 000000000000..2d530e41d0c0
--- /dev/null
+++ b/packages/remix/test/integration/common/routes/throw-redirect.tsx
@@ -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>;
+}
diff --git a/packages/remix/test/integration/test/client/throw-redirect.test.ts b/packages/remix/test/integration/test/client/throw-redirect.test.ts
new file mode 100644
index 000000000000..60ed3588c79b
--- /dev/null
+++ b/packages/remix/test/integration/test/client/throw-redirect.test.ts
@@ -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);
+});
diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts
index b6d4fc402ea7..24d67422c3ca 100644
--- a/packages/remix/test/integration/test/server/loader.test.ts
+++ b/packages/remix/test/integration/test/server/loader.test.ts
@@ -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);
+  });
 });