From dfd7920694a10e166cf3abacb3008ea1f9a14401 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 21 Oct 2025 11:24:21 +0200 Subject: [PATCH 01/13] Track GraphQl Request --- .../configuration/configuration.spec.ts | 28 ++- .../src/domain/configuration/configuration.ts | 17 +- .../src/domain/requestCollection.spec.ts | 69 +++++++- .../rum-core/src/domain/requestCollection.ts | 98 +++++++---- .../src/domain/resource/graphql.spec.ts | 160 ++++++++++++++++-- .../rum-core/src/domain/resource/graphql.ts | 101 ++++++++++- .../src/domain/resource/resourceCollection.ts | 4 +- test/e2e/lib/framework/serverApps/mock.ts | 29 +++- test/e2e/scenario/rum/graphql.scenario.ts | 88 +++++++++- 9 files changed, 528 insertions(+), 66 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 71eed948da..e476f5a736 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -524,8 +524,8 @@ describe('validateAndBuildRumConfiguration', () => { allowedGraphQlUrls: ['https://api.example.com/graphql', '/graphql'], })! expect(configuration.allowedGraphQlUrls).toEqual([ - { match: 'https://api.example.com/graphql', trackPayload: false }, - { match: '/graphql', trackPayload: false }, + { match: 'https://api.example.com/graphql', trackPayload: false, trackResponseErrors: false }, + { match: '/graphql', trackPayload: false, trackResponseErrors: false }, ]) }) @@ -535,8 +535,8 @@ describe('validateAndBuildRumConfiguration', () => { allowedGraphQlUrls: [{ match: /\/graphql$/i }, { match: 'https://api.example.com/graphql' }], })! expect(configuration.allowedGraphQlUrls).toEqual([ - { match: /\/graphql$/i, trackPayload: false }, - { match: 'https://api.example.com/graphql', trackPayload: false }, + { match: /\/graphql$/i, trackPayload: false, trackResponseErrors: false }, + { match: 'https://api.example.com/graphql', trackPayload: false, trackResponseErrors: false }, ]) }) @@ -546,7 +546,9 @@ describe('validateAndBuildRumConfiguration', () => { ...DEFAULT_INIT_CONFIGURATION, allowedGraphQlUrls: [{ match: customMatcher }], })! - expect(configuration.allowedGraphQlUrls).toEqual([{ match: customMatcher, trackPayload: false }]) + expect(configuration.allowedGraphQlUrls).toEqual([ + { match: customMatcher, trackPayload: false, trackResponseErrors: false }, + ]) }) it('should accept GraphQL options with trackPayload', () => { @@ -554,7 +556,19 @@ describe('validateAndBuildRumConfiguration', () => { ...DEFAULT_INIT_CONFIGURATION, allowedGraphQlUrls: [{ match: '/graphql', trackPayload: true }], })! - expect(configuration.allowedGraphQlUrls).toEqual([{ match: '/graphql', trackPayload: true }]) + expect(configuration.allowedGraphQlUrls).toEqual([ + { match: '/graphql', trackPayload: true, trackResponseErrors: false }, + ]) + }) + + it('should accept GraphQL options with trackResponseErrors', () => { + const configuration = validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + allowedGraphQlUrls: [{ match: '/graphql', trackResponseErrors: true }], + })! + expect(configuration.allowedGraphQlUrls).toEqual([ + { match: '/graphql', trackResponseErrors: true, trackPayload: false }, + ]) }) it('should reject invalid values', () => { @@ -621,6 +635,7 @@ describe('serializeRumConfiguration', () => { | MapRumInitConfigurationKey | 'selected_tracing_propagators' | 'use_track_graph_ql_payload' + | 'use_track_graph_ql_response_errors' > = serializeRumConfiguration(exhaustiveRumInitConfiguration) expect(serializedConfiguration).toEqual({ @@ -632,6 +647,7 @@ describe('serializeRumConfiguration', () => { use_allowed_tracing_urls: true, use_allowed_graph_ql_urls: true, use_track_graph_ql_payload: false, + use_track_graph_ql_response_errors: false, selected_tracing_propagators: ['tracecontext', 'datadog'], use_excluded_activity_urls: true, track_user_interactions: true, diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 3711903cb8..f41e76b0ca 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -280,6 +280,7 @@ export type FeatureFlagsForEvents = 'vital' | 'action' | 'long_task' | 'resource export interface GraphQlUrlOption { match: MatchOption trackPayload?: boolean + trackResponseErrors?: boolean } export interface RumConfiguration extends Configuration { @@ -461,11 +462,12 @@ function validateAndBuildGraphQlOptions(initConfiguration: RumInitConfiguration) initConfiguration.allowedGraphQlUrls.forEach((option) => { if (isMatchOption(option)) { - graphQlOptions.push({ match: option, trackPayload: false }) + graphQlOptions.push({ match: option, trackPayload: false, trackResponseErrors: false }) } else if (option && typeof option === 'object' && 'match' in option && isMatchOption(option.match)) { graphQlOptions.push({ match: option.match, trackPayload: !!option.trackPayload, + trackResponseErrors: !!option.trackResponseErrors, }) } }) @@ -485,6 +487,18 @@ function hasGraphQlPayloadTracking(allowedGraphQlUrls: RumInitConfiguration['all ) } +function hasGraphQlResponseErrorsTracking(allowedGraphQlUrls: RumInitConfiguration['allowedGraphQlUrls']): boolean { + return ( + isNonEmptyArray(allowedGraphQlUrls) && + allowedGraphQlUrls.some((option) => { + if (typeof option === 'object' && 'trackResponseErrors' in option) { + return !!option.trackResponseErrors + } + return false + }) + ) +} + export function serializeRumConfiguration(configuration: RumInitConfiguration) { const baseSerializedConfiguration = serializeConfiguration(configuration) @@ -498,6 +512,7 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { use_allowed_tracing_urls: isNonEmptyArray(configuration.allowedTracingUrls), use_allowed_graph_ql_urls: isNonEmptyArray(configuration.allowedGraphQlUrls), use_track_graph_ql_payload: hasGraphQlPayloadTracking(configuration.allowedGraphQlUrls), + use_track_graph_ql_response_errors: hasGraphQlResponseErrorsTracking(configuration.allowedGraphQlUrls), selected_tracing_propagators: getSelectedTracingPropagators(configuration), default_privacy_level: configuration.defaultPrivacyLevel, enable_privacy_for_action_name: configuration.enablePrivacyForActionName, diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts index cdd7218875..ac5db29011 100644 --- a/packages/rum-core/src/domain/requestCollection.spec.ts +++ b/packages/rum-core/src/domain/requestCollection.spec.ts @@ -35,7 +35,7 @@ describe('collect fetch', () => { context.spanId = createSpanIdentifier() }, } - ;({ stop: stopFetchTracking } = trackFetch(lifeCycle, tracerStub as Tracer)) + ;({ stop: stopFetchTracking } = trackFetch(lifeCycle, mockRumConfiguration(), tracerStub as Tracer)) fetch = window.fetch as MockFetch @@ -335,3 +335,70 @@ describe('collect xhr', () => { }) }) }) + +describe('GraphQL response errors tracking', () => { + const FAKE_GRAPHQL_URL = 'http://fake-url/graphql' + + it('should extract GraphQL errors when trackResponseErrors is enabled', (done) => { + const mockFetchManager = mockFetch() + const completeSpy = jasmine.createSpy('requestComplete') + const lifeCycle = new LifeCycle() + lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy) + + const configuration = mockRumConfiguration({ + allowedGraphQlUrls: [{ match: /\/graphql$/, trackResponseErrors: true }], + }) + const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: jasmine.createSpy() } + const { stop } = trackFetch(lifeCycle, configuration, tracerStub as Tracer) + registerCleanupTask(stop) + + const fetch = window.fetch as MockFetch + fetch(FAKE_GRAPHQL_URL, { + method: 'POST', + body: JSON.stringify({ query: 'query Test { test }' }), + }).resolveWith({ + status: 200, + responseText: JSON.stringify({ + data: null, + errors: [{ message: 'Not found' }, { message: 'Unauthorized' }], + }), + }) + + mockFetchManager.whenAllComplete(() => { + const request = completeSpy.calls.argsFor(0)[0] + expect(request.graphqlErrorsCount).toBe(2) + expect(request.graphqlErrors).toEqual([{ message: 'Not found' }, { message: 'Unauthorized' }]) + done() + }) + }) + + it('should not extract GraphQL errors when trackResponseErrors is false', (done) => { + const mockFetchManager = mockFetch() + const completeSpy = jasmine.createSpy('requestComplete') + const lifeCycle = new LifeCycle() + lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy) + + const configuration = mockRumConfiguration({ + allowedGraphQlUrls: [{ match: /\/graphql$/, trackResponseErrors: false }], + }) + const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: jasmine.createSpy() } + const { stop } = trackFetch(lifeCycle, configuration, tracerStub as Tracer) + registerCleanupTask(stop) + + const fetch = window.fetch as MockFetch + fetch(FAKE_GRAPHQL_URL, { + method: 'POST', + body: JSON.stringify({ query: 'query Test { test }' }), + }).resolveWith({ + status: 200, + responseText: JSON.stringify({ errors: [{ message: 'Error' }] }), + }) + + mockFetchManager.whenAllComplete(() => { + const request = completeSpy.calls.argsFor(0)[0] + expect(request.graphqlErrorsCount).toBeUndefined() + expect(request.graphqlErrors).toBeUndefined() + done() + }) + }) +}) diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index bdfcf9e7db..a694071358 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -24,6 +24,8 @@ import { isAllowedRequestUrl } from './resource/resourceUtils' import type { Tracer } from './tracing/tracer' import { startTracer } from './tracing/tracer' import type { SpanIdentifier, TraceIdentifier } from './tracing/identifier' +import type { GraphQlError } from './resource/graphql' +import { findGraphQlConfiguration, parseGraphQlResponse } from './resource/graphql' export interface CustomContext { requestIndex: number @@ -60,6 +62,8 @@ export interface RequestCompleteEvent { isAborted: boolean handlingStack?: string body?: unknown + graphqlErrorsCount?: number + graphqlErrors?: GraphQlError[] } let nextRequestIndex = 1 @@ -73,7 +77,7 @@ export function startRequestCollection( ) { const tracer = startTracer(configuration, sessionManager, userContext, accountContext) trackXhr(lifeCycle, configuration, tracer) - trackFetch(lifeCycle, tracer) + trackFetch(lifeCycle, configuration, tracer) } export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, tracer: Tracer) { @@ -118,7 +122,7 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, return { stop: () => subscription.unsubscribe() } } -export function trackFetch(lifeCycle: LifeCycle, tracer: Tracer) { +export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration, tracer: Tracer) { const subscription = initFetchObservable().subscribe((rawContext) => { const context = rawContext as RumFetchResolveContext | RumFetchStartContext if (!isAllowedRequestUrl(context.url)) { @@ -136,28 +140,34 @@ export function trackFetch(lifeCycle: LifeCycle, tracer: Tracer) { }) break case 'resolve': - waitForResponseToComplete(context, (duration: Duration) => { - tracer.clearTracingIfNeeded(context) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { - duration, - method: context.method, - requestIndex: context.requestIndex, - responseType: context.responseType, - spanId: context.spanId, - startClocks: context.startClocks, - status: context.status, - traceId: context.traceId, - traceSampled: context.traceSampled, - type: RequestType.FETCH, - url: context.url, - response: context.response, - init: context.init, - input: context.input, - isAborted: context.isAborted, - handlingStack: context.handlingStack, - body: context.init?.body, - }) - }) + waitForResponseToComplete( + context, + configuration, + (duration: Duration, graphqlErrorsCount?: number, graphqlErrors?: GraphQlError[]) => { + tracer.clearTracingIfNeeded(context) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { + duration, + method: context.method, + requestIndex: context.requestIndex, + responseType: context.responseType, + spanId: context.spanId, + startClocks: context.startClocks, + status: context.status, + traceId: context.traceId, + traceSampled: context.traceSampled, + type: RequestType.FETCH, + url: context.url, + response: context.response, + init: context.init, + input: context.input, + isAborted: context.isAborted, + handlingStack: context.handlingStack, + body: context.init?.body, + graphqlErrorsCount, + graphqlErrors, + }) + } + ) break } }) @@ -170,21 +180,37 @@ function getNextRequestIndex() { return result } -function waitForResponseToComplete(context: RumFetchResolveContext, callback: (duration: Duration) => void) { +function waitForResponseToComplete( + context: RumFetchResolveContext, + configuration: RumConfiguration, + callback: (duration: Duration, graphqlErrorsCount?: number, graphqlErrors?: GraphQlError[]) => void +) { const clonedResponse = context.response && tryToClone(context.response) if (!clonedResponse || !clonedResponse.body) { // do not try to wait for the response if the clone failed, fetch error or null body callback(elapsed(context.startClocks.timeStamp, timeStampNow())) - } else { - readBytesFromStream( - clonedResponse.body, - () => { - callback(elapsed(context.startClocks.timeStamp, timeStampNow())) - }, - { - bytesLimit: Number.POSITIVE_INFINITY, - collectStreamBody: false, - } - ) + return } + + const graphQlConfig = findGraphQlConfiguration(context.url, configuration) + const shouldExtractGraphQlErrors = graphQlConfig?.trackResponseErrors + + readBytesFromStream( + clonedResponse.body, + (error?: Error, bytes?: Uint8Array) => { + const duration = elapsed(context.startClocks.timeStamp, timeStampNow()) + + if (shouldExtractGraphQlErrors && !error && bytes) { + parseGraphQlResponse(new TextDecoder().decode(bytes), (errorsCount, errors) => { + callback(duration, errorsCount, errors) + }) + } else { + callback(duration) + } + }, + { + bytesLimit: Number.POSITIVE_INFINITY, + collectStreamBody: shouldExtractGraphQlErrors, + } + ) } diff --git a/packages/rum-core/src/domain/resource/graphql.spec.ts b/packages/rum-core/src/domain/resource/graphql.spec.ts index 06a08edd9b..cddce4aa63 100644 --- a/packages/rum-core/src/domain/resource/graphql.spec.ts +++ b/packages/rum-core/src/domain/resource/graphql.spec.ts @@ -1,5 +1,12 @@ +import { RequestType } from '@datadog/browser-core' import { mockRumConfiguration } from '../../../test' -import { extractGraphQlMetadata, findGraphQlConfiguration } from './graphql' +import type { RequestCompleteEvent } from '../requestCollection' +import { + extractGraphQlMetadata, + extractGraphQlRequestMetadata, + findGraphQlConfiguration, + parseGraphQlResponse, +} from './graphql' describe('GraphQL detection and metadata extraction', () => { describe('findGraphQlConfiguration', () => { @@ -35,7 +42,7 @@ describe('GraphQL detection and metadata extraction', () => { }) }) - describe('extractGraphQlMetadata', () => { + describe('extractGraphQlRequestMetadata', () => { it('should extract query operation type and name', () => { const requestBody = JSON.stringify({ query: 'query GetUser { user { id name } }', @@ -43,7 +50,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: { id: '123' }, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toEqual({ operationType: 'query', @@ -60,7 +67,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: {}, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toEqual({ operationType: 'query', @@ -77,7 +84,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: null, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toEqual({ operationType: 'query', @@ -88,13 +95,13 @@ describe('GraphQL detection and metadata extraction', () => { }) it('should return undefined for invalid JSON', () => { - const result = extractGraphQlMetadata('not valid json', true) + const result = extractGraphQlRequestMetadata('not valid json', true) expect(result).toBeUndefined() }) it('should return undefined for non-GraphQL request body', () => { const requestBody = JSON.stringify({ data: 'some data' }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toBeUndefined() }) @@ -105,7 +112,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: { id: '123' }, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toEqual({ operationType: 'query', @@ -122,7 +129,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: { id: '123' }, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toBeUndefined() }) @@ -133,19 +140,19 @@ describe('GraphQL detection and metadata extraction', () => { variables: { id: '123' }, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result).toBeUndefined() }) }) - describe('payload truncation', () => { + describe('request payload truncation', () => { it('should not truncate payload under 32KB', () => { const shortQuery = 'query GetUser { user { id } }' const requestBody = JSON.stringify({ query: shortQuery, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result?.payload).toBe(shortQuery) }) @@ -156,7 +163,7 @@ describe('GraphQL detection and metadata extraction', () => { query: longQuery, }) - const result = extractGraphQlMetadata(requestBody, true) + const result = extractGraphQlRequestMetadata(requestBody, true) expect(result?.payload?.length).toBe(32768 + 3) expect(result?.payload?.endsWith('...')).toBe(true) @@ -170,7 +177,7 @@ describe('GraphQL detection and metadata extraction', () => { variables: { id: '123' }, }) - const result = extractGraphQlMetadata(requestBody, false) + const result = extractGraphQlRequestMetadata(requestBody, false) expect(result).toEqual({ operationType: 'query', @@ -180,4 +187,129 @@ describe('GraphQL detection and metadata extraction', () => { }) }) }) + + describe('parseGraphQlResponse', () => { + it('should extract error count and detailed errors from GraphQL response', (done) => { + const responseText = JSON.stringify({ + data: null, + errors: [{ message: 'Field not found' }, { message: 'Unauthorized' }], + }) + + parseGraphQlResponse(responseText, (errorsCount, errors) => { + expect(errorsCount).toBe(2) + expect(errors).toEqual([{ message: 'Field not found' }, { message: 'Unauthorized' }]) + done() + }) + }) + + it('should extract detailed errors with extensions, locations and path', (done) => { + const responseText = JSON.stringify({ + data: null, + errors: [ + { + message: 'Field not found', + extensions: { code: 'FIELD_NOT_FOUND' }, + locations: [{ line: 2, column: 5 }], + path: ['user', 'profile'], + }, + ], + }) + + parseGraphQlResponse(responseText, (errorsCount, errors) => { + expect(errorsCount).toBe(1) + expect(errors).toEqual([ + { + message: 'Field not found', + code: 'FIELD_NOT_FOUND', + locations: [{ line: 2, column: 5 }], + path: ['user', 'profile'], + }, + ]) + done() + }) + }) + + it('should handle response without errors', (done) => { + const responseText = JSON.stringify({ + data: { user: { name: 'John' } }, + }) + + parseGraphQlResponse(responseText, (errorsCount, errors) => { + expect(errorsCount).toBeUndefined() + expect(errors).toBeUndefined() + done() + }) + }) + + it('should handle invalid JSON', (done) => { + parseGraphQlResponse('not valid json', (errorsCount, errors) => { + expect(errorsCount).toBeUndefined() + expect(errors).toBeUndefined() + done() + }) + }) + + it('should handle errors with missing extensions', (done) => { + const responseText = JSON.stringify({ + errors: [{ message: 'Simple error' }], + }) + + parseGraphQlResponse(responseText, (errorsCount, errors) => { + expect(errorsCount).toBe(1) + expect(errors).toEqual([{ message: 'Simple error' }]) + done() + }) + }) + }) + + describe('extractGraphQlMetadata', () => { + it('should extract both request and response errors for XHR', () => { + const requestBody = JSON.stringify({ + query: 'query GetUser { user { id } }', + }) + + const responseBody = JSON.stringify({ + errors: [{ message: 'Not found' }], + }) + + const request: RequestCompleteEvent = { + type: RequestType.XHR, + body: requestBody, + xhr: { response: responseBody } as XMLHttpRequest, + } as RequestCompleteEvent + + const result = extractGraphQlMetadata(request, { + match: '/graphql', + trackPayload: false, + trackResponseErrors: true, + }) + + expect(result?.operationType).toBe('query') + expect(result?.errors_count).toBe(1) + expect(result?.errors).toEqual([{ message: 'Not found' }]) + }) + + it('should extract response errors from Fetch request event', () => { + const requestBody = JSON.stringify({ + query: 'query GetUser { user { id } }', + }) + + const request: RequestCompleteEvent = { + type: RequestType.FETCH, + body: requestBody, + graphqlErrorsCount: 2, + graphqlErrors: [{ message: 'Error 1' }, { message: 'Error 2' }], + } as RequestCompleteEvent + + const result = extractGraphQlMetadata(request, { + match: '/graphql', + trackPayload: false, + trackResponseErrors: true, + }) + + expect(result?.operationType).toBe('query') + expect(result?.errors_count).toBe(2) + expect(result?.errors).toEqual([{ message: 'Error 1' }, { message: 'Error 2' }]) + }) + }) }) diff --git a/packages/rum-core/src/domain/resource/graphql.ts b/packages/rum-core/src/domain/resource/graphql.ts index b9336162a7..e36f82d6fb 100644 --- a/packages/rum-core/src/domain/resource/graphql.ts +++ b/packages/rum-core/src/domain/resource/graphql.ts @@ -1,23 +1,109 @@ -import { matchList, ONE_KIBI_BYTE, safeTruncate } from '@datadog/browser-core' +import { matchList, ONE_KIBI_BYTE, safeTruncate, RequestType } from '@datadog/browser-core' import type { RumConfiguration, GraphQlUrlOption } from '../configuration' +import type { RequestCompleteEvent } from '../requestCollection' /** * arbitrary value, byte precision not needed */ const GRAPHQL_PAYLOAD_LIMIT = 32 * ONE_KIBI_BYTE +export interface GraphQlError { + message: string + code?: string + locations?: Array<{ line: number; column: number }> + path?: Array +} + export interface GraphQlMetadata { operationType: 'query' | 'mutation' | 'subscription' operationName?: string variables?: string payload?: string + errors_count?: number + errors?: GraphQlError[] +} + +export function extractGraphQlMetadata( + request: RequestCompleteEvent, + graphQlConfig: GraphQlUrlOption +): GraphQlMetadata | undefined { + const metadata = extractGraphQlRequestMetadata(request.body, graphQlConfig.trackPayload) + if (!metadata) { + return + } + + if (request.type === RequestType.XHR && request.xhr && graphQlConfig.trackResponseErrors) { + extractGraphQlXhrResponseErrors(request.xhr, (errorsCount, errors) => { + if (errorsCount !== undefined) { + metadata.errors_count = errorsCount + } + if (errors !== undefined) { + metadata.errors = errors + } + }) + } else if (request.type === RequestType.FETCH) { + if (request.graphqlErrorsCount !== undefined) { + metadata.errors_count = request.graphqlErrorsCount + } + if (request.graphqlErrors !== undefined) { + metadata.errors = request.graphqlErrors + } + } + + return metadata +} + +export function parseGraphQlResponse( + responseText: string, + callback: (errorsCount?: number, errors?: GraphQlError[]) => void +) { + try { + const response = JSON.parse(responseText) + + if (!response || !Array.isArray(response.errors) || response.errors.length === 0) { + callback() + return + } + + const errors = (response.errors as unknown[]).map((error: unknown) => { + const errorObj = error as Record + const graphqlError: GraphQlError = { + message: typeof errorObj.message === 'string' ? errorObj.message : 'Unknown GraphQL error', + } + + const extensions = errorObj.extensions as Record | undefined + if (extensions?.code && typeof extensions.code === 'string') { + graphqlError.code = extensions.code + } + + if (Array.isArray(errorObj.locations)) { + graphqlError.locations = (errorObj.locations as unknown[]).map((loc: unknown) => { + const locObj = loc as Record + return { + line: typeof locObj.line === 'number' ? locObj.line : 0, + column: typeof locObj.column === 'number' ? locObj.column : 0, + } + }) + } + + if (Array.isArray(errorObj.path)) { + graphqlError.path = errorObj.path as Array + } + + return graphqlError + }) + + callback(errors.length, errors) + } catch { + callback() + } } export function findGraphQlConfiguration(url: string, configuration: RumConfiguration): GraphQlUrlOption | undefined { return configuration.allowedGraphQlUrls.find((graphQlOption) => matchList([graphQlOption.match], url)) } -export function extractGraphQlMetadata( +export function extractGraphQlRequestMetadata( requestBody: unknown, trackPayload: boolean = false ): GraphQlMetadata | undefined { @@ -62,3 +148,14 @@ export function extractGraphQlMetadata( function getOperationType(query: string) { return query.match(/^\s*(query|mutation|subscription)\b/i)?.[1] as 'query' | 'mutation' | 'subscription' | undefined } + +function extractGraphQlXhrResponseErrors( + xhr: XMLHttpRequest, + callback: (errorsCount?: number, errors?: GraphQlError[]) => void +) { + if (typeof xhr.response === 'string') { + parseGraphQlResponse(xhr.response, callback) + } else { + callback() + } +} diff --git a/packages/rum-core/src/domain/resource/resourceCollection.ts b/packages/rum-core/src/domain/resource/resourceCollection.ts index 7c353f0bed..6f41d430ca 100644 --- a/packages/rum-core/src/domain/resource/resourceCollection.ts +++ b/packages/rum-core/src/domain/resource/resourceCollection.ts @@ -39,7 +39,7 @@ import { retrieveInitialDocumentResourceTiming } from './retrieveInitialDocument import type { RequestRegistry } from './requestRegistry' import { createRequestRegistry } from './requestRegistry' import type { GraphQlMetadata } from './graphql' -import { findGraphQlConfiguration, extractGraphQlMetadata } from './graphql' +import { extractGraphQlMetadata, findGraphQlConfiguration } from './graphql' export function startResourceCollection( lifeCycle: LifeCycle, @@ -181,7 +181,7 @@ function computeGraphQlMetaData( return } - return extractGraphQlMetadata(request.body, graphQlConfig.trackPayload) + return extractGraphQlMetadata(request, graphQlConfig) } function getResourceDomainContext( diff --git a/test/e2e/lib/framework/serverApps/mock.ts b/test/e2e/lib/framework/serverApps/mock.ts index 88c684809f..4941acdd7b 100644 --- a/test/e2e/lib/framework/serverApps/mock.ts +++ b/test/e2e/lib/framework/serverApps/mock.ts @@ -97,9 +97,34 @@ export function createMockServerApp( setTimeout(() => res.send('ok'), timeoutDuration) }) - app.post('/graphql', (_req, res) => { + app.post('/graphql', (req, res) => { res.header('Content-Type', 'application/json') - res.json({ data: { result: 'success' } }) + + const scenario = req.query.scenario as string | undefined + + if (scenario === 'validation-error') { + res.json({ + data: null, + errors: [ + { + message: 'Field "unknownField" does not exist', + extensions: { code: 'GRAPHQL_VALIDATION_FAILED' }, + locations: [{ line: 2, column: 5 }], + path: ['user', 'unknownField'], + }, + ], + }) + } else if (scenario === 'multiple-errors') { + res.json({ + data: { user: null }, + errors: [ + { message: 'User not found' }, + { message: 'Insufficient permissions', extensions: { code: 'UNAUTHORIZED' } }, + ], + }) + } else { + res.json({ data: { result: 'success' } }) + } }) app.get('/redirect', (req, res) => { diff --git a/test/e2e/scenario/rum/graphql.scenario.ts b/test/e2e/scenario/rum/graphql.scenario.ts index fe9eecabb2..c092e15c54 100644 --- a/test/e2e/scenario/rum/graphql.scenario.ts +++ b/test/e2e/scenario/rum/graphql.scenario.ts @@ -1,9 +1,15 @@ import { test, expect } from '@playwright/test' import { createTest } from '../../lib/framework' -function buildGraphQlConfig({ trackPayload = false }: { trackPayload?: boolean } = {}) { +function buildGraphQlConfig({ + trackPayload = false, + trackResponseErrors = false, +}: { + trackPayload?: boolean + trackResponseErrors?: boolean +} = {}) { return { - allowedGraphQlUrls: [{ match: (url: string) => url.includes('graphql'), trackPayload }], + allowedGraphQlUrls: [{ match: (url: string) => url.includes('graphql'), trackPayload, trackResponseErrors }], } } @@ -78,4 +84,82 @@ test.describe('GraphQL tracking', () => { expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.graphql).toBeUndefined() }) + + createTest('track GraphQL response errors via fetch') + .withRum(buildGraphQlConfig({ trackPayload: false, trackResponseErrors: true })) + .run(async ({ intakeRegistry, flushEvents, page }) => { + await page.evaluate(() => + window.fetch('/graphql?scenario=validation-error', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + query: 'query GetUser { user { unknownField } }', + operationName: 'GetUser', + }), + }) + ) + + await flushEvents() + const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + expect(resourceEvent).toBeDefined() + expect(resourceEvent.resource.graphql).toEqual({ + operationType: 'query', + operationName: 'GetUser', + variables: undefined, + payload: undefined, + errors_count: 1, + errors: [ + { + message: 'Field "unknownField" does not exist', + code: 'GRAPHQL_VALIDATION_FAILED', + locations: [{ line: 2, column: 5 }], + path: ['user', 'unknownField'], + }, + ], + }) + }) + + createTest('track GraphQL response with multiple errors via XHR') + .withRum(buildGraphQlConfig({ trackResponseErrors: true })) + .run(async ({ intakeRegistry, flushEvents, page }) => { + await page.evaluate(() => { + const xhr = new XMLHttpRequest() + xhr.open('POST', '/graphql?scenario=multiple-errors') + xhr.setRequestHeader('Content-Type', 'application/json') + xhr.send( + JSON.stringify({ + query: 'query GetUser { user { name } }', + }) + ) + }) + + await flushEvents() + const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + expect(resourceEvent).toBeDefined() + expect(resourceEvent.resource.graphql?.errors_count).toBe(2) + expect(resourceEvent.resource.graphql?.errors).toEqual([ + { message: 'User not found' }, + { message: 'Insufficient permissions', code: 'UNAUTHORIZED' }, + ]) + }) + + createTest('should not track response errors when trackResponseErrors is false') + .withRum(buildGraphQlConfig({ trackPayload: true, trackResponseErrors: false })) + .run(async ({ intakeRegistry, flushEvents, page }) => { + await page.evaluate(() => + window.fetch('/graphql?scenario=validation-error', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + query: 'query Test { test }', + }), + }) + ) + + await flushEvents() + const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + expect(resourceEvent).toBeDefined() + expect(resourceEvent.resource.graphql?.errors_count).toBeUndefined() + expect(resourceEvent.resource.graphql?.errors).toBeUndefined() + }) }) From a3a003923f32ca3cf4713bea6afaacaaf4efcb5e Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 21 Oct 2025 16:43:16 +0200 Subject: [PATCH 02/13] format update --- .../domain/telemetry/telemetryEvent.types.ts | 4 ++ packages/rum-core/src/rumEvent.types.ts | 56 +++++++++++++++++-- rum-events-format | 2 +- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index 3aacc38c48..2800087371 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -217,6 +217,10 @@ export type TelemetryConfigurationEvent = CommonTelemetryProperties & { * Whether GraphQL payload tracking is used for at least one GraphQL endpoint */ use_track_graph_ql_payload?: boolean + /** + * Whether GraphQL response errors tracking is used for at least one GraphQL endpoint + */ + use_track_graph_ql_response_errors?: boolean /** * A list of selected tracing propagators */ diff --git a/packages/rum-core/src/rumEvent.types.ts b/packages/rum-core/src/rumEvent.types.ts index 987305a25f..b95ba22551 100644 --- a/packages/rum-core/src/rumEvent.types.ts +++ b/packages/rum-core/src/rumEvent.types.ts @@ -104,7 +104,7 @@ export type RumActionEvent = CommonProperties & /** * View properties */ - readonly view?: { + readonly view: { /** * Is the action starting in the foreground (focus in browser) */ @@ -479,7 +479,7 @@ export type RumErrorEvent = CommonProperties & /** * View properties */ - readonly view?: { + readonly view: { /** * Is the error starting in the foreground (focus in browser) */ @@ -529,15 +529,15 @@ export type RumLongTaskEvent = CommonProperties & */ readonly blocking_duration?: number /** - * Start time of the rendering cycle, which includes requestAnimationFrame callbacks, style and layout calculation, resize observer and intersection observer callbacks + * Time difference (in ns) between the timeOrigin and the start time of the rendering cycle, which includes requestAnimationFrame callbacks, style and layout calculation, resize observer and intersection observer callbacks */ readonly render_start?: number /** - * Start time of the time period spent in style and layout calculations + * Time difference (in ns) between the timeOrigin and the start time of the time period spent in style and layout calculations */ readonly style_and_layout_start?: number /** - * Start time of of the first UI event (mouse/keyboard and so on) to be handled during the course of this frame + * Time difference (in ns) between the timeOrigin and the start time of of the first UI event (mouse/keyboard and so on) to be handled during the course of this frame */ readonly first_ui_event_timestamp?: number /** @@ -845,6 +845,42 @@ export type RumResourceEvent = CommonProperties & * String representation of the operation variables */ variables?: string + /** + * Number of GraphQL errors in the response + */ + readonly error_count?: number + /** + * Array of GraphQL errors from the response + */ + readonly errors?: { + /** + * Error message + */ + readonly message: string + /** + * Error code (used by some providers) + */ + readonly code?: string + /** + * Array of error locations in the GraphQL query + */ + readonly locations?: { + /** + * Line number where the error occurred + */ + readonly line: number + /** + * Column number where the error occurred + */ + readonly column: number + [k: string]: unknown + }[] + /** + * Path to the field that caused the error + */ + readonly path?: (string | number)[] + [k: string]: unknown + }[] [k: string]: unknown } [k: string]: unknown @@ -1303,6 +1339,10 @@ export type RumVitalEvent = CommonProperties & readonly computed_value?: boolean [k: string]: unknown } + /** + * Profiling context + */ + profiling?: ProfilingInternalContextSchema [k: string]: unknown } [k: string]: unknown @@ -1467,7 +1507,7 @@ export interface CommonProperties { /** * View properties */ - readonly view: { + readonly view?: { /** * UUID of the view */ @@ -1876,6 +1916,10 @@ export interface StreamSchema { * how much did the media progress since the last context update (in ms) */ watch_time?: number + /** + * Percentage of amount of time watched relative to its total duration + */ + completion_percent?: number [k: string]: unknown } [k: string]: unknown diff --git a/rum-events-format b/rum-events-format index fbf83da4ab..14ab2d9eba 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit fbf83da4ab8e7b6ad956aa0038d6e9fef3ec0a9a +Subproject commit 14ab2d9eba0140976da009fd124f675632855ca2 From bdab4c3f28f5591d9f55f3fc2c7c174376b85b69 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 22 Oct 2025 17:30:09 +0200 Subject: [PATCH 03/13] update rum-events-format --- rum-events-format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rum-events-format b/rum-events-format index 14ab2d9eba..fbf83da4ab 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit 14ab2d9eba0140976da009fd124f675632855ca2 +Subproject commit fbf83da4ab8e7b6ad956aa0038d6e9fef3ec0a9a From 0450dd80d7c7d478613b4eb55f0c946f591b213a Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 22 Oct 2025 22:27:52 +0200 Subject: [PATCH 04/13] cherry pick commit so view is not optional anymore --- packages/rum-core/src/rumEvent.types.ts | 2 +- rum-events-format | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/rumEvent.types.ts b/packages/rum-core/src/rumEvent.types.ts index b95ba22551..6ad569f760 100644 --- a/packages/rum-core/src/rumEvent.types.ts +++ b/packages/rum-core/src/rumEvent.types.ts @@ -1507,7 +1507,7 @@ export interface CommonProperties { /** * View properties */ - readonly view?: { + readonly view: { /** * UUID of the view */ diff --git a/rum-events-format b/rum-events-format index fbf83da4ab..999c83da36 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit fbf83da4ab8e7b6ad956aa0038d6e9fef3ec0a9a +Subproject commit 999c83da362c598864e439fbcb2ba7af86abdfb8 From 1306118d0b50ca3c7ce00e2338c57fe74010f248 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Oct 2025 11:52:20 +0100 Subject: [PATCH 05/13] =?UTF-8?q?=F0=9F=A5=9C=20nitpicks=20and=20format?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/telemetry/telemetryEvent.types.ts | 2 +- .../configuration/configuration.spec.ts | 2 +- packages/rum-core/src/rumEvent.types.ts | 193 ++++++++++-------- rum-events-format | 2 +- test/e2e/scenario/rum/graphql.scenario.ts | 12 +- 5 files changed, 117 insertions(+), 94 deletions(-) diff --git a/packages/core/src/domain/telemetry/telemetryEvent.types.ts b/packages/core/src/domain/telemetry/telemetryEvent.types.ts index 2800087371..d7e9802e4c 100644 --- a/packages/core/src/domain/telemetry/telemetryEvent.types.ts +++ b/packages/core/src/domain/telemetry/telemetryEvent.types.ts @@ -845,7 +845,7 @@ export interface AddOperationStepVital { */ feature: 'add-operation-step-vital' /** - * Feature operations action type + * Operations step type */ action_type: 'start' | 'succeed' | 'fail' [k: string]: unknown diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index e476f5a736..06e5fdb973 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -567,7 +567,7 @@ describe('validateAndBuildRumConfiguration', () => { allowedGraphQlUrls: [{ match: '/graphql', trackResponseErrors: true }], })! expect(configuration.allowedGraphQlUrls).toEqual([ - { match: '/graphql', trackResponseErrors: true, trackPayload: false }, + { match: '/graphql', trackPayload: false, trackResponseErrors: true }, ]) }) diff --git a/packages/rum-core/src/rumEvent.types.ts b/packages/rum-core/src/rumEvent.types.ts index 6ad569f760..ba32b3aa8d 100644 --- a/packages/rum-core/src/rumEvent.types.ts +++ b/packages/rum-core/src/rumEvent.types.ts @@ -175,6 +175,9 @@ export type RumTransitionEvent = CommonProperties & { * RUM event type */ readonly type: 'transition' + view: { + [k: string]: unknown + } /** * Stream properties */ @@ -504,6 +507,9 @@ export type RumLongTaskEvent = CommonProperties & * RUM event type */ readonly type: 'long_task' + view: { + [k: string]: unknown + } /** * Long Task properties */ @@ -628,6 +634,9 @@ export type RumResourceEvent = CommonProperties & * RUM event type */ readonly type: 'resource' + view: { + [k: string]: unknown + } /** * Resource properties */ @@ -1315,120 +1324,134 @@ export type RumViewEvent = CommonProperties & } [k: string]: unknown } +export type RumVitalEvent = RumVitalDurationEvent | RumVitalOperationStepEvent | RumVitalAppLaunchEvent /** - * Schema of all properties of a Vital event + * Schema for a duration vital event. */ -export type RumVitalEvent = CommonProperties & +export type RumVitalDurationEvent = RumVitalEventCommonProperties & { + view: { + [k: string]: unknown + } + /** + * Vital properties + */ + readonly vital: { + /** + * Type of the vital. + */ + readonly type: 'duration' + /** + * Duration of the vital in nanoseconds. + */ + readonly duration: number + [k: string]: unknown + } + [k: string]: unknown +} +/** + * Schema of common properties for a Vital event + */ +export type RumVitalEventCommonProperties = CommonProperties & ViewContainerSchema & { /** * RUM event type */ readonly type: 'vital' - readonly vital: DurationProperties | AppLaunchProperties | FeatureOperationProperties /** - * Internal properties + * Vital properties */ - readonly _dd?: { + readonly vital: { /** - * Internal vital properties + * UUID of the vital */ - readonly vital?: { - /** - * Whether the value of the vital is computed by the SDK (as opposed to directly provided by the customer) - */ - readonly computed_value?: boolean - [k: string]: unknown - } + readonly id: string /** - * Profiling context + * Name of the vital, as it is also used as facet path for its value, it must contain only letters, digits, or the characters - _ . @ $ */ - profiling?: ProfilingInternalContextSchema + readonly name?: string + /** + * Description of the vital. It can be used as a secondary identifier (URL, React component name...) + */ + readonly description?: string [k: string]: unknown } [k: string]: unknown } /** - * Duration properties of a Vital event - */ -export type DurationProperties = VitalCommonProperties & { - /** - * Type of the vital. - */ - readonly type: 'duration' - /** - * Duration of the vital in nanoseconds. - */ - readonly duration: number - [k: string]: unknown -} -/** - * Schema of common properties for a Vital event + * Schema for a vital operation step event. */ -export type VitalCommonProperties = { - /** - * UUID of the vital - */ - readonly id: string - /** - * Name of the vital, as it is also used as facet path for its value, it must contain only letters, digits, or the characters - _ . @ $ - */ - readonly name?: string +export type RumVitalOperationStepEvent = RumVitalEventCommonProperties & { + view: { + [k: string]: unknown + } /** - * Description of the vital. It can be used as a secondary identifier (URL, React component name...) + * Vital properties */ - readonly description?: string + readonly vital: { + /** + * Type of the vital. + */ + readonly type: 'operation_step' + /** + * Optional key to distinguish between multiple operations of the same name running in parallel (e.g., 'photo_upload' with keys 'profile_pic' vs 'cover') + */ + readonly operation_key?: string + /** + * Type of the step that triggered the vital, if applicable + */ + readonly step_type: 'start' | 'update' | 'retry' | 'end' + /** + * Reason for the failure of the step, if applicable + */ + readonly failure_reason?: 'error' | 'abandoned' | 'other' + [k: string]: unknown + } [k: string]: unknown } /** * Schema for app launch metrics. */ -export type AppLaunchProperties = VitalCommonProperties & { - /** - * Type of the vital. - */ - readonly type: 'app_launch' - /** - * The metric of the app launch. - */ - readonly app_launch_metric: 'ttid' | 'ttfd' - /** - * Duration of the vital in nanoseconds. - */ - readonly duration: number - /** - * The type of the app launch. - */ - readonly startup_type?: 'cold_start' | 'warm_start' - /** - * Whether the app launch was prewarmed. - */ - readonly is_prewarmed?: boolean - /** - * If the app launch had a saved instance state bundle. - */ - readonly has_saved_instance_state_bundle?: boolean - [k: string]: unknown -} -/** - * Schema for a feature operation. - */ -export type FeatureOperationProperties = VitalCommonProperties & { +export type RumVitalAppLaunchEvent = RumVitalEventCommonProperties & { /** - * Type of the vital. + * Vital properties */ - readonly type: 'operation_step' - /** - * Optional key to distinguish between multiple operations of the same name running in parallel (e.g., 'photo_upload' with keys 'profile_pic' vs 'cover') - */ - readonly operation_key?: string - /** - * Type of the step that triggered the vital, if applicable - */ - readonly step_type?: 'start' | 'update' | 'retry' | 'end' + readonly vital: { + /** + * Type of the vital. + */ + readonly type: 'app_launch' + /** + * The metric of the app launch. + */ + readonly app_launch_metric: 'ttid' | 'ttfd' + /** + * Duration of the vital in nanoseconds. + */ + readonly duration: number + /** + * The type of the app launch. + */ + readonly startup_type?: 'cold_start' | 'warm_start' + /** + * Whether the app launch was prewarmed. + */ + readonly is_prewarmed?: boolean + /** + * If the app launch had a saved instance state bundle. + */ + readonly has_saved_instance_state_bundle?: boolean + [k: string]: unknown + } /** - * Reason for the failure of the step, if applicable + * Internal properties */ - readonly failure_reason?: 'error' | 'abandoned' | 'other' + readonly _dd?: { + /** + * Profiling context + */ + profiling?: ProfilingInternalContextSchema + [k: string]: unknown + } [k: string]: unknown } @@ -1507,7 +1530,7 @@ export interface CommonProperties { /** * View properties */ - readonly view: { + readonly view?: { /** * UUID of the view */ diff --git a/rum-events-format b/rum-events-format index 999c83da36..fe242fe9a0 160000 --- a/rum-events-format +++ b/rum-events-format @@ -1 +1 @@ -Subproject commit 999c83da362c598864e439fbcb2ba7af86abdfb8 +Subproject commit fe242fe9a02cc373e61127d7a2ef629991a5c28f diff --git a/test/e2e/scenario/rum/graphql.scenario.ts b/test/e2e/scenario/rum/graphql.scenario.ts index c092e15c54..bdfcc73820 100644 --- a/test/e2e/scenario/rum/graphql.scenario.ts +++ b/test/e2e/scenario/rum/graphql.scenario.ts @@ -31,7 +31,7 @@ test.describe('GraphQL tracking', () => { }) await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/graphql'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.method).toBe('POST') expect(resourceEvent.resource.graphql).toEqual({ @@ -58,7 +58,7 @@ test.describe('GraphQL tracking', () => { ) await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/graphql'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.method).toBe('POST') expect(resourceEvent.resource.graphql).toEqual({ @@ -80,7 +80,7 @@ test.describe('GraphQL tracking', () => { await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/ok'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/ok'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.graphql).toBeUndefined() }) @@ -100,7 +100,7 @@ test.describe('GraphQL tracking', () => { ) await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/graphql'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.graphql).toEqual({ operationType: 'query', @@ -134,7 +134,7 @@ test.describe('GraphQL tracking', () => { }) await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/graphql'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.graphql?.errors_count).toBe(2) expect(resourceEvent.resource.graphql?.errors).toEqual([ @@ -157,7 +157,7 @@ test.describe('GraphQL tracking', () => { ) await flushEvents() - const resourceEvent = intakeRegistry.rumResourceEvents.find((r) => r.resource.url.includes('/graphql'))! + const resourceEvent = intakeRegistry.rumResourceEvents.find((event) => event.resource.url.includes('/graphql'))! expect(resourceEvent).toBeDefined() expect(resourceEvent.resource.graphql?.errors_count).toBeUndefined() expect(resourceEvent.resource.graphql?.errors).toBeUndefined() From ef3b690afb1302fc54bd92a3c32bf7d37be53a3e Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Oct 2025 12:56:47 +0100 Subject: [PATCH 06/13] =?UTF-8?q?=F0=9F=91=8C=20Helper=20function=20for=20?= =?UTF-8?q?request=20collection=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/requestCollection.spec.ts | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts index ac5db29011..b6c65994a2 100644 --- a/packages/rum-core/src/domain/requestCollection.spec.ts +++ b/packages/rum-core/src/domain/requestCollection.spec.ts @@ -339,20 +339,25 @@ describe('collect xhr', () => { describe('GraphQL response errors tracking', () => { const FAKE_GRAPHQL_URL = 'http://fake-url/graphql' - it('should extract GraphQL errors when trackResponseErrors is enabled', (done) => { + function setupGraphQlFetchTest(trackResponseErrors: boolean) { const mockFetchManager = mockFetch() const completeSpy = jasmine.createSpy('requestComplete') const lifeCycle = new LifeCycle() lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy) const configuration = mockRumConfiguration({ - allowedGraphQlUrls: [{ match: /\/graphql$/, trackResponseErrors: true }], + allowedGraphQlUrls: [{ match: /\/graphql$/, trackResponseErrors }], }) const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: jasmine.createSpy() } const { stop } = trackFetch(lifeCycle, configuration, tracerStub as Tracer) registerCleanupTask(stop) - const fetch = window.fetch as MockFetch + return { mockFetchManager, completeSpy, fetch: window.fetch as MockFetch } + } + + it('should extract GraphQL errors when trackResponseErrors is enabled', (done) => { + const { mockFetchManager, completeSpy, fetch } = setupGraphQlFetchTest(true) + fetch(FAKE_GRAPHQL_URL, { method: 'POST', body: JSON.stringify({ query: 'query Test { test }' }), @@ -373,19 +378,8 @@ describe('GraphQL response errors tracking', () => { }) it('should not extract GraphQL errors when trackResponseErrors is false', (done) => { - const mockFetchManager = mockFetch() - const completeSpy = jasmine.createSpy('requestComplete') - const lifeCycle = new LifeCycle() - lifeCycle.subscribe(LifeCycleEventType.REQUEST_COMPLETED, completeSpy) - - const configuration = mockRumConfiguration({ - allowedGraphQlUrls: [{ match: /\/graphql$/, trackResponseErrors: false }], - }) - const tracerStub: Partial = { clearTracingIfNeeded, traceFetch: jasmine.createSpy() } - const { stop } = trackFetch(lifeCycle, configuration, tracerStub as Tracer) - registerCleanupTask(stop) + const { mockFetchManager, completeSpy, fetch } = setupGraphQlFetchTest(false) - const fetch = window.fetch as MockFetch fetch(FAKE_GRAPHQL_URL, { method: 'POST', body: JSON.stringify({ query: 'query Test { test }' }), From a8fdb57fe71eb1a8d628e0c952bf5c3234738bf3 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Oct 2025 12:57:45 +0100 Subject: [PATCH 07/13] =?UTF-8?q?=F0=9F=91=8C=20Use=20a=20mutable=20contex?= =?UTF-8?q?t=20and=20update=20relative=20test=20+=20move=20xhr=20extractio?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum-core/src/domain/requestCollection.ts | 95 +++++++++++-------- .../src/domain/resource/graphql.spec.ts | 34 +++++-- .../rum-core/src/domain/resource/graphql.ts | 34 ++----- 3 files changed, 88 insertions(+), 75 deletions(-) diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index a694071358..5fbce2cd9f 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -34,9 +34,16 @@ export interface CustomContext { traceSampled?: boolean } export interface RumFetchStartContext extends FetchStartContext, CustomContext {} -export interface RumFetchResolveContext extends FetchResolveContext, CustomContext {} +export interface RumFetchResolveContext extends FetchResolveContext, CustomContext { + duration?: Duration + graphqlErrorsCount?: number + graphqlErrors?: GraphQlError[] +} export interface RumXhrStartContext extends XhrStartContext, CustomContext {} -export interface RumXhrCompleteContext extends XhrCompleteContext, CustomContext {} +export interface RumXhrCompleteContext extends XhrCompleteContext, CustomContext { + graphqlErrorsCount?: number + graphqlErrors?: GraphQlError[] +} export interface RequestStartEvent { requestIndex: number @@ -98,6 +105,7 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, }) break case 'complete': + extractGraphQlErrorsFromXhr(context, configuration) tracer.clearTracingIfNeeded(context) lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { duration: context.duration, @@ -114,6 +122,8 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, isAborted: context.isAborted, handlingStack: context.handlingStack, body: context.body, + graphqlErrorsCount: context.graphqlErrorsCount, + graphqlErrors: context.graphqlErrors, }) break } @@ -140,34 +150,30 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration }) break case 'resolve': - waitForResponseToComplete( - context, - configuration, - (duration: Duration, graphqlErrorsCount?: number, graphqlErrors?: GraphQlError[]) => { - tracer.clearTracingIfNeeded(context) - lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { - duration, - method: context.method, - requestIndex: context.requestIndex, - responseType: context.responseType, - spanId: context.spanId, - startClocks: context.startClocks, - status: context.status, - traceId: context.traceId, - traceSampled: context.traceSampled, - type: RequestType.FETCH, - url: context.url, - response: context.response, - init: context.init, - input: context.input, - isAborted: context.isAborted, - handlingStack: context.handlingStack, - body: context.init?.body, - graphqlErrorsCount, - graphqlErrors, - }) - } - ) + waitForFetchResponseAndExtractGraphQlErrors(context, configuration, () => { + tracer.clearTracingIfNeeded(context) + lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { + duration: context.duration!, + method: context.method, + requestIndex: context.requestIndex, + responseType: context.responseType, + spanId: context.spanId, + startClocks: context.startClocks, + status: context.status, + traceId: context.traceId, + traceSampled: context.traceSampled, + type: RequestType.FETCH, + url: context.url, + response: context.response, + init: context.init, + input: context.input, + isAborted: context.isAborted, + handlingStack: context.handlingStack, + body: context.init?.body, + graphqlErrorsCount: context.graphqlErrorsCount, + graphqlErrors: context.graphqlErrors, + }) + }) break } }) @@ -180,15 +186,28 @@ function getNextRequestIndex() { return result } -function waitForResponseToComplete( +function extractGraphQlErrorsFromXhr(context: RumXhrCompleteContext, configuration: RumConfiguration) { + const graphQlConfig = findGraphQlConfiguration(context.url, configuration) + if (!graphQlConfig?.trackResponseErrors || !context.xhr || typeof context.xhr.response !== 'string') { + return + } + + parseGraphQlResponse(context.xhr.response, (graphqlErrorsCount, graphqlErrors) => { + context.graphqlErrorsCount = graphqlErrorsCount + context.graphqlErrors = graphqlErrors + }) +} + +function waitForFetchResponseAndExtractGraphQlErrors( context: RumFetchResolveContext, configuration: RumConfiguration, - callback: (duration: Duration, graphqlErrorsCount?: number, graphqlErrors?: GraphQlError[]) => void + onComplete: () => void ) { const clonedResponse = context.response && tryToClone(context.response) if (!clonedResponse || !clonedResponse.body) { // do not try to wait for the response if the clone failed, fetch error or null body - callback(elapsed(context.startClocks.timeStamp, timeStampNow())) + context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) + onComplete() return } @@ -198,14 +217,16 @@ function waitForResponseToComplete( readBytesFromStream( clonedResponse.body, (error?: Error, bytes?: Uint8Array) => { - const duration = elapsed(context.startClocks.timeStamp, timeStampNow()) + context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) if (shouldExtractGraphQlErrors && !error && bytes) { - parseGraphQlResponse(new TextDecoder().decode(bytes), (errorsCount, errors) => { - callback(duration, errorsCount, errors) + parseGraphQlResponse(new TextDecoder().decode(bytes), (graphqlErrorsCount, graphqlErrors) => { + context.graphqlErrorsCount = graphqlErrorsCount + context.graphqlErrors = graphqlErrors + onComplete() }) } else { - callback(duration) + onComplete() } }, { diff --git a/packages/rum-core/src/domain/resource/graphql.spec.ts b/packages/rum-core/src/domain/resource/graphql.spec.ts index cddce4aa63..3efbe0bc80 100644 --- a/packages/rum-core/src/domain/resource/graphql.spec.ts +++ b/packages/rum-core/src/domain/resource/graphql.spec.ts @@ -1,4 +1,3 @@ -import { RequestType } from '@datadog/browser-core' import { mockRumConfiguration } from '../../../test' import type { RequestCompleteEvent } from '../requestCollection' import { @@ -263,19 +262,15 @@ describe('GraphQL detection and metadata extraction', () => { }) describe('extractGraphQlMetadata', () => { - it('should extract both request and response errors for XHR', () => { + it('should extract request metadata and response errors from RequestCompleteEvent', () => { const requestBody = JSON.stringify({ query: 'query GetUser { user { id } }', }) - const responseBody = JSON.stringify({ - errors: [{ message: 'Not found' }], - }) - const request: RequestCompleteEvent = { - type: RequestType.XHR, body: requestBody, - xhr: { response: responseBody } as XMLHttpRequest, + graphqlErrorsCount: 1, + graphqlErrors: [{ message: 'Not found' }], } as RequestCompleteEvent const result = extractGraphQlMetadata(request, { @@ -289,13 +284,32 @@ describe('GraphQL detection and metadata extraction', () => { expect(result?.errors).toEqual([{ message: 'Not found' }]) }) - it('should extract response errors from Fetch request event', () => { + it('should extract request metadata without response errors when not provided', () => { + const requestBody = JSON.stringify({ + query: 'query GetUser { user { id } }', + }) + + const request: RequestCompleteEvent = { + body: requestBody, + } as RequestCompleteEvent + + const result = extractGraphQlMetadata(request, { + match: '/graphql', + trackPayload: false, + trackResponseErrors: true, + }) + + expect(result?.operationType).toBe('query') + expect(result?.errors_count).toBeUndefined() + expect(result?.errors).toBeUndefined() + }) + + it('should handle multiple errors from RequestCompleteEvent', () => { const requestBody = JSON.stringify({ query: 'query GetUser { user { id } }', }) const request: RequestCompleteEvent = { - type: RequestType.FETCH, body: requestBody, graphqlErrorsCount: 2, graphqlErrors: [{ message: 'Error 1' }, { message: 'Error 2' }], diff --git a/packages/rum-core/src/domain/resource/graphql.ts b/packages/rum-core/src/domain/resource/graphql.ts index e36f82d6fb..53f0650dc0 100644 --- a/packages/rum-core/src/domain/resource/graphql.ts +++ b/packages/rum-core/src/domain/resource/graphql.ts @@ -1,4 +1,4 @@ -import { matchList, ONE_KIBI_BYTE, safeTruncate, RequestType } from '@datadog/browser-core' +import { matchList, ONE_KIBI_BYTE, safeTruncate } from '@datadog/browser-core' import type { RumConfiguration, GraphQlUrlOption } from '../configuration' import type { RequestCompleteEvent } from '../requestCollection' @@ -32,22 +32,11 @@ export function extractGraphQlMetadata( return } - if (request.type === RequestType.XHR && request.xhr && graphQlConfig.trackResponseErrors) { - extractGraphQlXhrResponseErrors(request.xhr, (errorsCount, errors) => { - if (errorsCount !== undefined) { - metadata.errors_count = errorsCount - } - if (errors !== undefined) { - metadata.errors = errors - } - }) - } else if (request.type === RequestType.FETCH) { - if (request.graphqlErrorsCount !== undefined) { - metadata.errors_count = request.graphqlErrorsCount - } - if (request.graphqlErrors !== undefined) { - metadata.errors = request.graphqlErrors - } + if (request.graphqlErrorsCount !== undefined) { + metadata.errors_count = request.graphqlErrorsCount + } + if (request.graphqlErrors !== undefined) { + metadata.errors = request.graphqlErrors } return metadata @@ -148,14 +137,3 @@ export function extractGraphQlRequestMetadata( function getOperationType(query: string) { return query.match(/^\s*(query|mutation|subscription)\b/i)?.[1] as 'query' | 'mutation' | 'subscription' | undefined } - -function extractGraphQlXhrResponseErrors( - xhr: XMLHttpRequest, - callback: (errorsCount?: number, errors?: GraphQlError[]) => void -) { - if (typeof xhr.response === 'string') { - parseGraphQlResponse(xhr.response, callback) - } else { - callback() - } -} From 044902cf0503be1d032c0664997a2adefdaedade Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 02:59:04 +0100 Subject: [PATCH 08/13] =?UTF-8?q?=F0=9F=91=8C=20Synchronous=20function=20/?= =?UTF-8?q?=20Cast=20simplification=20/=20Error=20Handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum-core/src/domain/requestCollection.ts | 23 ++--- .../src/domain/resource/graphql.spec.ts | 55 ++++++------ .../rum-core/src/domain/resource/graphql.ts | 85 ++++++++++--------- 3 files changed, 84 insertions(+), 79 deletions(-) diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index 5fbce2cd9f..ce73ca5188 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -192,10 +192,11 @@ function extractGraphQlErrorsFromXhr(context: RumXhrCompleteContext, configurati return } - parseGraphQlResponse(context.xhr.response, (graphqlErrorsCount, graphqlErrors) => { - context.graphqlErrorsCount = graphqlErrorsCount - context.graphqlErrors = graphqlErrors - }) + const result = parseGraphQlResponse(context.xhr.response) + if (result) { + context.graphqlErrorsCount = result.errorsCount + context.graphqlErrors = result.errors + } } function waitForFetchResponseAndExtractGraphQlErrors( @@ -220,14 +221,14 @@ function waitForFetchResponseAndExtractGraphQlErrors( context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) if (shouldExtractGraphQlErrors && !error && bytes) { - parseGraphQlResponse(new TextDecoder().decode(bytes), (graphqlErrorsCount, graphqlErrors) => { - context.graphqlErrorsCount = graphqlErrorsCount - context.graphqlErrors = graphqlErrors - onComplete() - }) - } else { - onComplete() + const result = parseGraphQlResponse(new TextDecoder().decode(bytes)) + if (result) { + context.graphqlErrorsCount = result.errorsCount + context.graphqlErrors = result.errors + } } + + onComplete() }, { bytesLimit: Number.POSITIVE_INFINITY, diff --git a/packages/rum-core/src/domain/resource/graphql.spec.ts b/packages/rum-core/src/domain/resource/graphql.spec.ts index 3efbe0bc80..bbe562c6ea 100644 --- a/packages/rum-core/src/domain/resource/graphql.spec.ts +++ b/packages/rum-core/src/domain/resource/graphql.spec.ts @@ -188,20 +188,21 @@ describe('GraphQL detection and metadata extraction', () => { }) describe('parseGraphQlResponse', () => { - it('should extract error count and detailed errors from GraphQL response', (done) => { + it('should extract error count and detailed errors from GraphQL response', () => { const responseText = JSON.stringify({ data: null, errors: [{ message: 'Field not found' }, { message: 'Unauthorized' }], }) - parseGraphQlResponse(responseText, (errorsCount, errors) => { - expect(errorsCount).toBe(2) - expect(errors).toEqual([{ message: 'Field not found' }, { message: 'Unauthorized' }]) - done() + const result = parseGraphQlResponse(responseText) + + expect(result).toEqual({ + errorsCount: 2, + errors: [{ message: 'Field not found' }, { message: 'Unauthorized' }], }) }) - it('should extract detailed errors with extensions, locations and path', (done) => { + it('should extract detailed errors with extensions, locations and path', () => { const responseText = JSON.stringify({ data: null, errors: [ @@ -214,49 +215,47 @@ describe('GraphQL detection and metadata extraction', () => { ], }) - parseGraphQlResponse(responseText, (errorsCount, errors) => { - expect(errorsCount).toBe(1) - expect(errors).toEqual([ + const result = parseGraphQlResponse(responseText) + + expect(result).toEqual({ + errorsCount: 1, + errors: [ { message: 'Field not found', code: 'FIELD_NOT_FOUND', locations: [{ line: 2, column: 5 }], path: ['user', 'profile'], }, - ]) - done() + ], }) }) - it('should handle response without errors', (done) => { + it('should handle response without errors', () => { const responseText = JSON.stringify({ data: { user: { name: 'John' } }, }) - parseGraphQlResponse(responseText, (errorsCount, errors) => { - expect(errorsCount).toBeUndefined() - expect(errors).toBeUndefined() - done() - }) + const result = parseGraphQlResponse(responseText) + + expect(result).toBeUndefined() }) - it('should handle invalid JSON', (done) => { - parseGraphQlResponse('not valid json', (errorsCount, errors) => { - expect(errorsCount).toBeUndefined() - expect(errors).toBeUndefined() - done() - }) + it('should handle invalid JSON', () => { + const result = parseGraphQlResponse('not valid json') + + expect(result).toBeUndefined() }) - it('should handle errors with missing extensions', (done) => { + it('should handle errors with missing extensions', () => { const responseText = JSON.stringify({ errors: [{ message: 'Simple error' }], }) - parseGraphQlResponse(responseText, (errorsCount, errors) => { - expect(errorsCount).toBe(1) - expect(errors).toEqual([{ message: 'Simple error' }]) - done() + const result = parseGraphQlResponse(responseText) + + expect(result).toEqual({ + errorsCount: 1, + errors: [{ message: 'Simple error' }], }) }) }) diff --git a/packages/rum-core/src/domain/resource/graphql.ts b/packages/rum-core/src/domain/resource/graphql.ts index 53f0650dc0..c1d1d894bf 100644 --- a/packages/rum-core/src/domain/resource/graphql.ts +++ b/packages/rum-core/src/domain/resource/graphql.ts @@ -1,4 +1,4 @@ -import { matchList, ONE_KIBI_BYTE, safeTruncate } from '@datadog/browser-core' +import { isNonEmptyArray, matchList, ONE_KIBI_BYTE, safeTruncate } from '@datadog/browser-core' import type { RumConfiguration, GraphQlUrlOption } from '../configuration' import type { RequestCompleteEvent } from '../requestCollection' @@ -14,6 +14,11 @@ export interface GraphQlError { path?: Array } +export interface GraphQlResponseErrors { + errorsCount: number + errors: GraphQlError[] +} + export interface GraphQlMetadata { operationType: 'query' | 'mutation' | 'subscription' operationName?: string @@ -42,49 +47,49 @@ export function extractGraphQlMetadata( return metadata } -export function parseGraphQlResponse( - responseText: string, - callback: (errorsCount?: number, errors?: GraphQlError[]) => void -) { +export function parseGraphQlResponse(responseText: string): GraphQlResponseErrors | undefined { + let response: unknown try { - const response = JSON.parse(responseText) + response = JSON.parse(responseText) + } catch { + // Invalid JSON response + return + } - if (!response || !Array.isArray(response.errors) || response.errors.length === 0) { - callback() - return + if (!response || typeof response !== 'object') { + return + } + + const responseObj = response as Record + + if (!isNonEmptyArray(responseObj.errors)) { + return + } + + const errors = (responseObj.errors as Array>).map((error) => { + const graphqlError: GraphQlError = { + message: typeof error.message === 'string' ? error.message : 'Unknown GraphQL error', } - const errors = (response.errors as unknown[]).map((error: unknown) => { - const errorObj = error as Record - const graphqlError: GraphQlError = { - message: typeof errorObj.message === 'string' ? errorObj.message : 'Unknown GraphQL error', - } - - const extensions = errorObj.extensions as Record | undefined - if (extensions?.code && typeof extensions.code === 'string') { - graphqlError.code = extensions.code - } - - if (Array.isArray(errorObj.locations)) { - graphqlError.locations = (errorObj.locations as unknown[]).map((loc: unknown) => { - const locObj = loc as Record - return { - line: typeof locObj.line === 'number' ? locObj.line : 0, - column: typeof locObj.column === 'number' ? locObj.column : 0, - } - }) - } - - if (Array.isArray(errorObj.path)) { - graphqlError.path = errorObj.path as Array - } - - return graphqlError - }) - - callback(errors.length, errors) - } catch { - callback() + const extensions = error.extensions as Record | undefined + if (extensions?.code && typeof extensions.code === 'string') { + graphqlError.code = extensions.code + } + + if (Array.isArray(error.locations)) { + graphqlError.locations = error.locations + } + + if (Array.isArray(error.path)) { + graphqlError.path = error.path + } + + return graphqlError + }) + + return { + errorsCount: errors.length, + errors, } } From f803b7b69703f408fe0a1e8c72cf6b9ce6d01b38 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 03:03:19 +0100 Subject: [PATCH 09/13] =?UTF-8?q?=F0=9F=91=8C=20move=20parseGraphQLRespons?= =?UTF-8?q?e=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/requestCollection.spec.ts | 22 +++++----- .../rum-core/src/domain/requestCollection.ts | 44 +++++++------------ .../src/domain/resource/graphql.spec.ts | 10 +++-- .../rum-core/src/domain/resource/graphql.ts | 11 ++--- 4 files changed, 38 insertions(+), 49 deletions(-) diff --git a/packages/rum-core/src/domain/requestCollection.spec.ts b/packages/rum-core/src/domain/requestCollection.spec.ts index b6c65994a2..90a7bda4d3 100644 --- a/packages/rum-core/src/domain/requestCollection.spec.ts +++ b/packages/rum-core/src/domain/requestCollection.spec.ts @@ -336,7 +336,7 @@ describe('collect xhr', () => { }) }) -describe('GraphQL response errors tracking', () => { +describe('GraphQL response text collection', () => { const FAKE_GRAPHQL_URL = 'http://fake-url/graphql' function setupGraphQlFetchTest(trackResponseErrors: boolean) { @@ -355,29 +355,30 @@ describe('GraphQL response errors tracking', () => { return { mockFetchManager, completeSpy, fetch: window.fetch as MockFetch } } - it('should extract GraphQL errors when trackResponseErrors is enabled', (done) => { + it('should collect responseText when trackResponseErrors is enabled', (done) => { const { mockFetchManager, completeSpy, fetch } = setupGraphQlFetchTest(true) + const responseBody = JSON.stringify({ + data: null, + errors: [{ message: 'Not found' }, { message: 'Unauthorized' }], + }) + fetch(FAKE_GRAPHQL_URL, { method: 'POST', body: JSON.stringify({ query: 'query Test { test }' }), }).resolveWith({ status: 200, - responseText: JSON.stringify({ - data: null, - errors: [{ message: 'Not found' }, { message: 'Unauthorized' }], - }), + responseText: responseBody, }) mockFetchManager.whenAllComplete(() => { const request = completeSpy.calls.argsFor(0)[0] - expect(request.graphqlErrorsCount).toBe(2) - expect(request.graphqlErrors).toEqual([{ message: 'Not found' }, { message: 'Unauthorized' }]) + expect(request.responseText).toBe(responseBody) done() }) }) - it('should not extract GraphQL errors when trackResponseErrors is false', (done) => { + it('should not collect responseText when trackResponseErrors is false', (done) => { const { mockFetchManager, completeSpy, fetch } = setupGraphQlFetchTest(false) fetch(FAKE_GRAPHQL_URL, { @@ -390,8 +391,7 @@ describe('GraphQL response errors tracking', () => { mockFetchManager.whenAllComplete(() => { const request = completeSpy.calls.argsFor(0)[0] - expect(request.graphqlErrorsCount).toBeUndefined() - expect(request.graphqlErrors).toBeUndefined() + expect(request.responseText).toBeUndefined() done() }) }) diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index ce73ca5188..dc763af646 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -24,8 +24,7 @@ import { isAllowedRequestUrl } from './resource/resourceUtils' import type { Tracer } from './tracing/tracer' import { startTracer } from './tracing/tracer' import type { SpanIdentifier, TraceIdentifier } from './tracing/identifier' -import type { GraphQlError } from './resource/graphql' -import { findGraphQlConfiguration, parseGraphQlResponse } from './resource/graphql' +import { findGraphQlConfiguration } from './resource/graphql' export interface CustomContext { requestIndex: number @@ -36,13 +35,11 @@ export interface CustomContext { export interface RumFetchStartContext extends FetchStartContext, CustomContext {} export interface RumFetchResolveContext extends FetchResolveContext, CustomContext { duration?: Duration - graphqlErrorsCount?: number - graphqlErrors?: GraphQlError[] + responseText?: string } export interface RumXhrStartContext extends XhrStartContext, CustomContext {} export interface RumXhrCompleteContext extends XhrCompleteContext, CustomContext { - graphqlErrorsCount?: number - graphqlErrors?: GraphQlError[] + responseText?: string } export interface RequestStartEvent { @@ -69,8 +66,7 @@ export interface RequestCompleteEvent { isAborted: boolean handlingStack?: string body?: unknown - graphqlErrorsCount?: number - graphqlErrors?: GraphQlError[] + responseText?: string } let nextRequestIndex = 1 @@ -105,7 +101,7 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, }) break case 'complete': - extractGraphQlErrorsFromXhr(context, configuration) + extractResponseTextFromXhr(context, configuration) tracer.clearTracingIfNeeded(context) lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { duration: context.duration, @@ -122,8 +118,7 @@ export function trackXhr(lifeCycle: LifeCycle, configuration: RumConfiguration, isAborted: context.isAborted, handlingStack: context.handlingStack, body: context.body, - graphqlErrorsCount: context.graphqlErrorsCount, - graphqlErrors: context.graphqlErrors, + responseText: context.responseText, }) break } @@ -150,7 +145,7 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration }) break case 'resolve': - waitForFetchResponseAndExtractGraphQlErrors(context, configuration, () => { + waitForFetchResponseAndExtractResponseText(context, configuration, () => { tracer.clearTracingIfNeeded(context) lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, { duration: context.duration!, @@ -170,8 +165,7 @@ export function trackFetch(lifeCycle: LifeCycle, configuration: RumConfiguration isAborted: context.isAborted, handlingStack: context.handlingStack, body: context.init?.body, - graphqlErrorsCount: context.graphqlErrorsCount, - graphqlErrors: context.graphqlErrors, + responseText: context.responseText, }) }) break @@ -186,20 +180,16 @@ function getNextRequestIndex() { return result } -function extractGraphQlErrorsFromXhr(context: RumXhrCompleteContext, configuration: RumConfiguration) { +function extractResponseTextFromXhr(context: RumXhrCompleteContext, configuration: RumConfiguration) { const graphQlConfig = findGraphQlConfiguration(context.url, configuration) if (!graphQlConfig?.trackResponseErrors || !context.xhr || typeof context.xhr.response !== 'string') { return } - const result = parseGraphQlResponse(context.xhr.response) - if (result) { - context.graphqlErrorsCount = result.errorsCount - context.graphqlErrors = result.errors - } + context.responseText = context.xhr.response } -function waitForFetchResponseAndExtractGraphQlErrors( +function waitForFetchResponseAndExtractResponseText( context: RumFetchResolveContext, configuration: RumConfiguration, onComplete: () => void @@ -213,26 +203,22 @@ function waitForFetchResponseAndExtractGraphQlErrors( } const graphQlConfig = findGraphQlConfiguration(context.url, configuration) - const shouldExtractGraphQlErrors = graphQlConfig?.trackResponseErrors + const shouldCollectResponseText = graphQlConfig?.trackResponseErrors readBytesFromStream( clonedResponse.body, (error?: Error, bytes?: Uint8Array) => { context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) - if (shouldExtractGraphQlErrors && !error && bytes) { - const result = parseGraphQlResponse(new TextDecoder().decode(bytes)) - if (result) { - context.graphqlErrorsCount = result.errorsCount - context.graphqlErrors = result.errors - } + if (shouldCollectResponseText && !error && bytes) { + context.responseText = new TextDecoder().decode(bytes) } onComplete() }, { bytesLimit: Number.POSITIVE_INFINITY, - collectStreamBody: shouldExtractGraphQlErrors, + collectStreamBody: shouldCollectResponseText, } ) } diff --git a/packages/rum-core/src/domain/resource/graphql.spec.ts b/packages/rum-core/src/domain/resource/graphql.spec.ts index bbe562c6ea..ad28fb6d34 100644 --- a/packages/rum-core/src/domain/resource/graphql.spec.ts +++ b/packages/rum-core/src/domain/resource/graphql.spec.ts @@ -268,8 +268,9 @@ describe('GraphQL detection and metadata extraction', () => { const request: RequestCompleteEvent = { body: requestBody, - graphqlErrorsCount: 1, - graphqlErrors: [{ message: 'Not found' }], + responseText: JSON.stringify({ + errors: [{ message: 'Not found' }], + }), } as RequestCompleteEvent const result = extractGraphQlMetadata(request, { @@ -310,8 +311,9 @@ describe('GraphQL detection and metadata extraction', () => { const request: RequestCompleteEvent = { body: requestBody, - graphqlErrorsCount: 2, - graphqlErrors: [{ message: 'Error 1' }, { message: 'Error 2' }], + responseText: JSON.stringify({ + errors: [{ message: 'Error 1' }, { message: 'Error 2' }], + }), } as RequestCompleteEvent const result = extractGraphQlMetadata(request, { diff --git a/packages/rum-core/src/domain/resource/graphql.ts b/packages/rum-core/src/domain/resource/graphql.ts index c1d1d894bf..b2e75fd844 100644 --- a/packages/rum-core/src/domain/resource/graphql.ts +++ b/packages/rum-core/src/domain/resource/graphql.ts @@ -37,11 +37,12 @@ export function extractGraphQlMetadata( return } - if (request.graphqlErrorsCount !== undefined) { - metadata.errors_count = request.graphqlErrorsCount - } - if (request.graphqlErrors !== undefined) { - metadata.errors = request.graphqlErrors + if (graphQlConfig.trackResponseErrors && request.responseText) { + const responseErrors = parseGraphQlResponse(request.responseText) + if (responseErrors) { + metadata.errors_count = responseErrors.errorsCount + metadata.errors = responseErrors.errors + } } return metadata From 84bfaf040f12b781e95f1a5bfbdfd4170afa18aa Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 05:13:37 +0100 Subject: [PATCH 10/13] =?UTF-8?q?=F0=9F=91=8C=20Merge=20the=20two=20readRe?= =?UTF-8?q?ponseFunction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/index.ts | 2 + .../core/src/tools/readResponseBody.spec.ts | 191 ++++++++++++++++++ packages/core/src/tools/readResponseBody.ts | 135 +++++++++++++ .../networkError/networkErrorCollection.ts | 101 ++------- .../rum-core/src/domain/requestCollection.ts | 29 +-- 5 files changed, 365 insertions(+), 93 deletions(-) create mode 100644 packages/core/src/tools/readResponseBody.spec.ts create mode 100644 packages/core/src/tools/readResponseBody.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0935c47ed2..190f5051c9 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -131,6 +131,8 @@ export { CustomerDataType, CustomerContextKey, ContextManagerMethod } from './do export type { ValueHistory, ValueHistoryEntry } from './tools/valueHistory' export { createValueHistory, CLEAR_OLD_VALUES_INTERVAL } from './tools/valueHistory' export { readBytesFromStream } from './tools/readBytesFromStream' +export type { ReadResponseBodyResult, ReadResponseBodyOptions, RequestContext } from './tools/readResponseBody' +export { readResponseBody } from './tools/readResponseBody' export type { SessionState } from './domain/session/sessionState' export { STORAGE_POLL_DELAY } from './domain/session/sessionStore' export { SESSION_STORE_KEY } from './domain/session/storeStrategies/sessionStoreStrategy' diff --git a/packages/core/src/tools/readResponseBody.spec.ts b/packages/core/src/tools/readResponseBody.spec.ts new file mode 100644 index 0000000000..b270686c49 --- /dev/null +++ b/packages/core/src/tools/readResponseBody.spec.ts @@ -0,0 +1,191 @@ +import { MockResponse } from '../../test' +import { readResponseBody } from './readResponseBody' + +describe('readResponseBody', () => { + describe('for XHR requests', () => { + it('should read string response from XHR', (done) => { + const xhr = { response: 'test response' } as XMLHttpRequest + + readResponseBody( + { xhr }, + (result) => { + expect(result.body).toBe('test response') + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should return non-string XHR response as-is', (done) => { + const xhr = { response: { foo: 'bar' } } as XMLHttpRequest + + readResponseBody( + { xhr }, + (result) => { + expect(result.body).toEqual({ foo: 'bar' }) + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should truncate XHR response when bytesLimit is specified', (done) => { + const xhr = { response: 'Lorem ipsum dolor sit amet orci aliquam.' } as XMLHttpRequest + + readResponseBody( + { xhr }, + (result) => { + expect(result.body).toBe('Lorem ipsum dolor sit amet orci ...') + expect(result.limitExceeded).toBe(true) + done() + }, + { bytesLimit: 32 } + ) + }) + + it('should not truncate XHR response when size equals limit', (done) => { + const text = 'foo' + const xhr = { response: text } as XMLHttpRequest + + readResponseBody( + { xhr }, + (result) => { + expect(result.body).toBe(text) + expect(result.limitExceeded).toBe(false) + done() + }, + { bytesLimit: text.length } + ) + }) + + it('should return undefined body when XHR is not present', (done) => { + readResponseBody( + {}, + (result) => { + expect(result.body).toBeUndefined() + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + }) + + describe('for Fetch requests', () => { + it('should read response text from Fetch', (done) => { + const response = new MockResponse({ responseText: 'fetch response' }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBe('fetch response') + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should return undefined when response body is null', (done) => { + const response = new MockResponse({}) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBeUndefined() + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should return undefined when response body is already used', (done) => { + const response = new MockResponse({ bodyUsed: true }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBeUndefined() + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should return undefined when response body is disturbed', (done) => { + const response = new MockResponse({ bodyDisturbed: true }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBeUndefined() + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + + it('should not consume the original response body', (done) => { + const response = new MockResponse({ responseText: 'test' }) + + readResponseBody( + { response }, + () => { + expect(response.bodyUsed).toBe(false) + done() + }, + {} + ) + }) + + it('should truncate fetch response when bytesLimit is specified', (done) => { + const text = 'Lorem ipsum dolor sit amet' + const response = new MockResponse({ responseText: text }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBe('Lorem ipsu...') + expect(result.limitExceeded).toBe(true) + done() + }, + { bytesLimit: 10 } + ) + }) + + it('should not truncate when size equals limit', (done) => { + const text = 'foo' + const response = new MockResponse({ responseText: text }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBe(text) + expect(result.limitExceeded).toBe(false) + done() + }, + { bytesLimit: text.length } + ) + }) + + it('should handle error reading from response stream', (done) => { + const response = new MockResponse({ responseTextError: new Error('locked') }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBeUndefined() + expect(result.error).toEqual(jasmine.any(Error)) + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) + }) +}) diff --git a/packages/core/src/tools/readResponseBody.ts b/packages/core/src/tools/readResponseBody.ts new file mode 100644 index 0000000000..5daf053093 --- /dev/null +++ b/packages/core/src/tools/readResponseBody.ts @@ -0,0 +1,135 @@ +import { tryToClone } from './utils/responseUtils' +import { readBytesFromStream } from './readBytesFromStream' +import { monitor } from './monitor' + +export interface ReadResponseBodyResult { + body?: unknown + limitExceeded: boolean + error?: Error +} + +export interface ReadResponseBodyOptions { + bytesLimit?: number +} + +export interface RequestContext { + xhr?: XMLHttpRequest + response?: Response +} + +/** + * Reads the response body from an XHR or Fetch request context. + * For XHR requests, reads directly from xhr.response. + * For Fetch requests, clones the response and reads from the stream. + * Optionally truncates the response text if bytesLimit is specified. + */ +export function readResponseBody( + request: RequestContext, + callback: (result: ReadResponseBodyResult) => void, + options: ReadResponseBodyOptions +) { + if (request.xhr) { + readXhrResponseBody(request.xhr, callback, options) + } else if (request.response) { + readFetchResponseBody(request.response, callback, options) + } else { + callback({ body: undefined, limitExceeded: false }) + } +} + +function readXhrResponseBody( + xhr: XMLHttpRequest, + callback: (result: ReadResponseBodyResult) => void, + options: ReadResponseBodyOptions +) { + if (typeof xhr.response === 'string') { + const truncated = truncateText(xhr.response, options.bytesLimit) + callback({ + body: truncated.text, + limitExceeded: truncated.limitExceeded, + }) + } else { + callback({ + body: xhr.response, + limitExceeded: false, + }) + } +} + +function readFetchResponseBody( + response: Response, + callback: (result: ReadResponseBodyResult) => void, + options: ReadResponseBodyOptions +) { + const clonedResponse = tryToClone(response) + if (!clonedResponse || !clonedResponse.body) { + callback({ body: undefined, limitExceeded: false }) + return + } + + // Legacy Edge support: use response.text() instead of reading the stream + if (!window.TextDecoder) { + clonedResponse.text().then( + monitor((text) => { + const truncated = truncateText(text, options.bytesLimit) + callback({ + body: truncated.text, + limitExceeded: truncated.limitExceeded, + }) + }), + monitor((error) => { + callback({ + body: undefined, + limitExceeded: false, + error, + }) + }) + ) + return + } + + const bytesLimit = options.bytesLimit ?? Number.POSITIVE_INFINITY + readBytesFromStream( + clonedResponse.body, + (error, bytes, limitExceeded) => { + if (error) { + callback({ + body: undefined, + limitExceeded: false, + error, + }) + return + } + + if (!bytes) { + callback({ body: undefined, limitExceeded: false }) + return + } + + let responseText = new TextDecoder().decode(bytes) + if (limitExceeded) { + responseText += '...' + } + + callback({ + body: responseText, + limitExceeded: limitExceeded || false, + }) + }, + { + bytesLimit, + collectStreamBody: true, + } + ) +} + +function truncateText(text: string, bytesLimit?: number): { text: string; limitExceeded: boolean } { + if (bytesLimit === undefined || text.length <= bytesLimit) { + return { text, limitExceeded: false } + } + + return { + text: `${text.substring(0, bytesLimit)}...`, + limitExceeded: true, + } +} diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.ts b/packages/logs/src/domain/networkError/networkErrorCollection.ts index d67ee301a9..e91691fafd 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.ts @@ -8,10 +8,8 @@ import { initFetchObservable, computeStackTrace, toStackTraceString, - monitor, noop, - readBytesFromStream, - tryToClone, + readResponseBody, isServerError, isIntakeUrl, } from '@datadog/browser-core' @@ -97,11 +95,13 @@ export function computeXhrResponseData( configuration: LogsConfiguration, callback: (responseData: unknown) => void ) { - if (typeof xhr.response === 'string') { - callback(truncateResponseText(xhr.response, configuration)) - } else { - callback(xhr.response) - } + readResponseBody( + { xhr } as any, + (result) => { + callback(result.body) + }, + { bytesLimit: configuration.requestErrorResponseLengthLimit } + ) } export function computeFetchErrorText( @@ -117,55 +117,19 @@ export function computeFetchResponseText( configuration: LogsConfiguration, callback: (responseText?: string) => void ) { - const clonedResponse = tryToClone(response) - if (!clonedResponse || !clonedResponse.body) { - // if the clone failed or if the body is null, let's not try to read it. - callback() - } else if (!window.TextDecoder) { - // If the browser doesn't support TextDecoder, let's read the whole response then truncate it. - // - // This should only be the case on early versions of Edge (before they migrated to Chromium). - // Even if it could be possible to implement a workaround for the missing TextDecoder API (using - // a Blob and FileReader), we found another issue preventing us from reading only the first - // bytes from the response: contrary to other browsers, when reading from the cloned response, - // if the original response gets canceled, the cloned response is also canceled and we can't - // know about it. In the following illustration, the promise returned by `reader.read()` may - // never be fulfilled: - // - // fetch('/').then((response) => { - // const reader = response.clone().body.getReader() - // readMore() - // function readMore() { - // reader.read().then( - // (result) => { - // if (result.done) { - // console.log('done') - // } else { - // readMore() - // } - // }, - // () => console.log('error') - // ) - // } - // response.body.getReader().cancel() - // }) - clonedResponse.text().then( - monitor((text) => callback(truncateResponseText(text, configuration))), - monitor((error) => callback(`Unable to retrieve response: ${error as string}`)) - ) - } else { - truncateResponseStream( - clonedResponse.body, - configuration.requestErrorResponseLengthLimit, - (error, responseText) => { - if (error) { - callback(`Unable to retrieve response: ${error as unknown as string}`) - } else { - callback(responseText) - } + readResponseBody( + { response } as any, + (result) => { + if (result.error) { + callback(`Unable to retrieve response: ${result.error as unknown as string}`) + } else if (typeof result.body === 'string') { + callback(result.body) + } else { + callback() } - ) - } + }, + { bytesLimit: configuration.requestErrorResponseLengthLimit } + ) } function isRejected(request: { status: number; responseType?: string }) { @@ -185,28 +149,3 @@ function format(type: RequestType) { } return 'Fetch' } - -function truncateResponseStream( - stream: ReadableStream, - bytesLimit: number, - callback: (error?: Error, responseText?: string) => void -) { - readBytesFromStream( - stream, - (error, bytes, limitExceeded) => { - if (error) { - callback(error) - } else { - let responseText = new TextDecoder().decode(bytes) - if (limitExceeded) { - responseText += '...' - } - callback(undefined, responseText) - } - }, - { - bytesLimit, - collectStreamBody: true, - } - ) -} diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index dc763af646..edc7beb4de 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -11,7 +11,7 @@ import { RequestType, initFetchObservable, initXhrObservable, - readBytesFromStream, + readResponseBody, elapsed, timeStampNow, tryToClone, @@ -182,11 +182,19 @@ function getNextRequestIndex() { function extractResponseTextFromXhr(context: RumXhrCompleteContext, configuration: RumConfiguration) { const graphQlConfig = findGraphQlConfiguration(context.url, configuration) - if (!graphQlConfig?.trackResponseErrors || !context.xhr || typeof context.xhr.response !== 'string') { + if (!graphQlConfig?.trackResponseErrors) { return } - context.responseText = context.xhr.response + readResponseBody( + context, + (result) => { + if (typeof result.body === 'string') { + context.responseText = result.body + } + }, + {} + ) } function waitForFetchResponseAndExtractResponseText( @@ -205,20 +213,17 @@ function waitForFetchResponseAndExtractResponseText( const graphQlConfig = findGraphQlConfiguration(context.url, configuration) const shouldCollectResponseText = graphQlConfig?.trackResponseErrors - readBytesFromStream( - clonedResponse.body, - (error?: Error, bytes?: Uint8Array) => { + readResponseBody( + context, + (result) => { context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) - if (shouldCollectResponseText && !error && bytes) { - context.responseText = new TextDecoder().decode(bytes) + if (shouldCollectResponseText && typeof result.body === 'string') { + context.responseText = result.body } onComplete() }, - { - bytesLimit: Number.POSITIVE_INFINITY, - collectStreamBody: shouldCollectResponseText, - } + {} ) } From f9496a1c85d8017171d1f916864d0b620e3c67b9 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 11:05:23 +0100 Subject: [PATCH 11/13] =?UTF-8?q?=F0=9F=91=8C=20do=20not=20collect=20if=20?= =?UTF-8?q?not=20needed=20/=20remove=20useless=20check=20/=20simplify=20st?= =?UTF-8?q?ring=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/src/tools/readResponseBody.spec.ts | 32 +++++++++++++++++-- packages/core/src/tools/readResponseBody.ts | 10 +++--- .../networkErrorCollection.spec.ts | 4 +-- .../networkError/networkErrorCollection.ts | 4 +-- .../rum-core/src/domain/requestCollection.ts | 23 +++---------- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/packages/core/src/tools/readResponseBody.spec.ts b/packages/core/src/tools/readResponseBody.spec.ts index b270686c49..bd8fb46a42 100644 --- a/packages/core/src/tools/readResponseBody.spec.ts +++ b/packages/core/src/tools/readResponseBody.spec.ts @@ -17,13 +17,13 @@ describe('readResponseBody', () => { ) }) - it('should return non-string XHR response as-is', (done) => { + it('should return undefined for non-string XHR response', (done) => { const xhr = { response: { foo: 'bar' } } as XMLHttpRequest readResponseBody( { xhr }, (result) => { - expect(result.body).toEqual({ foo: 'bar' }) + expect(result.body).toBeUndefined() expect(result.limitExceeded).toBe(false) done() }, @@ -187,5 +187,33 @@ describe('readResponseBody', () => { {} ) }) + + it('should not collect body when collectBody is false', (done) => { + const response = new MockResponse({ responseText: 'test response' }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBeUndefined() + expect(result.limitExceeded).toBe(false) + done() + }, + { collectBody: false } + ) + }) + + it('should collect body by default when collectBody is not specified', (done) => { + const response = new MockResponse({ responseText: 'test response' }) + + readResponseBody( + { response }, + (result) => { + expect(result.body).toBe('test response') + expect(result.limitExceeded).toBe(false) + done() + }, + {} + ) + }) }) }) diff --git a/packages/core/src/tools/readResponseBody.ts b/packages/core/src/tools/readResponseBody.ts index 5daf053093..54c6bb52a0 100644 --- a/packages/core/src/tools/readResponseBody.ts +++ b/packages/core/src/tools/readResponseBody.ts @@ -3,13 +3,14 @@ import { readBytesFromStream } from './readBytesFromStream' import { monitor } from './monitor' export interface ReadResponseBodyResult { - body?: unknown + body?: string limitExceeded: boolean error?: Error } export interface ReadResponseBodyOptions { bytesLimit?: number + collectBody?: boolean } export interface RequestContext { @@ -50,7 +51,7 @@ function readXhrResponseBody( }) } else { callback({ - body: xhr.response, + body: undefined, limitExceeded: false, }) } @@ -89,6 +90,7 @@ function readFetchResponseBody( } const bytesLimit = options.bytesLimit ?? Number.POSITIVE_INFINITY + const collectBody = options.collectBody ?? true readBytesFromStream( clonedResponse.body, (error, bytes, limitExceeded) => { @@ -101,7 +103,7 @@ function readFetchResponseBody( return } - if (!bytes) { + if (!collectBody || !bytes) { callback({ body: undefined, limitExceeded: false }) return } @@ -118,7 +120,7 @@ function readFetchResponseBody( }, { bytesLimit, - collectStreamBody: true, + collectStreamBody: collectBody, } ) } diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts b/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts index 618b09eff6..a93244e7cc 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.spec.ts @@ -160,10 +160,10 @@ describe('computeXhrResponseData', () => { }) }) - it('return the response value directly if it is not a string', (done) => { + it('returns undefined if the response value is not a string', (done) => { const xhr = { response: { foo: 'bar' } } as XMLHttpRequest computeXhrResponseData(xhr, CONFIGURATION, (responseData) => { - expect(responseData).toEqual({ foo: 'bar' }) + expect(responseData).toBeUndefined() done() }) }) diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.ts b/packages/logs/src/domain/networkError/networkErrorCollection.ts index e91691fafd..aa9ff362ba 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.ts @@ -100,7 +100,7 @@ export function computeXhrResponseData( (result) => { callback(result.body) }, - { bytesLimit: configuration.requestErrorResponseLengthLimit } + { bytesLimit: configuration.requestErrorResponseLengthLimit, collectBody: true } ) } @@ -128,7 +128,7 @@ export function computeFetchResponseText( callback() } }, - { bytesLimit: configuration.requestErrorResponseLengthLimit } + { bytesLimit: configuration.requestErrorResponseLengthLimit, collectBody: true } ) } diff --git a/packages/rum-core/src/domain/requestCollection.ts b/packages/rum-core/src/domain/requestCollection.ts index edc7beb4de..6383cdd805 100644 --- a/packages/rum-core/src/domain/requestCollection.ts +++ b/packages/rum-core/src/domain/requestCollection.ts @@ -14,7 +14,6 @@ import { readResponseBody, elapsed, timeStampNow, - tryToClone, } from '@datadog/browser-core' import type { RumSessionManager } from '..' import type { RumConfiguration } from './configuration' @@ -189,11 +188,9 @@ function extractResponseTextFromXhr(context: RumXhrCompleteContext, configuratio readResponseBody( context, (result) => { - if (typeof result.body === 'string') { - context.responseText = result.body - } + context.responseText = result.body }, - {} + { collectBody: true } ) } @@ -202,14 +199,6 @@ function waitForFetchResponseAndExtractResponseText( configuration: RumConfiguration, onComplete: () => void ) { - const clonedResponse = context.response && tryToClone(context.response) - if (!clonedResponse || !clonedResponse.body) { - // do not try to wait for the response if the clone failed, fetch error or null body - context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) - onComplete() - return - } - const graphQlConfig = findGraphQlConfiguration(context.url, configuration) const shouldCollectResponseText = graphQlConfig?.trackResponseErrors @@ -217,13 +206,9 @@ function waitForFetchResponseAndExtractResponseText( context, (result) => { context.duration = elapsed(context.startClocks.timeStamp, timeStampNow()) - - if (shouldCollectResponseText && typeof result.body === 'string') { - context.responseText = result.body - } - + context.responseText = result.body onComplete() }, - {} + { collectBody: shouldCollectResponseText } ) } From 887a8075a044c989dfd19254fb34c31de780628c Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 12:35:52 +0100 Subject: [PATCH 12/13] =?UTF-8?q?=F0=9F=90=9B=20Fix=20useless=20cast?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../logs/src/domain/networkError/networkErrorCollection.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/logs/src/domain/networkError/networkErrorCollection.ts b/packages/logs/src/domain/networkError/networkErrorCollection.ts index aa9ff362ba..584f6ac898 100644 --- a/packages/logs/src/domain/networkError/networkErrorCollection.ts +++ b/packages/logs/src/domain/networkError/networkErrorCollection.ts @@ -96,7 +96,7 @@ export function computeXhrResponseData( callback: (responseData: unknown) => void ) { readResponseBody( - { xhr } as any, + { xhr }, (result) => { callback(result.body) }, @@ -118,7 +118,7 @@ export function computeFetchResponseText( callback: (responseText?: string) => void ) { readResponseBody( - { response } as any, + { response }, (result) => { if (result.error) { callback(`Unable to retrieve response: ${result.error as unknown as string}`) From 8f7e0918ef67c4896cd6f9b81d62497414d01447 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Wed, 29 Oct 2025 12:42:22 +0100 Subject: [PATCH 13/13] =?UTF-8?q?=F0=9F=A5=9C=20Modify=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/tools/readResponseBody.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/core/src/tools/readResponseBody.ts b/packages/core/src/tools/readResponseBody.ts index 54c6bb52a0..ce9618eb98 100644 --- a/packages/core/src/tools/readResponseBody.ts +++ b/packages/core/src/tools/readResponseBody.ts @@ -18,12 +18,11 @@ export interface RequestContext { response?: Response } -/** - * Reads the response body from an XHR or Fetch request context. - * For XHR requests, reads directly from xhr.response. - * For Fetch requests, clones the response and reads from the stream. - * Optionally truncates the response text if bytesLimit is specified. - */ +// Reads the response body from an XHR or Fetch request context. +// For XHR requests, reads directly from xhr.response. +// For Fetch requests, clones the response and reads from the stream. +// Optionally truncates the response text if bytesLimit is specified. + export function readResponseBody( request: RequestContext, callback: (result: ReadResponseBodyResult) => void,