fix(browser): Refactor breadcrumb handlers to use hint

Signed-off-by: Kaung Zin Hein <[email protected]>

Author: Zen-cronic

packages/browser/src/integrations/breadcrumbs.ts

@@ -36,6 +36,7 @@ import {
   safeJoin,
   severityLevelFromString,
 } from '@sentry/core';
+import type { FetchHint, XhrHint } from '@sentry-internal/replay';
 import { DEBUG_BUILD } from '../debug-build';
 import { WINDOW } from '../helpers';
 
@@ -258,7 +259,7 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr)
       level: getBreadcrumbLogLevelFromHttpStatusCode(status_code),
     };
 
-    client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
+    client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as XhrHint);
 
     addBreadcrumb(breadcrumb, hint);
   };
@@ -305,7 +306,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
         type: 'http',
       } satisfies Breadcrumb;
 
-      client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
+      client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as FetchHint);
 
       addBreadcrumb(breadcrumb, hint);
     } else {
@@ -329,7 +330,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
         level: getBreadcrumbLogLevelFromHttpStatusCode(data.status_code),
       };
 
-      client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
+      client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as FetchHint);
 
       addBreadcrumb(breadcrumb, hint);
     }

packages/browser/src/integrations/graphqlClient.ts

@@ -1,5 +1,6 @@
 import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
-import { FetchHint, getBodyString, XhrHint } from '@sentry-internal/replay';
+import { getBodyString } from '@sentry-internal/replay';
+import type { FetchHint, XhrHint } from '@sentry-internal/replay';
 import {
   SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
@@ -22,6 +23,11 @@ interface GraphQLRequestPayload {
   extensions?: Record<string, unknown>;
 }
 
+interface GraphQLOperation {
+  operationType: string | undefined;
+  operationName: string | undefined;
+}
+
 const INTEGRATION_NAME = 'GraphQLClient';
 
 const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
@@ -65,7 +71,6 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
       span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);
       span.setAttribute('graphql.document', payload);
     }
-    
   });
 }
 
@@ -92,11 +97,6 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient
           data['graphql.document'] = (graphqlBody as GraphQLRequestPayload).query;
           data['graphql.operation'] = operationInfo;
         }
-
-        // The body prop attached to HandlerDataFetch for the span should be removed.
-        if (isFetch && data.body) {
-          delete data.body;
-        }
       }
     }
   });
@@ -121,7 +121,7 @@ function _getGraphQLOperation(requestBody: GraphQLRequestPayload): string {
  */
 function _getRequestPayloadXhrOrFetch(hint: XhrHint | FetchHint): string | undefined {
   const isXhr = 'xhr' in hint;
-  const isFetch = !isXhr
+  const isFetch = !isXhr;
 
   let body: string | undefined;
 
@@ -136,6 +136,7 @@ function _getRequestPayloadXhrOrFetch(hint: XhrHint | FetchHint): string | undef
   return body;
 }
 
+// Duplicate from deprecated @sentry-utils/src/instrument/fetch.ts
 function hasProp<T extends string>(obj: unknown, prop: T): obj is Record<string, string> {
   return !!obj && typeof obj === 'object' && !!(obj as Record<string, string>)[prop];
 }
@@ -154,13 +155,9 @@ export function parseFetchPayload(fetchArgs: unknown[]): string | undefined {
   return hasProp(arg, 'body') ? String(arg.body) : undefined;
 }
 
-interface GraphQLOperation {
-  operationType: string | undefined;
-  operationName: string | undefined;
-}
-
 /**
  * Extract the name and type of the operation from the GraphQL query.
+ * Exported for tests only.
  * @param query
  */
 export function parseGraphQLQuery(query: string): GraphQLOperation {

packages/browser/src/tracing/request.ts

@@ -4,6 +4,7 @@ import {
   addXhrInstrumentationHandler,
 } from '@sentry-internal/browser-utils';
 import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/core';
+import type { XhrHint } from '@sentry-internal/replay';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -33,7 +34,6 @@ import {
   stringMatchesSomePattern,
 } from '@sentry/core';
 import { WINDOW } from '../helpers';
-import { XhrHint } from '@sentry-internal/replay';
 
 /** Options for Request Instrumentation */
 export interface RequestInstrumentationOptions {

packages/browser/test/integrations/graphqlClient.test.ts

@@ -1,11 +1,10 @@
-import { getGraphQLRequestPayload, parseFetchPayload, parseGraphQLQuery } from "../../src/integrations/graphqlClient";
+import { getGraphQLRequestPayload, parseFetchPayload, parseGraphQLQuery } from '../../src/integrations/graphqlClient';
 
 describe('GraphqlClient', () => {
-    describe('parseFetchPayload', () => {
-
+  describe('parseFetchPayload', () => {
     const data = [1, 2, 3];
     const jsonData = '{"data":[1,2,3]}';
-  
+
     it.each([
       ['string URL only', ['http://example.com'], undefined],
       ['URL object only', [new URL('http://example.com')], undefined],
@@ -28,10 +27,10 @@ describe('GraphqlClient', () => {
       ],
     ])('%s', (_name, args, expected) => {
       const actual = parseFetchPayload(args as unknown[]);
-  
+
       expect(actual).toEqual(expected);
     });
-  }); 
+  });
 
   describe('parseGraphQLQuery', () => {
     const queryOne = `query Test {
@@ -72,7 +71,7 @@ describe('GraphqlClient', () => {
       expect(parseGraphQLQuery(input)).toEqual(output);
     });
   });
-  
+
   describe('getGraphQLRequestPayload', () => {
     test('should return undefined for non-GraphQL request', () => {
       const requestBody = { data: [1, 2, 3] };
@@ -90,5 +89,3 @@ describe('GraphqlClient', () => {
     });
   });
 });
-
-

packages/core/src/client.ts

@@ -16,7 +16,6 @@ import type {
   FeedbackEvent,
   FetchBreadcrumbHint,
   HandlerDataFetch,
-  HandlerDataXhr,
   Integration,
   MonitorConfig,
   Outcome,
@@ -69,12 +68,12 @@ const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of missing
 type RequestBody = null | Blob | BufferSource | FormData | URLSearchParams | string;
 
 export type XhrHint = XhrBreadcrumbHint & {
-    xhr: XMLHttpRequest & SentryWrappedXMLHttpRequest;
-    input?: RequestBody;
+  xhr: XMLHttpRequest & SentryWrappedXMLHttpRequest;
+  input?: RequestBody;
 };
 export type FetchHint = FetchBreadcrumbHint & {
-    input: HandlerDataFetch['args'];
-    response: Response;
+  input: HandlerDataFetch['args'];
+  response: Response;
 };
 
 /**
@@ -596,15 +595,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;
 
   /** @inheritdoc */
-  public on(
-    hook: 'beforeOutgoingRequestSpan',
-    callback: (span: Span, hint: XhrHint | FetchHint ) => void,
-  ): () => void;
+  public on(hook: 'beforeOutgoingRequestSpan', callback: (span: Span, hint: XhrHint | FetchHint) => void): () => void;
 
   /** @inheritdoc */
   public on(
     hook: 'beforeOutgoingRequestBreadcrumb',
-    callback: (breadcrumb: Breadcrumb, handlerData: HandlerDataXhr | HandlerDataFetch) => void,
+    callback: (breadcrumb: Breadcrumb, hint: XhrHint | FetchHint) => void,
   ): () => void;
 
   public on(hook: 'flush', callback: () => void): () => void;
@@ -737,11 +733,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   public emit(hook: 'beforeOutgoingRequestSpan', span: Span, hint: XhrHint | FetchHint): void;
 
   /** @inheritdoc */
-  public emit(
-    hook: 'beforeOutgoingRequestBreadcrumb',
-    breadcrumb: Breadcrumb,
-    handlerData: HandlerDataXhr | HandlerDataFetch,
-  ): void;
+  public emit(hook: 'beforeOutgoingRequestBreadcrumb', breadcrumb: Breadcrumb, hint: XhrHint | FetchHint): void;
 
   /** @inheritdoc */
   public emit(hook: 'flush'): void;

packages/core/src/fetch.ts

@@ -6,6 +6,7 @@ import {
   isInstanceOf,
   parseUrl,
 } from '@sentry/utils';
+import type { FetchHint } from './baseclient';
 import { getClient, getCurrentScope, getIsolationScope } from './currentScopes';
 import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes';
 import { SPAN_STATUS_ERROR, setHttpStatus, startInactiveSpan } from './tracing';
@@ -17,7 +18,6 @@ import { parseUrl } from './utils-hoist/url';
 import { hasTracingEnabled } from './utils/hasTracingEnabled';
 import { getActiveSpan } from './utils/spanUtils';
 import { getTraceData } from './utils/traceData';
-import { FetchHint } from './baseclient';
 
 type PolymorphicRequestHeaders =
   | Record<string, string | undefined>
@@ -110,8 +110,14 @@ export function instrumentFetchRequest(
   }
 
   if (client) {
-    const fetchHint = { input: handlerData.args , response: handlerData.response, startTimestamp: handlerData.startTimestamp, endTimestamp: handlerData.endTimestamp } satisfies FetchHint
-    client.emit('beforeOutgoingRequestSpan', span, fetchHint);
+    // There's no 'input' key in HandlerDataFetch
+    const fetchHint = {
+      input: handlerData.args,
+      response: handlerData.response,
+      startTimestamp: handlerData.startTimestamp,
+      endTimestamp: handlerData.endTimestamp,
+    };
+    client.emit('beforeOutgoingRequestSpan', span, fetchHint as FetchHint);
   }
 
   return span;

packages/core/src/utils-hoist/instrument/fetch.ts

@@ -234,4 +234,3 @@ export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: str
     method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET',
   };
 }
-

packages/types/src/client.ts

@@ -9,7 +9,7 @@ import type { DynamicSamplingContext, Envelope } from './envelope';
 import type { Event, EventHint } from './event';
 import type { EventProcessor } from './eventprocessor';
 import type { FeedbackEvent } from './feedback';
-import type { HandlerDataFetch, HandlerDataXhr, SentryWrappedXMLHttpRequest } from './instrument';
+import type { HandlerDataFetch, SentryWrappedXMLHttpRequest } from './instrument';
 import type { Integration } from './integration';
 import type { ClientOptions } from './options';
 import type { ParameterizedString } from './parameterize';
@@ -309,18 +309,15 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    * A hook for GraphQL client integration to enhance a span with request data.
    * @returns A function that, when executed, removes the registered callback.
    */
-  on(
-    hook: 'beforeOutgoingRequestSpan',
-    callback: (span: Span, hint: XhrHint | FetchHint) => void,
-  ): () => void;
+  on(hook: 'beforeOutgoingRequestSpan', callback: (span: Span, hint: XhrHint | FetchHint) => void): () => void;
 
   /**
    * A hook for GraphQL client integration to enhance a breadcrumb with request data.
    * @returns A function that, when executed, removes the registered callback.
    */
   on(
     hook: 'beforeOutgoingRequestBreadcrumb',
-    callback: (breadcrumb: Breadcrumb, handlerData: HandlerDataXhr | HandlerDataFetch) => void,
+    callback: (breadcrumb: Breadcrumb, hint: XhrHint | FetchHint) => void,
   ): () => void;
 
   /**
@@ -427,11 +424,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
   /**
    * Emit a hook event for GraphQL client integration to enhance a breadcrumb with request data.
    */
-  emit(
-    hook: 'beforeOutgoingRequestBreadcrumb',
-    breadcrumb: Breadcrumb,
-    handlerData: HandlerDataXhr | HandlerDataFetch,
-  ): void;
+  emit(hook: 'beforeOutgoingRequestBreadcrumb', breadcrumb: Breadcrumb, hint: XhrHint | FetchHint): void;
 
   /**
    * Emit a hook event for client flush

packages/utils/test/instrument/fetch.test.ts

@@ -27,7 +27,6 @@ describe('instrument > parseFetchArgs', () => {
     expect(actual).toEqual(expected);
   });
 });
-
 describe('instrument > parseFetchPayload', () => {
   const data = [1, 2, 3];
   const jsonData = '{"data":[1,2,3]}';