From 2c3066efd5c8047ceb48a4740afe6e043afe0713 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 13 Jul 2023 15:14:36 +0100 Subject: [PATCH 1/2] feat(remix): Add Remix v2 support (#8415) Adds support for new error handling utilities of Remix v2. ([ErrorBoundary](https://remix.run/docs/en/main/route/error-boundary-v2), [handleError](https://github.com/remix-run/remix/releases/tag/remix%401.17.0)) ## `ErrorBoundary` v2 Remix's `ErrorBoundary` captures all client / server / SSR errors and shows a customizable error page. In v1, to capture client-side errors we were wrapping the whole Remix application with `@sentry/react`s `ErrorBoundary` which caused inconsistencies in error pages. (See: https://github.com/getsentry/sentry-javascript/issues/5762) v2 implementation does not wrap user's application with `@sentry/react`'s ErrorBoundary, instead it exports a capturing utility to be used inside the Remix application's `ErrorBoundary` function. Can be used like: ```typescript import { captureRemixErrorBoundaryError } from '@sentry/remix'; export const ErrorBoundary: V2_ErrorBoundaryComponent = () => { const error = useRouteError(); captureRemixErrorBoundaryError(error); return
...
; }; ``` It also requires `v2_errorBoundary` [future flag](https://remix.run/docs/en/1.18.0/pages/api-development-strategy#current-future-flags) to be enabled. ## `handleError` For server-side errors apart from 'Error Responses' (thrown responses are handled in `ErrorBoundary`), this implementation exports another utility to be used in `handleError` function. The errors we capture in `handleError` also appear on `ErrorBoundary` functions but stacktraces are not available. So, we skip those errors in `captureRemixErrorBoundaryError` function. `handleError` can be instrumented as below: ```typescript export function handleError(error: unknown, { request }: DataFunctionArgs): void { if (error instanceof Error) { Sentry.captureRemixServerException(error, 'remix.server', request); } else { // Optionally Sentry.captureException(error); } ``` --- packages/remix/src/client/errors.tsx | 65 +++++++++++++++++++ .../client.tsx => client/performance.tsx} | 5 +- packages/remix/src/index.client.tsx | 3 +- packages/remix/src/index.server.ts | 4 +- packages/remix/src/utils/futureFlags.ts | 35 ++++++++++ packages/remix/src/utils/instrumentServer.ts | 30 +++++++-- .../remix/src/utils/serverAdapters/express.ts | 2 +- packages/remix/src/utils/vendor/response.ts | 2 +- .../remix/src/utils/{ => vendor}/types.ts | 37 +++++++++++ packages/remix/src/utils/web-fetch.ts | 2 +- .../test/integration/app_v2/entry.server.tsx | 10 ++- .../remix/test/integration/app_v2/root.tsx | 13 +++- .../routes/action-json-response.$id.tsx | 4 +- .../routes/loader-json-response.$id.tsx | 2 +- .../test/client/errorboundary.test.ts | 16 +++-- .../integration/test/server/action.test.ts | 22 ++++--- .../integration/test/server/loader.test.ts | 8 +-- 17 files changed, 225 insertions(+), 35 deletions(-) create mode 100644 packages/remix/src/client/errors.tsx rename packages/remix/src/{performance/client.tsx => client/performance.tsx} (95%) create mode 100644 packages/remix/src/utils/futureFlags.ts rename packages/remix/src/utils/{ => vendor}/types.ts (89%) 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', From 2b4121fad0e04724619de62d656e6323b8289f1f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 13 Jul 2023 19:09:19 +0100 Subject: [PATCH 2/2] feat(core): Add `ModuleMetadata` integration (#8475) - Adds the `ModuleMetadata` integration that fetches metadata injected via bundler plugins and attaches is to the `module_metadata` property of every `StackFrame`. - This can later be used in `beforeSend` or another integration to route events depending on the metadata. - This integration is - Exported separately from `@sentry/core` (ie. not in `Integrations`) so it doesn't get included in default bundles - Exported separately from `@sentry/browser` so that it can be used without depending directly on core - Uses the `beforeEnvelope` hook to strip the `module_metadata` property from stack frames - Adds a test to ensure `module_metadata` is available in `beforeSend` and is stripped before sending --- packages/browser/src/index.ts | 1 + packages/core/src/index.ts | 2 +- packages/core/src/integrations/metadata.ts | 57 ++++++++++++++++ .../test/lib/integrations/metadata.test.ts | 66 +++++++++++++++++++ packages/core/test/mocks/client.ts | 11 +++- 5 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/integrations/metadata.ts create mode 100644 packages/core/test/lib/integrations/metadata.test.ts diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 0a69fc44d858..9dbe7f977d7e 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -34,6 +34,7 @@ export { spanStatusfromHttpCode, trace, makeMultiplexedTransport, + ModuleMetadata, } from '@sentry/core'; export type { SpanStatusType } from '@sentry/core'; export type { Span } from '@sentry/types'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index db9d21c2f2e8..9ff61f05cdb3 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,7 +46,7 @@ export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { DEFAULT_ENVIRONMENT } from './constants'; - +export { ModuleMetadata } from './integrations/metadata'; import * as Integrations from './integrations'; export { Integrations }; diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts new file mode 100644 index 000000000000..05af1d88ebe9 --- /dev/null +++ b/packages/core/src/integrations/metadata.ts @@ -0,0 +1,57 @@ +import type { EventItem, EventProcessor, Hub, Integration } from '@sentry/types'; +import { forEachEnvelopeItem } from '@sentry/utils'; + +import { addMetadataToStackFrames, stripMetadataFromStackFrames } from '../metadata'; + +/** + * Adds module metadata to stack frames. + * + * Metadata can be injected by the Sentry bundler plugins using the `_experiments.moduleMetadata` config option. + * + * When this integration is added, the metadata passed to the bundler plugin is added to the stack frames of all events + * under the `module_metadata` property. This can be used to help in tagging or routing of events from different teams + * our sources + */ +export class ModuleMetadata implements Integration { + /* + * @inheritDoc + */ + public static id: string = 'ModuleMetadata'; + + /** + * @inheritDoc + */ + public name: string = ModuleMetadata.id; + + /** + * @inheritDoc + */ + public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void { + const client = getCurrentHub().getClient(); + + if (!client || typeof client.on !== 'function') { + return; + } + + // We need to strip metadata from stack frames before sending them to Sentry since these are client side only. + client.on('beforeEnvelope', envelope => { + forEachEnvelopeItem(envelope, (item, type) => { + if (type === 'event') { + const event = Array.isArray(item) ? (item as EventItem)[1] : undefined; + + if (event) { + stripMetadataFromStackFrames(event); + item[1] = event; + } + } + }); + }); + + const stackParser = client.getOptions().stackParser; + + addGlobalEventProcessor(event => { + addMetadataToStackFrames(stackParser, event); + return event; + }); + } +} diff --git a/packages/core/test/lib/integrations/metadata.test.ts b/packages/core/test/lib/integrations/metadata.test.ts new file mode 100644 index 000000000000..464da2d97fbb --- /dev/null +++ b/packages/core/test/lib/integrations/metadata.test.ts @@ -0,0 +1,66 @@ +import type { Event } from '@sentry/types'; +import { createStackParser, GLOBAL_OBJ, nodeStackLineParser, parseEnvelope } from '@sentry/utils'; +import { TextDecoder, TextEncoder } from 'util'; + +import { createTransport, getCurrentHub, ModuleMetadata } from '../../../src'; +import { getDefaultTestClientOptions, TestClient } from '../../mocks/client'; + +const stackParser = createStackParser(nodeStackLineParser()); + +const stack = new Error().stack || ''; + +describe('ModuleMetadata integration', () => { + beforeEach(() => { + TestClient.sendEventCalled = undefined; + TestClient.instance = undefined; + + GLOBAL_OBJ._sentryModuleMetadata = GLOBAL_OBJ._sentryModuleMetadata || {}; + GLOBAL_OBJ._sentryModuleMetadata[stack] = { team: 'frontend' }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('Adds and removes metadata from stack frames', done => { + const options = getDefaultTestClientOptions({ + dsn: 'https://username@domain/123', + enableSend: true, + stackParser, + integrations: [new ModuleMetadata()], + beforeSend: (event, _hint) => { + // copy the frames since reverse in in-place + const lastFrame = [...(event.exception?.values?.[0].stacktrace?.frames || [])].reverse()[0]; + // Ensure module_metadata is populated in beforeSend callback + expect(lastFrame?.module_metadata).toEqual({ team: 'frontend' }); + return event; + }, + transport: () => + createTransport({ recordDroppedEvent: () => undefined, textEncoder: new TextEncoder() }, async req => { + const [, items] = parseEnvelope(req.body, new TextEncoder(), new TextDecoder()); + + expect(items[0][1]).toBeDefined(); + const event = items[0][1] as Event; + const error = event.exception?.values?.[0]; + + // Ensure we're looking at the same error we threw + expect(error?.value).toEqual('Some error'); + + const lastFrame = [...(error?.stacktrace?.frames || [])].reverse()[0]; + // Ensure the last frame is in fact for this file + expect(lastFrame?.filename).toEqual(__filename); + + // Ensure module_metadata has been stripped from the event + expect(lastFrame?.module_metadata).toBeUndefined(); + + done(); + return {}; + }), + }); + + const client = new TestClient(options); + const hub = getCurrentHub(); + hub.bindClient(client); + hub.captureException(new Error('Some error')); + }); +}); diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index 7ca980189e19..f7d8830a1402 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -54,7 +54,7 @@ export class TestClient extends BaseClient { // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types public eventFromException(exception: any): PromiseLike { - return resolvedSyncPromise({ + const event: Event = { exception: { values: [ { @@ -65,7 +65,14 @@ export class TestClient extends BaseClient { }, ], }, - }); + }; + + const frames = this._options.stackParser(exception.stack || '', 1); + if (frames.length && event?.exception?.values?.[0]) { + event.exception.values[0] = { ...event.exception.values[0], stacktrace: { frames } }; + } + + return resolvedSyncPromise(event); } public eventFromMessage(