Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(remix): Add v2 support #8415

Merged
merged 2 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions packages/remix/src/client/errors.tsx
Original file line number Diff line number Diff line change
@@ -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);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -93,7 +95,8 @@ export function withSentry<P extends Record<string, unknown>, 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 {
Expand Down
3 changes: 2 additions & 1 deletion packages/remix/src/index.client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion packages/remix/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
35 changes: 35 additions & 0 deletions packages/remix/src/utils/futureFlags.ts
Original file line number Diff line number Diff line change
@@ -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;
}
30 changes: 25 additions & 5 deletions packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,27 @@ 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,
RequestHandler,
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;

Expand All @@ -56,7 +60,16 @@ async function extractResponseError(response: Response): Promise<unknown> {
return responseData;
}

async function captureRemixServerException(err: unknown, name: string, request: Request): Promise<void> {
/**
* 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<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) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion packages/remix/src/utils/serverAdapters/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type {
ExpressResponse,
ReactRouterDomPkg,
ServerBuild,
} from '../types';
} from '../vendor/types';

let pkg: ReactRouterDomPkg;

Expand Down
2 changes: 1 addition & 1 deletion packages/remix/src/utils/vendor/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -133,6 +169,7 @@ export interface ServerBuild {
assets: AssetsManifest;
publicPath?: string;
assetsBuildDirectory?: string;
future?: FutureConfig;
}

export interface HandleDocumentRequestFunction {
Expand Down
2 changes: 1 addition & 1 deletion packages/remix/src/utils/web-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion packages/remix/test/integration/app_v2/entry.server.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down
13 changes: 11 additions & 2 deletions packages/remix/test/integration/app_v2/root.tsx
Original file line number Diff line number Diff line change
@@ -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 = () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remix question: The ErrorBoundary is picked up during build I assume, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It covers both build and runtime. Client-side React errors, and runtime server errors also end up here. For server-side runtime errors, handleError gives us the stacktraces (ErrorBoundary does not), so this implementation skips capturing for that case.

const error = useRouteError();

captureRemixErrorBoundaryError(error);

return <div>error</div>;
};

export const meta: V2_MetaFunction = ({ data }) => [
{ charset: 'utf-8' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
16 changes: 10 additions & 6 deletions packages/remix/test/integration/test/client/errorboundary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
},
]);
});
Loading