Skip to content

Commit fea96aa

Browse files
Zen-cronicZen-cronic
committed
fix(browser): Refactor breadcrumb handlers to use hint
Signed-off-by: Kaung Zin Hein <[email protected]>
1 parent fa29963 commit fea96aa

File tree

9 files changed

+41
-57
lines changed

9 files changed

+41
-57
lines changed

‎packages/browser/src/integrations/breadcrumbs.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
severityLevelFromString,
3434
} from '@sentry/utils';
3535

36+
import type { FetchHint, XhrHint } from '@sentry-internal/replay';
3637
import { DEBUG_BUILD } from '../debug-build';
3738
import { WINDOW } from '../helpers';
3839

@@ -255,7 +256,7 @@ function _getXhrBreadcrumbHandler(client: Client): (handlerData: HandlerDataXhr)
255256
level: getBreadcrumbLogLevelFromHttpStatusCode(status_code),
256257
};
257258

258-
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
259+
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as XhrHint);
259260

260261
addBreadcrumb(breadcrumb, hint);
261262
};
@@ -298,7 +299,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
298299
type: 'http',
299300
} satisfies Breadcrumb;
300301

301-
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
302+
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as FetchHint);
302303

303304
addBreadcrumb(breadcrumb, hint);
304305
} else {
@@ -321,7 +322,7 @@ function _getFetchBreadcrumbHandler(client: Client): (handlerData: HandlerDataFe
321322
level: getBreadcrumbLogLevelFromHttpStatusCode(data.status_code),
322323
};
323324

324-
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, handlerData);
325+
client.emit('beforeOutgoingRequestBreadcrumb', breadcrumb, hint as FetchHint);
325326

326327
addBreadcrumb(breadcrumb, hint);
327328
}

‎packages/browser/src/integrations/graphqlClient.ts‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
2-
import { FetchHint, getBodyString, XhrHint } from '@sentry-internal/replay';
2+
import { getBodyString } from '@sentry-internal/replay';
3+
import type { FetchHint, XhrHint } from '@sentry-internal/replay';
34
import {
45
SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
56
SEMANTIC_ATTRIBUTE_SENTRY_OP,
@@ -22,6 +23,11 @@ interface GraphQLRequestPayload {
2223
extensions?: Record<string, unknown>;
2324
}
2425

26+
interface GraphQLOperation {
27+
operationType: string | undefined;
28+
operationName: string | undefined;
29+
}
30+
2531
const INTEGRATION_NAME = 'GraphQLClient';
2632

2733
const _graphqlClientIntegration = ((options: GraphQLClientOptions) => {
@@ -65,7 +71,6 @@ function _updateSpanWithGraphQLData(client: Client, options: GraphQLClientOption
6571
span.updateName(`${httpMethod} ${httpUrl} (${operationInfo})`);
6672
span.setAttribute('graphql.document', payload);
6773
}
68-
6974
});
7075
}
7176

@@ -92,11 +97,6 @@ function _updateBreadcrumbWithGraphQLData(client: Client, options: GraphQLClient
9297
data['graphql.document'] = (graphqlBody as GraphQLRequestPayload).query;
9398
data['graphql.operation'] = operationInfo;
9499
}
95-
96-
// The body prop attached to HandlerDataFetch for the span should be removed.
97-
if (isFetch && data.body) {
98-
delete data.body;
99-
}
100100
}
101101
}
102102
});
@@ -121,7 +121,7 @@ function _getGraphQLOperation(requestBody: GraphQLRequestPayload): string {
121121
*/
122122
function _getRequestPayloadXhrOrFetch(hint: XhrHint | FetchHint): string | undefined {
123123
const isXhr = 'xhr' in hint;
124-
const isFetch = !isXhr
124+
const isFetch = !isXhr;
125125

126126
let body: string | undefined;
127127

@@ -136,6 +136,7 @@ function _getRequestPayloadXhrOrFetch(hint: XhrHint | FetchHint): string | undef
136136
return body;
137137
}
138138

139+
// Duplicate from deprecated @sentry-utils/src/instrument/fetch.ts
139140
function hasProp<T extends string>(obj: unknown, prop: T): obj is Record<string, string> {
140141
return !!obj && typeof obj === 'object' && !!(obj as Record<string, string>)[prop];
141142
}
@@ -154,13 +155,9 @@ export function parseFetchPayload(fetchArgs: unknown[]): string | undefined {
154155
return hasProp(arg, 'body') ? String(arg.body) : undefined;
155156
}
156157

157-
interface GraphQLOperation {
158-
operationType: string | undefined;
159-
operationName: string | undefined;
160-
}
161-
162158
/**
163159
* Extract the name and type of the operation from the GraphQL query.
160+
* Exported for tests only.
164161
* @param query
165162
*/
166163
export function parseGraphQLQuery(query: string): GraphQLOperation {

‎packages/browser/src/tracing/request.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
addPerformanceInstrumentationHandler,
44
addXhrInstrumentationHandler,
55
} from '@sentry-internal/browser-utils';
6+
import type { XhrHint } from '@sentry-internal/replay';
67
import {
78
SEMANTIC_ATTRIBUTE_SENTRY_OP,
89
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
@@ -32,7 +33,6 @@ import {
3233
stringMatchesSomePattern,
3334
} from '@sentry/utils';
3435
import { WINDOW } from '../helpers';
35-
import { XhrHint } from '@sentry-internal/replay';
3636

3737
/** Options for Request Instrumentation */
3838
export interface RequestInstrumentationOptions {

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
import { getGraphQLRequestPayload, parseFetchPayload, parseGraphQLQuery } from "../../src/integrations/graphqlClient";
1+
import { getGraphQLRequestPayload, parseFetchPayload, parseGraphQLQuery } from '../../src/integrations/graphqlClient';
22

33
describe('GraphqlClient', () => {
4-
describe('parseFetchPayload', () => {
5-
4+
describe('parseFetchPayload', () => {
65
const data = [1, 2, 3];
76
const jsonData = '{"data":[1,2,3]}';
8-
7+
98
it.each([
109
['string URL only', ['http://example.com'], undefined],
1110
['URL object only', [new URL('http://example.com')], undefined],
@@ -28,10 +27,10 @@ describe('GraphqlClient', () => {
2827
],
2928
])('%s', (_name, args, expected) => {
3029
const actual = parseFetchPayload(args as unknown[]);
31-
30+
3231
expect(actual).toEqual(expected);
3332
});
34-
});
33+
});
3534

3635
describe('parseGraphQLQuery', () => {
3736
const queryOne = `query Test {
@@ -72,7 +71,7 @@ describe('GraphqlClient', () => {
7271
expect(parseGraphQLQuery(input)).toEqual(output);
7372
});
7473
});
75-
74+
7675
describe('getGraphQLRequestPayload', () => {
7776
test('should return undefined for non-GraphQL request', () => {
7877
const requestBody = { data: [1, 2, 3] };
@@ -90,5 +89,3 @@ describe('GraphqlClient', () => {
9089
});
9190
});
9291
});
93-
94-

‎packages/core/src/baseclient.ts‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import type {
1616
FeedbackEvent,
1717
FetchBreadcrumbHint,
1818
HandlerDataFetch,
19-
HandlerDataXhr,
2019
Integration,
2120
Outcome,
2221
ParameterizedString,
@@ -73,12 +72,12 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca
7372
type RequestBody = null | Blob | BufferSource | FormData | URLSearchParams | string;
7473

7574
export type XhrHint = XhrBreadcrumbHint & {
76-
xhr: XMLHttpRequest & SentryWrappedXMLHttpRequest;
77-
input?: RequestBody;
75+
xhr: XMLHttpRequest & SentryWrappedXMLHttpRequest;
76+
input?: RequestBody;
7877
};
7978
export type FetchHint = FetchBreadcrumbHint & {
80-
input: HandlerDataFetch['args'];
81-
response: Response;
79+
input: HandlerDataFetch['args'];
80+
response: Response;
8281
};
8382

8483
/**
@@ -479,15 +478,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
479478
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;
480479

481480
/** @inheritdoc */
482-
public on(
483-
hook: 'beforeOutgoingRequestSpan',
484-
callback: (span: Span, hint: XhrHint | FetchHint ) => void,
485-
): () => void;
481+
public on(hook: 'beforeOutgoingRequestSpan', callback: (span: Span, hint: XhrHint | FetchHint) => void): () => void;
486482

487483
/** @inheritdoc */
488484
public on(
489485
hook: 'beforeOutgoingRequestBreadcrumb',
490-
callback: (breadcrumb: Breadcrumb, handlerData: HandlerDataXhr | HandlerDataFetch) => void,
486+
callback: (breadcrumb: Breadcrumb, hint: XhrHint | FetchHint) => void,
491487
): () => void;
492488

493489
public on(hook: 'flush', callback: () => void): () => void;
@@ -572,11 +568,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
572568
public emit(hook: 'beforeOutgoingRequestSpan', span: Span, hint: XhrHint | FetchHint): void;
573569

574570
/** @inheritdoc */
575-
public emit(
576-
hook: 'beforeOutgoingRequestBreadcrumb',
577-
breadcrumb: Breadcrumb,
578-
handlerData: HandlerDataXhr | HandlerDataFetch,
579-
): void;
571+
public emit(hook: 'beforeOutgoingRequestBreadcrumb', breadcrumb: Breadcrumb, hint: XhrHint | FetchHint): void;
580572

581573
/** @inheritdoc */
582574
public emit(hook: 'flush'): void;

‎packages/core/src/fetch.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
isInstanceOf,
77
parseUrl,
88
} from '@sentry/utils';
9+
import type { FetchHint } from './baseclient';
910
import { getClient, getCurrentScope, getIsolationScope } from './currentScopes';
1011
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from './semanticAttributes';
1112
import {
@@ -18,7 +19,6 @@ import {
1819
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
1920
import { hasTracingEnabled } from './utils/hasTracingEnabled';
2021
import { getActiveSpan, spanToTraceHeader } from './utils/spanUtils';
21-
import { FetchHint } from './baseclient';
2222

2323
type PolymorphicRequestHeaders =
2424
| Record<string, string | undefined>
@@ -114,8 +114,14 @@ export function instrumentFetchRequest(
114114
}
115115

116116
if (client) {
117-
const fetchHint = { input: handlerData.args , response: handlerData.response, startTimestamp: handlerData.startTimestamp, endTimestamp: handlerData.endTimestamp } satisfies FetchHint
118-
client.emit('beforeOutgoingRequestSpan', span, fetchHint);
117+
// There's no 'input' key in HandlerDataFetch
118+
const fetchHint = {
119+
input: handlerData.args,
120+
response: handlerData.response,
121+
startTimestamp: handlerData.startTimestamp,
122+
endTimestamp: handlerData.endTimestamp,
123+
};
124+
client.emit('beforeOutgoingRequestSpan', span, fetchHint as FetchHint);
119125
}
120126

121127
return span;

‎packages/types/src/client.ts‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type { DynamicSamplingContext, Envelope } from './envelope';
99
import type { Event, EventHint } from './event';
1010
import type { EventProcessor } from './eventprocessor';
1111
import type { FeedbackEvent } from './feedback';
12-
import type { HandlerDataFetch, HandlerDataXhr, SentryWrappedXMLHttpRequest } from './instrument';
12+
import type { HandlerDataFetch, SentryWrappedXMLHttpRequest } from './instrument';
1313
import type { Integration } from './integration';
1414
import type { ClientOptions } from './options';
1515
import type { ParameterizedString } from './parameterize';
@@ -309,18 +309,15 @@ export interface Client<O extends ClientOptions = ClientOptions> {
309309
* A hook for GraphQL client integration to enhance a span with request data.
310310
* @returns A function that, when executed, removes the registered callback.
311311
*/
312-
on(
313-
hook: 'beforeOutgoingRequestSpan',
314-
callback: (span: Span, hint: XhrHint | FetchHint) => void,
315-
): () => void;
312+
on(hook: 'beforeOutgoingRequestSpan', callback: (span: Span, hint: XhrHint | FetchHint) => void): () => void;
316313

317314
/**
318315
* A hook for GraphQL client integration to enhance a breadcrumb with request data.
319316
* @returns A function that, when executed, removes the registered callback.
320317
*/
321318
on(
322319
hook: 'beforeOutgoingRequestBreadcrumb',
323-
callback: (breadcrumb: Breadcrumb, handlerData: HandlerDataXhr | HandlerDataFetch) => void,
320+
callback: (breadcrumb: Breadcrumb, hint: XhrHint | FetchHint) => void,
324321
): () => void;
325322

326323
/**
@@ -427,11 +424,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
427424
/**
428425
* Emit a hook event for GraphQL client integration to enhance a breadcrumb with request data.
429426
*/
430-
emit(
431-
hook: 'beforeOutgoingRequestBreadcrumb',
432-
breadcrumb: Breadcrumb,
433-
handlerData: HandlerDataXhr | HandlerDataFetch,
434-
): void;
427+
emit(hook: 'beforeOutgoingRequestBreadcrumb', breadcrumb: Breadcrumb, hint: XhrHint | FetchHint): void;
435428

436429
/**
437430
* Emit a hook event for client flush

‎packages/utils/src/instrument/fetch.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,4 +214,3 @@ export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: str
214214
method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET',
215215
};
216216
}
217-

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,3 @@ describe('instrument > parseFetchArgs', () => {
2727
expect(actual).toEqual(expected);
2828
});
2929
});
30-

0 commit comments

Comments
 (0)