diff --git a/packages/remix/src/client/errors.tsx b/packages/remix/src/client/errors.tsx new file mode 100644 index 000000000000..9c9fd5c4b449 --- /dev/null +++ b/packages/remix/src/client/errors.tsx @@ -0,0 +1,65 @@ +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; +} + +/** + * Captures an error that is thrown inside a Remix ErrorBoundary. + * + * @param error The error to capture. + * @returns void + */ +export function captureRemixErrorBoundaryError(error: unknown): void { + const isClientSideRuntimeError = !isNodeEnv() && error instanceof Error; + const isRemixErrorResponse = isErrorResponse(error); + // Server-side errors apart from `ErrorResponse`s also appear here without their stacktraces. + // So, we only capture: + // 1. `ErrorResponse`s + // 2. Client-side runtime errors here, + // And other server - side errors in `handleError` function where stacktraces are available. + if (isRemixErrorResponse || isClientSideRuntimeError) { + const eventData = isRemixErrorResponse + ? { + function: 'ErrorResponse', + ...error.data, + } + : { + function: 'ReactError', + }; + + withScope(scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'instrument', + handled: true, + data: eventData, + }); + return event; + }); + + if (isRemixErrorResponse) { + if (isString(error.data)) { + captureException(error.data); + } else if (error.statusText) { + captureException(error.statusText); + } else { + captureException(error); + } + } else { + captureException(error); + } + }); + } +} diff --git a/packages/remix/src/performance/client.tsx b/packages/remix/src/client/performance.tsx similarity index 95% rename from packages/remix/src/performance/client.tsx rename to packages/remix/src/client/performance.tsx index 879c93e51f42..a3f7815b7bdc 100644 --- a/packages/remix/src/performance/client.tsx +++ b/packages/remix/src/client/performance.tsx @@ -4,6 +4,8 @@ import type { Transaction, TransactionContext } from '@sentry/types'; import { isNodeEnv, logger } from '@sentry/utils'; import * as React from 'react'; +import { getFutureFlagsBrowser } from '../utils/futureFlags'; + const DEFAULT_TAGS = { 'routing.instrumentation': 'remix-router', } as const; @@ -93,7 +95,8 @@ export function withSentry

, R extends React.FC wrapWithErrorBoundary?: boolean; errorBoundaryOptions?: ErrorBoundaryProps; } = { - wrapWithErrorBoundary: true, + // We don't want to wrap application with Sentry's ErrorBoundary by default for Remix v2 + wrapWithErrorBoundary: getFutureFlagsBrowser()?.v2_errorBoundary ? false : true, errorBoundaryOptions: {}, }, ): R { diff --git a/packages/remix/src/index.client.tsx b/packages/remix/src/index.client.tsx index 5c76ee4907bf..64951a3f10cd 100644 --- a/packages/remix/src/index.client.tsx +++ b/packages/remix/src/index.client.tsx @@ -3,7 +3,8 @@ import { configureScope, init as reactInit } from '@sentry/react'; import { buildMetadata } from './utils/metadata'; import type { RemixOptions } from './utils/remixOptions'; -export { remixRouterInstrumentation, withSentry } from './performance/client'; +export { remixRouterInstrumentation, withSentry } from './client/performance'; +export { captureRemixErrorBoundaryError } from './client/errors'; export * from '@sentry/react'; export function init(options: RemixOptions): void { diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 91839a809993..4c37351001c2 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -54,8 +54,10 @@ export { // Keeping the `*` exports for backwards compatibility and types export * from '@sentry/node'; +export { captureRemixServerException } from './utils/instrumentServer'; export { ErrorBoundary, withErrorBoundary } from '@sentry/react'; -export { remixRouterInstrumentation, withSentry } from './performance/client'; +export { remixRouterInstrumentation, withSentry } from './client/performance'; +export { captureRemixErrorBoundaryError } from './client/errors'; export { wrapExpressCreateRequestHandler } from './utils/serverAdapters/express'; function sdkAlreadyInitialized(): boolean { diff --git a/packages/remix/src/utils/futureFlags.ts b/packages/remix/src/utils/futureFlags.ts new file mode 100644 index 000000000000..7d797c19e8a2 --- /dev/null +++ b/packages/remix/src/utils/futureFlags.ts @@ -0,0 +1,35 @@ +import { GLOBAL_OBJ } from '@sentry/utils'; + +import type { FutureConfig, ServerBuild } from './vendor/types'; + +export type EnhancedGlobal = typeof GLOBAL_OBJ & { + __remixContext?: { + future?: FutureConfig; + }; +}; + +/** + * Get the future flags from the Remix browser context + * + * @returns The future flags + */ +export function getFutureFlagsBrowser(): FutureConfig | undefined { + const window = GLOBAL_OBJ as EnhancedGlobal; + + if (!window.__remixContext) { + return; + } + + return window.__remixContext.future; +} + +/** + * Get the future flags from the Remix server build + * + * @param build The Remix server build + * + * @returns The future flags + */ +export function getFutureFlagsServer(build: ServerBuild): FutureConfig | undefined { + return build.future; +} diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 64dea4cfb92b..b0ba45f69c26 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -13,12 +13,15 @@ import { tracingContextFromHeaders, } from '@sentry/utils'; +import { getFutureFlagsServer } from './futureFlags'; +import { extractData, getRequestMatch, isDeferredData, isResponse, json, matchServerRoutes } from './vendor/response'; import type { AppData, CreateRequestHandlerFunction, DataFunction, DataFunctionArgs, EntryContext, + FutureConfig, HandleDocumentRequestFunction, ReactRouterDomPkg, RemixRequest, @@ -26,10 +29,11 @@ import type { ServerBuild, ServerRoute, ServerRouteManifest, -} from './types'; -import { extractData, getRequestMatch, isDeferredData, isResponse, json, matchServerRoutes } from './vendor/response'; +} from './vendor/types'; import { normalizeRemixRequest } from './web-fetch'; +let FUTURE_FLAGS: FutureConfig | undefined; + // Flag to track if the core request handler is instrumented. export let isRequestHandlerWrapped = false; @@ -56,7 +60,16 @@ async function extractResponseError(response: Response): Promise { return responseData; } -async function captureRemixServerException(err: unknown, name: string, request: Request): Promise { +/** + * Captures an exception happened in the Remix server. + * + * @param err The error to capture. + * @param name The name of the origin function. + * @param request The request object. + * + * @returns A promise that resolves when the exception is captured. + */ +export async function captureRemixServerException(err: unknown, name: string, request: Request): Promise { // 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) { @@ -145,7 +158,10 @@ function makeWrappedDocumentRequestFunction( span?.finish(); } catch (err) { - await captureRemixServerException(err, 'documentRequest', request); + if (!FUTURE_FLAGS?.v2_errorBoundary) { + await captureRemixServerException(err, 'documentRequest', request); + } + throw err; } @@ -182,7 +198,10 @@ function makeWrappedDataFunction(origFn: DataFunction, id: string, name: 'action currentScope.setSpan(activeTransaction); span?.finish(); } catch (err) { - await captureRemixServerException(err, name, args.request); + if (!FUTURE_FLAGS?.v2_errorBoundary) { + await captureRemixServerException(err, name, args.request); + } + throw err; } @@ -439,6 +458,7 @@ function makeWrappedCreateRequestHandler( isRequestHandlerWrapped = true; return function (this: unknown, build: ServerBuild, ...args: unknown[]): RequestHandler { + FUTURE_FLAGS = getFutureFlagsServer(build); const newBuild = instrumentBuild(build); const requestHandler = origCreateRequestHandler.call(this, newBuild, ...args); diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts index 000ad3a00b15..742c938f2d06 100644 --- a/packages/remix/src/utils/serverAdapters/express.ts +++ b/packages/remix/src/utils/serverAdapters/express.ts @@ -20,7 +20,7 @@ import type { ExpressResponse, ReactRouterDomPkg, ServerBuild, -} from '../types'; +} from '../vendor/types'; let pkg: ReactRouterDomPkg; diff --git a/packages/remix/src/utils/vendor/response.ts b/packages/remix/src/utils/vendor/response.ts index ae85fff74734..fed25dd0f534 100644 --- a/packages/remix/src/utils/vendor/response.ts +++ b/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, ReactRouterDomPkg, RouteMatch, ServerRoute } from './types'; /** * Based on Remix Implementation diff --git a/packages/remix/src/utils/types.ts b/packages/remix/src/utils/vendor/types.ts similarity index 89% rename from packages/remix/src/utils/types.ts rename to packages/remix/src/utils/vendor/types.ts index 74dcf10215cc..faaa7e5f6f60 100644 --- a/packages/remix/src/utils/types.ts +++ b/packages/remix/src/utils/vendor/types.ts @@ -14,6 +14,42 @@ import type * as Express from 'express'; import type { Agent } from 'https'; import type { ComponentType } from 'react'; +type Dev = { + command?: string; + scheme?: string; + host?: string; + port?: number; + restart?: boolean; + tlsKey?: string; + tlsCert?: string; +}; + +export interface FutureConfig { + unstable_dev: boolean | Dev; + /** @deprecated Use the `postcss` config option instead */ + unstable_postcss: boolean; + /** @deprecated Use the `tailwind` config option instead */ + unstable_tailwind: boolean; + v2_errorBoundary: boolean; + v2_headers: boolean; + v2_meta: boolean; + v2_normalizeFormMethod: boolean; + v2_routeConvention: boolean; +} + +export interface RemixConfig { + [key: string]: any; + future: FutureConfig; +} + +export interface ErrorResponse { + status: number; + statusText: string; + data: any; + error?: Error; + internal: boolean; +} + export type RemixRequestState = { method: string; redirect: RequestRedirect; @@ -133,6 +169,7 @@ export interface ServerBuild { assets: AssetsManifest; publicPath?: string; assetsBuildDirectory?: string; + future?: FutureConfig; } export interface HandleDocumentRequestFunction { diff --git a/packages/remix/src/utils/web-fetch.ts b/packages/remix/src/utils/web-fetch.ts index 1e69a77b5dba..1961329c2f4b 100644 --- a/packages/remix/src/utils/web-fetch.ts +++ b/packages/remix/src/utils/web-fetch.ts @@ -24,8 +24,8 @@ import { logger } from '@sentry/utils'; -import type { RemixRequest } from './types'; import { getClientIPAddress } from './vendor/getIpAddress'; +import type { RemixRequest } from './vendor/types'; /* * Symbol extractor utility to be able to access internal fields of Remix requests. diff --git a/packages/remix/test/integration/app_v2/entry.server.tsx b/packages/remix/test/integration/app_v2/entry.server.tsx index d48f2644fac4..784cb2a39cd4 100644 --- a/packages/remix/test/integration/app_v2/entry.server.tsx +++ b/packages/remix/test/integration/app_v2/entry.server.tsx @@ -1,4 +1,4 @@ -import type { EntryContext } from '@remix-run/node'; +import type { EntryContext, DataFunctionArgs } from '@remix-run/node'; import { RemixServer } from '@remix-run/react'; import { renderToString } from 'react-dom/server'; import * as Sentry from '@sentry/remix'; @@ -11,6 +11,14 @@ Sentry.init({ autoSessionTracking: false, }); +export function handleError(error: unknown, { request }: DataFunctionArgs): void { + if (error instanceof Error) { + Sentry.captureRemixServerException(error, 'remix.server', request); + } else { + Sentry.captureException(error); + } +} + export default function handleRequest( request: Request, responseStatusCode: number, diff --git a/packages/remix/test/integration/app_v2/root.tsx b/packages/remix/test/integration/app_v2/root.tsx index 5af1f8cc7a1a..2320451cee74 100644 --- a/packages/remix/test/integration/app_v2/root.tsx +++ b/packages/remix/test/integration/app_v2/root.tsx @@ -1,6 +1,15 @@ import { V2_MetaFunction, LoaderFunction, json, defer, redirect } from '@remix-run/node'; -import { Links, LiveReload, Meta, Outlet, Scripts, ScrollRestoration } from '@remix-run/react'; -import { withSentry } from '@sentry/remix'; +import { Links, LiveReload, Meta, Outlet, Scripts, ScrollRestoration, useRouteError } from '@remix-run/react'; +import { V2_ErrorBoundaryComponent } from '@remix-run/react/dist/routeModules'; +import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix'; + +export const ErrorBoundary: V2_ErrorBoundaryComponent = () => { + const error = useRouteError(); + + captureRemixErrorBoundaryError(error); + + return

error
; +}; export const meta: V2_MetaFunction = ({ data }) => [ { charset: 'utf-8' }, diff --git a/packages/remix/test/integration/common/routes/action-json-response.$id.tsx b/packages/remix/test/integration/common/routes/action-json-response.$id.tsx index 2c7a19059596..ff0f6940fe44 100644 --- a/packages/remix/test/integration/common/routes/action-json-response.$id.tsx +++ b/packages/remix/test/integration/common/routes/action-json-response.$id.tsx @@ -3,8 +3,10 @@ import { useActionData } from '@remix-run/react'; export const loader: LoaderFunction = async ({ params: { id } }) => { if (id === '-1') { - throw new Error('Unexpected Server Error from Loader'); + throw new Error('Unexpected Server Error'); } + + return null; }; export const action: ActionFunction = async ({ params: { id } }) => { diff --git a/packages/remix/test/integration/common/routes/loader-json-response.$id.tsx b/packages/remix/test/integration/common/routes/loader-json-response.$id.tsx index 55b53e2d70dc..a4ad3dc48339 100644 --- a/packages/remix/test/integration/common/routes/loader-json-response.$id.tsx +++ b/packages/remix/test/integration/common/routes/loader-json-response.$id.tsx @@ -5,7 +5,7 @@ type LoaderData = { id: string }; export const loader: LoaderFunction = async ({ params: { id } }) => { if (id === '-2') { - throw new Error('Unexpected Server Error from Loader'); + throw new Error('Unexpected Server Error'); } if (id === '-1') { diff --git a/packages/remix/test/integration/test/client/errorboundary.test.ts b/packages/remix/test/integration/test/client/errorboundary.test.ts index b90b3e8d3eaa..9c496e3e3040 100644 --- a/packages/remix/test/integration/test/client/errorboundary.test.ts +++ b/packages/remix/test/integration/test/client/errorboundary.test.ts @@ -21,16 +21,20 @@ test('should capture React component errors.', async ({ page }) => { expect(errorEnvelope.level).toBe('error'); expect(errorEnvelope.sdk?.name).toBe('sentry.javascript.remix'); expect(errorEnvelope.exception?.values).toMatchObject([ - { - type: 'React ErrorBoundary Error', - value: 'Sentry React Component Error', - stacktrace: { frames: expect.any(Array) }, - }, + ...(!useV2 + ? [ + { + type: 'React ErrorBoundary Error', + value: 'Sentry React Component Error', + stacktrace: { frames: expect.any(Array) }, + }, + ] + : []), { type: 'Error', value: 'Sentry React Component Error', stacktrace: { frames: expect.any(Array) }, - mechanism: { type: 'generic', handled: true }, + mechanism: { type: useV2 ? 'instrument' : 'generic', handled: true }, }, ]); }); diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts index cc25a87611d4..a2a4632ba962 100644 --- a/packages/remix/test/integration/test/server/action.test.ts +++ b/packages/remix/test/integration/test/server/action.test.ts @@ -81,7 +81,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada stacktrace: expect.any(Object), mechanism: { data: { - function: 'action', + function: useV2 ? 'remix.server' : 'action', }, handled: true, type: 'instrument', @@ -197,11 +197,11 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada values: [ { type: 'Error', - value: 'Unexpected Server Error from Loader', + value: 'Unexpected Server Error', stacktrace: expect.any(Object), mechanism: { data: { - function: 'loader', + function: useV2 ? 'remix.server' : 'loader', }, handled: true, type: 'instrument', @@ -254,7 +254,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada stacktrace: expect.any(Object), mechanism: { data: { - function: 'action', + function: useV2 ? 'ErrorResponse' : 'action', }, handled: true, type: 'instrument', @@ -303,11 +303,13 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada values: [ { type: 'Error', - value: 'Non-Error exception captured with keys: data', + value: useV2 + ? 'Non-Error exception captured with keys: data, internal, status, statusText' + : 'Non-Error exception captured with keys: data', stacktrace: expect.any(Object), mechanism: { data: { - function: 'action', + function: useV2 ? 'ErrorResponse' : 'action', }, handled: true, type: 'instrument', @@ -360,7 +362,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada stacktrace: expect.any(Object), mechanism: { data: { - function: 'action', + function: useV2 ? 'ErrorResponse' : 'action', }, handled: true, type: 'instrument', @@ -409,11 +411,13 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada values: [ { type: 'Error', - value: 'Non-Error exception captured with keys: [object has no keys]', + value: useV2 + ? 'Non-Error exception captured with keys: data, internal, status, statusText' + : 'Non-Error exception captured with keys: [object has no keys]', stacktrace: expect.any(Object), mechanism: { data: { - function: 'action', + function: useV2 ? 'ErrorResponse' : 'action', }, handled: true, type: 'instrument', diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts index 8a99c699cc37..ccaa93b05e36 100644 --- a/packages/remix/test/integration/test/server/loader.test.ts +++ b/packages/remix/test/integration/test/server/loader.test.ts @@ -35,11 +35,11 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada values: [ { type: 'Error', - value: 'Unexpected Server Error from Loader', + value: 'Unexpected Server Error', stacktrace: expect.any(Object), mechanism: { data: { - function: 'loader', + function: useV2 ? 'remix.server' : 'loader', }, handled: true, type: 'instrument', @@ -134,11 +134,11 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada values: [ { type: 'Error', - value: 'Unexpected Server Error from Loader', + value: 'Unexpected Server Error', stacktrace: expect.any(Object), mechanism: { data: { - function: 'loader', + function: useV2 ? 'remix.server' : 'loader', }, handled: true, type: 'instrument',