fix(remix): Capture thrown fetch responses. (#10166)

Fixes: #10160
Potentially fixes:
#10002

This PR adds support for extracting `header` data from thrown non-Remix
responses (fetch responses) inside loaders / actions.

Author: onurtemizkan

packages/remix/src/utils/instrumentServer.ts

@@ -69,12 +69,12 @@ function isCatchResponse(response: Response): boolean {
 async function extractResponseError(response: Response): Promise<unknown> {
   const responseData = await extractData(response);
 
-  if (typeof responseData === 'string') {
-    return responseData;
+  if (typeof responseData === 'string' && responseData.length > 0) {
+    return new Error(responseData);
   }
 
   if (response.statusText) {
-    return response.statusText;
+    return new Error(response.statusText);
   }
 
   return responseData;
@@ -92,7 +92,7 @@ export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs
   // We are skipping thrown responses here as they are handled by
   // `captureRemixServerException` at loader / action level
   // We don't want to capture them twice.
-  // This function if only for capturing unhandled server-side exceptions.
+  // This function is only for capturing unhandled server-side exceptions.
   // https://remix.run/docs/en/main/file-conventions/entry.server#thrown-responses
   // https://remix.run/docs/en/v1/api/conventions#throwing-responses-in-loaders
   if (isResponse(err) || isRouteErrorResponse(err)) {
@@ -145,7 +145,7 @@ export async function captureRemixServerException(err: unknown, name: string, re
   captureException(isResponse(objectifiedErr) ? await extractResponseError(objectifiedErr) : objectifiedErr, scope => {
     // eslint-disable-next-line deprecation/deprecation
     const transaction = getActiveTransaction();
-    const activeTransactionName = transaction ? spanToJSON(transaction) : undefined;
+    const activeTransactionName = transaction ? spanToJSON(transaction).description : undefined;
 
     scope.setSDKProcessingMetadata({
       request: {

packages/remix/src/utils/web-fetch.ts

@@ -1,3 +1,4 @@
+/* eslint-disable complexity */
 // Based on Remix's implementation of Fetch API
 // https://github.com/remix-run/web-std-io/blob/d2a003fe92096aaf97ab2a618b74875ccaadc280/packages/fetch/
 // The MIT License (MIT)
@@ -70,12 +71,12 @@ export const getSearch = (parsedURL: URL): string => {
 export const normalizeRemixRequest = (request: RemixRequest): Record<string, any> => {
   const { requestInternalsSymbol, bodyInternalsSymbol } = getInternalSymbols(request);
 
-  if (!requestInternalsSymbol) {
-    throw new Error('Could not find request internals symbol');
+  if (!requestInternalsSymbol && !request.headers) {
+    throw new Error('Could not find request headers');
   }
 
-  const { parsedURL } = request[requestInternalsSymbol];
-  const headers = new Headers(request[requestInternalsSymbol].headers);
+  const parsedURL = requestInternalsSymbol ? request[requestInternalsSymbol].parsedURL : new URL(request.url);
+  const headers = requestInternalsSymbol ? new Headers(request[requestInternalsSymbol].headers) : request.headers;
 
   // Fetch step 1.3
   if (!headers.has('Accept')) {
@@ -88,7 +89,7 @@ export const normalizeRemixRequest = (request: RemixRequest): Record<string, any
     contentLengthValue = '0';
   }
 
-  if (request.body !== null) {
+  if (request.body !== null && request[bodyInternalsSymbol]) {
     const totalBytes = request[bodyInternalsSymbol].size;
     // Set Content-Length if totalBytes is a number (that is not NaN)
     if (typeof totalBytes === 'number' && !Number.isNaN(totalBytes)) {

packages/remix/test/integration/app_v1/routes/loader-throw-response/$id.tsx

@@ -0,0 +1,2 @@
+export * from '../../../common/routes/loader-throw-response.$id';
+export { default } from '../../../common/routes/loader-throw-response.$id';

packages/remix/test/integration/app_v2/routes/loader-throw-response.$id.tsx

@@ -0,0 +1,2 @@
+export * from '../../common/routes/loader-throw-response.$id';
+export { default } from '../../common/routes/loader-throw-response.$id';

packages/remix/test/integration/common/routes/loader-throw-response.$id.tsx

@@ -0,0 +1,24 @@
+import { LoaderFunction } from '@remix-run/node';
+import { useLoaderData } from '@remix-run/react';
+
+export const loader: LoaderFunction = async ({ params: { id } }) => {
+  if (id === '-1') {
+    throw new Response(null, {
+      status: 500,
+      statusText: 'Not found',
+    });
+  }
+
+  return { message: 'hello world' };
+};
+
+export default function LoaderThrowResponse() {
+  const data = useLoaderData();
+
+  return (
+    <div>
+      <h1>Loader Throw Response</h1>
+      <span>{data ? data.message : 'No Data'} </span>
+    </div>
+  );
+}

packages/remix/test/integration/test/server/loader.test.ts

@@ -50,6 +50,33 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
     });
   });
 
+  it('reports a thrown error response the loader', async () => {
+    const env = await RemixTestEnv.init(adapter);
+    const url = `${env.url}/loader-throw-response/-1`;
+
+    const envelopes = await env.getMultipleEnvelopeRequest({ url, count: 1, envelopeType: ['event'] });
+    const event = envelopes[0][2];
+
+    assertSentryEvent(event, {
+      exception: {
+        values: [
+          {
+            type: 'Error',
+            value: 'Not found',
+            stacktrace: expect.any(Object),
+            mechanism: {
+              data: {
+                function: 'loader',
+              },
+              handled: false,
+              type: 'instrument',
+            },
+          },
+        ],
+      },
+    });
+  });
+
   it('correctly instruments a parameterized Remix API loader', async () => {
     const env = await RemixTestEnv.init(adapter);
     const url = `${env.url}/loader-json-response/123123`;
@@ -250,7 +277,8 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
 
     const envelopesCount = await env.countEnvelopes({
       url,
-      envelopeType: ['event'],
+      envelopeType: 'event',
+      timeout: 3000,
     });
 
     expect(envelopesCount).toBe(0);