Switch to isRouteErrorResponse on Remix v2.

Author: onurtemizkan

packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx

@@ -7,10 +7,14 @@
 import { PassThrough } from 'node:stream';
 
 import type { AppLoadContext, EntryContext, DataFunctionArgs } from '@remix-run/node';
-import { json } from '@remix-run/node';
+import { createReadableStreamFromReadable } from '@remix-run/node';
 import { RemixServer } from '@remix-run/react';
 import { renderToPipeableStream } from 'react-dom/server';
 import * as Sentry from '@sentry/remix';
+import { installGlobals } from '@remix-run/node';
+import isbot from 'isbot';
+
+installGlobals();
 
 const ABORT_DELAY = 5_000;
 
@@ -21,11 +25,7 @@ Sentry.init({
 });
 
 export function handleError(error: unknown, { request }: DataFunctionArgs): void {
-  if (error instanceof Error) {
-    Sentry.captureRemixServerException(error, 'remix.server', request);
-  } else {
-    Sentry.captureException(error);
-  }
+  Sentry.captureRemixServerException(error, 'remix.server', request);
 }
 
 export default function handleRequest(
@@ -35,7 +35,55 @@ export default function handleRequest(
   remixContext: EntryContext,
   loadContext: AppLoadContext,
 ) {
-  handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext);
+  return isbot(request.headers.get('user-agent'))
+    ? handleBotRequest(request, responseStatusCode, responseHeaders, remixContext)
+    : handleBrowserRequest(request, responseStatusCode, responseHeaders, remixContext);
+}
+
+function handleBotRequest(
+  request: Request,
+  responseStatusCode: number,
+  responseHeaders: Headers,
+  remixContext: EntryContext,
+) {
+  return new Promise((resolve, reject) => {
+    let shellRendered = false;
+    const { pipe, abort } = renderToPipeableStream(
+      <RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
+      {
+        onAllReady() {
+          shellRendered = true;
+          const body = new PassThrough();
+          const stream = createReadableStreamFromReadable(body);
+
+          responseHeaders.set('Content-Type', 'text/html');
+
+          resolve(
+            new Response(stream, {
+              headers: responseHeaders,
+              status: responseStatusCode,
+            }),
+          );
+
+          pipe(body);
+        },
+        onShellError(error: unknown) {
+          reject(error);
+        },
+        onError(error: unknown) {
+          responseStatusCode = 500;
+          // Log streaming rendering errors from inside the shell.  Don't log
+          // errors encountered during initial shell rendering since they'll
+          // reject and get logged in handleDocumentRequest.
+          if (shellRendered) {
+            console.error(error);
+          }
+        },
+      },
+    );
+
+    setTimeout(abort, ABORT_DELAY);
+  });
 }
 
 function handleBrowserRequest(
@@ -45,16 +93,19 @@ function handleBrowserRequest(
   remixContext: EntryContext,
 ) {
   return new Promise((resolve, reject) => {
+    let shellRendered = false;
     const { pipe, abort } = renderToPipeableStream(
       <RemixServer context={remixContext} url={request.url} abortDelay={ABORT_DELAY} />,
       {
         onShellReady() {
+          shellRendered = true;
           const body = new PassThrough();
+          const stream = createReadableStreamFromReadable(body);
 
           responseHeaders.set('Content-Type', 'text/html');
 
           resolve(
-            json(body, {
+            new Response(stream, {
               headers: responseHeaders,
               status: responseStatusCode,
             }),
@@ -66,8 +117,13 @@ function handleBrowserRequest(
           reject(error);
         },
         onError(error: unknown) {
-          console.error(error);
           responseStatusCode = 500;
+          // Log streaming rendering errors from inside the shell.  Don't log
+          // errors encountered during initial shell rendering since they'll
+          // reject and get logged in handleDocumentRequest.
+          if (shellRendered) {
+            console.error(error);
+          }
         },
       },
     );

packages/remix/src/client/errors.tsx

@@ -1,19 +1,7 @@
 import { captureException, withScope } from '@sentry/core';
 import { addExceptionMechanism, isNodeEnv, isString } from '@sentry/utils';
 
-import type { ErrorResponse } from '../utils/vendor/types';
-
-/**
- * Checks whether the given error is an ErrorResponse.
- * ErrorResponse is when users throw a response from their loader or action functions.
- * This is in fact a server-side error that we capture on the client.
- *
- * @param error The error to check.
- * @returns boolean
- */
-function isErrorResponse(error: unknown): error is ErrorResponse {
-  return typeof error === 'object' && error !== null && 'status' in error && 'statusText' in error;
-}
+import { isRouteErrorResponse } from '../utils/vendor/response';
 
 /**
  * Captures an error that is thrown inside a Remix ErrorBoundary.
@@ -24,7 +12,7 @@ function isErrorResponse(error: unknown): error is ErrorResponse {
 export function captureRemixErrorBoundaryError(error: unknown): string | undefined {
   let eventId: string | undefined;
   const isClientSideRuntimeError = !isNodeEnv() && error instanceof Error;
-  const isRemixErrorResponse = isErrorResponse(error);
+  const isRemixErrorResponse = isRouteErrorResponse(error);
   // Server-side errors apart from `ErrorResponse`s also appear here without their stacktraces.
   // So, we only capture:
   //    1. `ErrorResponse`s

packages/remix/src/utils/instrumentServer.ts

@@ -14,7 +14,15 @@ import {
 } from '@sentry/utils';
 
 import { getFutureFlagsServer, getRemixVersionFromBuild } from './futureFlags';
-import { extractData, getRequestMatch, isDeferredData, isResponse, json, matchServerRoutes } from './vendor/response';
+import {
+  extractData,
+  getRequestMatch,
+  isDeferredData,
+  isResponse,
+  isRouteErrorResponse,
+  json,
+  matchServerRoutes,
+} from './vendor/response';
 import type {
   AppData,
   CreateRequestHandlerFunction,
@@ -33,6 +41,7 @@ import type {
 import { normalizeRemixRequest } from './web-fetch';
 
 let FUTURE_FLAGS: FutureConfig | undefined;
+let IS_REMIX_V2: boolean | undefined;
 
 // Flag to track if the core request handler is instrumented.
 export let isRequestHandlerWrapped = false;
@@ -72,7 +81,11 @@ async function extractResponseError(response: Response): Promise<unknown> {
 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 (isResponse(err) && err.status < 500) {
+  if (IS_REMIX_V2) {
+    if (isRouteErrorResponse(err) && err.status < 500) {
+      return;
+    }
+  } else if (isResponse(err) && err.status < 500) {
     return;
   }
 
@@ -397,7 +410,10 @@ function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBui
 
       const res = (await origRequestHandler.call(this, request, loadContext)) as Response;
 
-      transaction.setHttpStatus(res.status);
+      if (isResponse(res)) {
+        transaction.setHttpStatus(res.status);
+      }
+
       transaction.finish();
 
       return res;
@@ -412,6 +428,7 @@ export function instrumentBuild(build: ServerBuild): ServerBuild {
   const routes: ServerRouteManifest = {};
 
   const remixVersion = getRemixVersionFromBuild(build);
+  IS_REMIX_V2 = remixVersion === 2;
 
   const wrappedEntry = { ...build.entry, module: { ...build.entry.module } };
 

packages/remix/src/utils/vendor/response.ts

@@ -6,7 +6,7 @@
 //
 // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-import type { DeferredData, ReactRouterDomPkg, RouteMatch, ServerRoute } from './types';
+import type { DeferredData, ErrorResponse, ReactRouterDomPkg, RouteMatch, ServerRoute } from './types';
 
 /**
  * Based on Remix Implementation
@@ -140,3 +140,22 @@ export function isDeferredData(value: any): value is DeferredData {
     typeof deferred.resolveData === 'function'
   );
 }
+
+/**
+ * https://github.com/remix-run/react-router/blob/f9b3dbd9cbf513366c456b33d95227f42f36da63/packages/router/utils.ts#L1574
+ *
+ * Check if the given error is an ErrorResponse generated from a 4xx/5xx
+ * Response thrown from an action/loader
+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function isRouteErrorResponse(value: any): value is ErrorResponse {
+  const error: ErrorResponse = value;
+
+  return (
+    error != null &&
+    typeof error.status === 'number' &&
+    typeof error.statusText === 'string' &&
+    typeof error.internal === 'boolean' &&
+    'data' in error
+  );
+}

packages/remix/test/integration/app_v2/entry.server.tsx

@@ -12,11 +12,7 @@ Sentry.init({
 });
 
 export function handleError(error: unknown, { request }: DataFunctionArgs): void {
-  if (error instanceof Error) {
-    Sentry.captureRemixServerException(error, 'remix.server', request);
-  } else {
-    Sentry.captureException(error);
-  }
+  Sentry.captureRemixServerException(error, 'remix.server', request);
 }
 
 export default function handleRequest(