Merge branch 'feat/tracing-add-more-network-timings' of github.com:ge…

…tsentry/sentry-javascript into feat/tracing-add-more-network-timings

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';

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 };

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;
+    });
+  }
+}

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'));
+  });
+});

packages/core/test/mocks/client.ts

@@ -54,7 +54,7 @@ export class TestClient extends BaseClient<TestClientOptions> {
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
   public eventFromException(exception: any): PromiseLike<Event> {
-    return resolvedSyncPromise({
+    const event: Event = {
       exception: {
         values: [
           {
@@ -65,7 +65,14 @@ export class TestClient extends BaseClient<TestClientOptions> {
           },
         ],
       },
-    });
+    };
+
+    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(

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);
+      }
+    });
+  }
+}

packages/remix/src/performance/client.tsx → 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<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 {

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 {

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 {

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;
+}

packages/remix/src/utils/instrumentServer.ts

@@ -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;
 
@@ -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) {
@@ -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);
 

packages/remix/src/utils/serverAdapters/express.ts

@@ -20,7 +20,7 @@ import type {
   ExpressResponse,
   ReactRouterDomPkg,
   ServerBuild,
-} from '../types';
+} from '../vendor/types';
 
 let pkg: ReactRouterDomPkg;