From 0e05a40b133e5fa2b61734c0f183e4c44fc990e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Wed, 6 Aug 2025 17:19:56 +0200 Subject: [PATCH 01/10] feat(core): MCP Server - Capture prompt results from prompt function calls (#17284) closes #17283 includes these attributes for `mcp.server` spans: - `mcp.prompt.result.description` - `mcp.prompt.result.message_content` - `mcp.prompt.result.message_role` - `mcp.prompt.result.message_count` Example: Screenshot 2025-08-01 at 12 40 46 Needed to make `attributeExtraction.ts` <300 lines of code (requirement) so it's now split between `sessionExtraction.ts`, `sessionExtraction.ts` and `resultExtraction.ts`. So changes explained so it's easier to review: - The only function this PR adds is `extractPromptResultAttributes` inside `resultExtraction.ts`. - It adds the prompt results as PII in `piiFiltering.ts`. Just add them to the `set`. - adds a `else if (method === 'prompts/get')` to execute the `extractPromptResultAttributes` function. - adds a test that checks we're capturing the results and updates the PII test to check PII result attributes are being removed if sending PII is not enabled. --- .../mcp-server/attributeExtraction.ts | 267 +----------------- .../src/integrations/mcp-server/attributes.ts | 22 ++ .../integrations/mcp-server/correlation.ts | 13 +- .../integrations/mcp-server/piiFiltering.ts | 34 ++- .../mcp-server/resultExtraction.ts | 126 +++++++++ .../mcp-server/sessionExtraction.ts | 202 +++++++++++++ .../mcp-server/sessionManagement.ts | 4 +- .../src/integrations/mcp-server/transport.ts | 12 +- .../src/integrations/mcp-server/validation.ts | 9 + .../mcp-server/piiFiltering.test.ts | 42 ++- .../mcp-server/semanticConventions.test.ts | 72 +++++ .../transportInstrumentation.test.ts | 2 +- 12 files changed, 526 insertions(+), 279 deletions(-) create mode 100644 packages/core/src/integrations/mcp-server/resultExtraction.ts create mode 100644 packages/core/src/integrations/mcp-server/sessionExtraction.ts diff --git a/packages/core/src/integrations/mcp-server/attributeExtraction.ts b/packages/core/src/integrations/mcp-server/attributeExtraction.ts index 68eade987a08..8f1e5a77d94d 100644 --- a/packages/core/src/integrations/mcp-server/attributeExtraction.ts +++ b/packages/core/src/integrations/mcp-server/attributeExtraction.ts @@ -1,66 +1,18 @@ /** - * Attribute extraction and building functions for MCP server instrumentation + * Core attribute extraction and building functions for MCP server instrumentation */ import { isURLObjectRelative, parseStringToURLObject } from '../../utils/url'; import { - CLIENT_ADDRESS_ATTRIBUTE, - CLIENT_PORT_ATTRIBUTE, MCP_LOGGING_DATA_TYPE_ATTRIBUTE, MCP_LOGGING_LEVEL_ATTRIBUTE, MCP_LOGGING_LOGGER_ATTRIBUTE, MCP_LOGGING_MESSAGE_ATTRIBUTE, - MCP_PROTOCOL_VERSION_ATTRIBUTE, MCP_REQUEST_ID_ATTRIBUTE, MCP_RESOURCE_URI_ATTRIBUTE, - MCP_SERVER_NAME_ATTRIBUTE, - MCP_SERVER_TITLE_ATTRIBUTE, - MCP_SERVER_VERSION_ATTRIBUTE, - MCP_SESSION_ID_ATTRIBUTE, - MCP_TOOL_RESULT_CONTENT_COUNT_ATTRIBUTE, - MCP_TOOL_RESULT_IS_ERROR_ATTRIBUTE, - MCP_TRANSPORT_ATTRIBUTE, - NETWORK_PROTOCOL_VERSION_ATTRIBUTE, - NETWORK_TRANSPORT_ATTRIBUTE, } from './attributes'; import { extractTargetInfo, getRequestArguments } from './methodConfig'; -import { - getClientInfoForTransport, - getProtocolVersionForTransport, - getSessionDataForTransport, -} from './sessionManagement'; -import type { - ExtraHandlerData, - JsonRpcNotification, - JsonRpcRequest, - McpSpanType, - MCPTransport, - PartyInfo, - SessionData, -} from './types'; - -/** - * Extracts transport types based on transport constructor name - * @param transport - MCP transport instance - * @returns Transport type mapping for span attributes - */ -export function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { - const transportName = transport.constructor?.name?.toLowerCase() || ''; - - if (transportName.includes('stdio')) { - return { mcpTransport: 'stdio', networkTransport: 'pipe' }; - } - - if (transportName.includes('streamablehttp') || transportName.includes('streamable')) { - return { mcpTransport: 'http', networkTransport: 'tcp' }; - } - - if (transportName.includes('sse')) { - return { mcpTransport: 'sse', networkTransport: 'tcp' }; - } - - return { mcpTransport: 'unknown', networkTransport: 'unknown' }; -} +import type { JsonRpcNotification, JsonRpcRequest, McpSpanType } from './types'; /** * Extracts additional attributes for specific notification types @@ -138,155 +90,6 @@ export function getNotificationAttributes( return attributes; } -/** - * Extracts and validates PartyInfo from an unknown object - * @param obj - Unknown object that might contain party info - * @returns Validated PartyInfo object with only string properties - */ -function extractPartyInfo(obj: unknown): PartyInfo { - const partyInfo: PartyInfo = {}; - - if (obj && typeof obj === 'object' && obj !== null) { - const source = obj as Record; - if (typeof source.name === 'string') partyInfo.name = source.name; - if (typeof source.title === 'string') partyInfo.title = source.title; - if (typeof source.version === 'string') partyInfo.version = source.version; - } - - return partyInfo; -} - -/** - * Extracts session data from "initialize" requests - * @param request - JSON-RPC "initialize" request containing client info and protocol version - * @returns Session data extracted from request parameters including protocol version and client info - */ -export function extractSessionDataFromInitializeRequest(request: JsonRpcRequest): SessionData { - const sessionData: SessionData = {}; - if (request.params && typeof request.params === 'object' && request.params !== null) { - const params = request.params as Record; - if (typeof params.protocolVersion === 'string') { - sessionData.protocolVersion = params.protocolVersion; - } - if (params.clientInfo) { - sessionData.clientInfo = extractPartyInfo(params.clientInfo); - } - } - return sessionData; -} - -/** - * Extracts session data from "initialize" response - * @param result - "initialize" response result containing server info and protocol version - * @returns Partial session data extracted from response including protocol version and server info - */ -export function extractSessionDataFromInitializeResponse(result: unknown): Partial { - const sessionData: Partial = {}; - if (result && typeof result === 'object') { - const resultObj = result as Record; - if (typeof resultObj.protocolVersion === 'string') sessionData.protocolVersion = resultObj.protocolVersion; - if (resultObj.serverInfo) { - sessionData.serverInfo = extractPartyInfo(resultObj.serverInfo); - } - } - return sessionData; -} - -/** - * Build client attributes from stored client info - * @param transport - MCP transport instance - * @returns Client attributes for span instrumentation - */ -export function getClientAttributes(transport: MCPTransport): Record { - const clientInfo = getClientInfoForTransport(transport); - const attributes: Record = {}; - - if (clientInfo?.name) { - attributes['mcp.client.name'] = clientInfo.name; - } - if (clientInfo?.title) { - attributes['mcp.client.title'] = clientInfo.title; - } - if (clientInfo?.version) { - attributes['mcp.client.version'] = clientInfo.version; - } - - return attributes; -} - -/** - * Build server attributes from stored server info - * @param transport - MCP transport instance - * @returns Server attributes for span instrumentation - */ -export function getServerAttributes(transport: MCPTransport): Record { - const serverInfo = getSessionDataForTransport(transport)?.serverInfo; - const attributes: Record = {}; - - if (serverInfo?.name) { - attributes[MCP_SERVER_NAME_ATTRIBUTE] = serverInfo.name; - } - if (serverInfo?.title) { - attributes[MCP_SERVER_TITLE_ATTRIBUTE] = serverInfo.title; - } - if (serverInfo?.version) { - attributes[MCP_SERVER_VERSION_ATTRIBUTE] = serverInfo.version; - } - - return attributes; -} - -/** - * Extracts client connection info from extra handler data - * @param extra - Extra handler data containing connection info - * @returns Client address and port information - */ -export function extractClientInfo(extra: ExtraHandlerData): { - address?: string; - port?: number; -} { - return { - address: - extra?.requestInfo?.remoteAddress || - extra?.clientAddress || - extra?.request?.ip || - extra?.request?.connection?.remoteAddress, - port: extra?.requestInfo?.remotePort || extra?.clientPort || extra?.request?.connection?.remotePort, - }; -} - -/** - * Build transport and network attributes - * @param transport - MCP transport instance - * @param extra - Optional extra handler data - * @returns Transport attributes for span instrumentation - */ -export function buildTransportAttributes( - transport: MCPTransport, - extra?: ExtraHandlerData, -): Record { - const sessionId = transport.sessionId; - const clientInfo = extra ? extractClientInfo(extra) : {}; - const { mcpTransport, networkTransport } = getTransportTypes(transport); - const clientAttributes = getClientAttributes(transport); - const serverAttributes = getServerAttributes(transport); - const protocolVersion = getProtocolVersionForTransport(transport); - - const attributes = { - ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), - ...(clientInfo.address && { [CLIENT_ADDRESS_ATTRIBUTE]: clientInfo.address }), - ...(clientInfo.port && { [CLIENT_PORT_ATTRIBUTE]: clientInfo.port }), - [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, - [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, - [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', - ...(protocolVersion && { [MCP_PROTOCOL_VERSION_ATTRIBUTE]: protocolVersion }), - ...clientAttributes, - ...serverAttributes, - }; - - return attributes; -} - /** * Build type-specific attributes based on message type * @param type - Span type (request or notification) @@ -313,67 +116,5 @@ export function buildTypeSpecificAttributes( return getNotificationAttributes(message.method, params || {}); } -/** - * Build attributes for tool result content items - * @param content - Array of content items from tool result - * @returns Attributes extracted from each content item including type, text, mime type, URI, and resource info - */ -function buildAllContentItemAttributes(content: unknown[]): Record { - const attributes: Record = { - [MCP_TOOL_RESULT_CONTENT_COUNT_ATTRIBUTE]: content.length, - }; - - for (const [i, item] of content.entries()) { - if (typeof item !== 'object' || item === null) continue; - - const contentItem = item as Record; - const prefix = content.length === 1 ? 'mcp.tool.result' : `mcp.tool.result.${i}`; - - const safeSet = (key: string, value: unknown): void => { - if (typeof value === 'string') attributes[`${prefix}.${key}`] = value; - }; - - safeSet('content_type', contentItem.type); - safeSet('mime_type', contentItem.mimeType); - safeSet('uri', contentItem.uri); - safeSet('name', contentItem.name); - - if (typeof contentItem.text === 'string') { - const text = contentItem.text; - const maxLength = 500; - attributes[`${prefix}.content`] = text.length > maxLength ? `${text.slice(0, maxLength - 3)}...` : text; - } - - if (typeof contentItem.data === 'string') { - attributes[`${prefix}.data_size`] = contentItem.data.length; - } - - const resource = contentItem.resource; - if (typeof resource === 'object' && resource !== null) { - const res = resource as Record; - safeSet('resource_uri', res.uri); - safeSet('resource_mime_type', res.mimeType); - } - } - - return attributes; -} - -/** - * Extract tool result attributes for span instrumentation - * @param result - Tool execution result - * @returns Attributes extracted from tool result content - */ -export function extractToolResultAttributes(result: unknown): Record { - let attributes: Record = {}; - if (typeof result !== 'object' || result === null) return attributes; - - const resultObj = result as Record; - if (typeof resultObj.isError === 'boolean') { - attributes[MCP_TOOL_RESULT_IS_ERROR_ATTRIBUTE] = resultObj.isError; - } - if (Array.isArray(resultObj.content)) { - attributes = { ...attributes, ...buildAllContentItemAttributes(resultObj.content) }; - } - return attributes; -} +// Re-export buildTransportAttributes for spans.ts +export { buildTransportAttributes } from './sessionExtraction'; diff --git a/packages/core/src/integrations/mcp-server/attributes.ts b/packages/core/src/integrations/mcp-server/attributes.ts index 074bd09b7bdf..273bdbdb9560 100644 --- a/packages/core/src/integrations/mcp-server/attributes.ts +++ b/packages/core/src/integrations/mcp-server/attributes.ts @@ -76,6 +76,28 @@ export const MCP_TOOL_RESULT_CONTENT_COUNT_ATTRIBUTE = 'mcp.tool.result.content_ /** Serialized content of the tool result */ export const MCP_TOOL_RESULT_CONTENT_ATTRIBUTE = 'mcp.tool.result.content'; +/** Prefix for tool result attributes that contain sensitive content */ +export const MCP_TOOL_RESULT_PREFIX = 'mcp.tool.result'; + +// ============================================================================= +// PROMPT RESULT ATTRIBUTES +// ============================================================================= + +/** Description of the prompt result */ +export const MCP_PROMPT_RESULT_DESCRIPTION_ATTRIBUTE = 'mcp.prompt.result.description'; + +/** Number of messages in the prompt result */ +export const MCP_PROMPT_RESULT_MESSAGE_COUNT_ATTRIBUTE = 'mcp.prompt.result.message_count'; + +/** Role of the message in the prompt result (for single message results) */ +export const MCP_PROMPT_RESULT_MESSAGE_ROLE_ATTRIBUTE = 'mcp.prompt.result.message_role'; + +/** Content of the message in the prompt result (for single message results) */ +export const MCP_PROMPT_RESULT_MESSAGE_CONTENT_ATTRIBUTE = 'mcp.prompt.result.message_content'; + +/** Prefix for prompt result attributes that contain sensitive content */ +export const MCP_PROMPT_RESULT_PREFIX = 'mcp.prompt.result'; + // ============================================================================= // REQUEST ARGUMENT ATTRIBUTES // ============================================================================= diff --git a/packages/core/src/integrations/mcp-server/correlation.ts b/packages/core/src/integrations/mcp-server/correlation.ts index 7f00341bdd5a..7a73f63f64e3 100644 --- a/packages/core/src/integrations/mcp-server/correlation.ts +++ b/packages/core/src/integrations/mcp-server/correlation.ts @@ -9,8 +9,8 @@ import { getClient } from '../../currentScopes'; import { SPAN_STATUS_ERROR } from '../../tracing'; import type { Span } from '../../types-hoist/span'; -import { extractToolResultAttributes } from './attributeExtraction'; import { filterMcpPiiFromSpanData } from './piiFiltering'; +import { extractPromptResultAttributes, extractToolResultAttributes } from './resultExtraction'; import type { MCPTransport, RequestId, RequestSpanMapValue } from './types'; /** @@ -69,6 +69,13 @@ export function completeSpanWithResults(transport: MCPTransport, requestId: Requ const toolAttributes = filterMcpPiiFromSpanData(rawToolAttributes, sendDefaultPii); span.setAttributes(toolAttributes); + } else if (method === 'prompts/get') { + const rawPromptAttributes = extractPromptResultAttributes(result); + const client = getClient(); + const sendDefaultPii = Boolean(client?.getOptions().sendDefaultPii); + const promptAttributes = filterMcpPiiFromSpanData(rawPromptAttributes, sendDefaultPii); + + span.setAttributes(promptAttributes); } span.end(); @@ -83,7 +90,9 @@ export function completeSpanWithResults(transport: MCPTransport, requestId: Requ */ export function cleanupPendingSpansForTransport(transport: MCPTransport): number { const spanMap = transportToSpanMap.get(transport); - if (!spanMap) return 0; + if (!spanMap) { + return 0; + } const pendingCount = spanMap.size; diff --git a/packages/core/src/integrations/mcp-server/piiFiltering.ts b/packages/core/src/integrations/mcp-server/piiFiltering.ts index 654427ca2d6d..ff801cbf2a1e 100644 --- a/packages/core/src/integrations/mcp-server/piiFiltering.ts +++ b/packages/core/src/integrations/mcp-server/piiFiltering.ts @@ -9,9 +9,13 @@ import { CLIENT_ADDRESS_ATTRIBUTE, CLIENT_PORT_ATTRIBUTE, MCP_LOGGING_MESSAGE_ATTRIBUTE, + MCP_PROMPT_RESULT_DESCRIPTION_ATTRIBUTE, + MCP_PROMPT_RESULT_MESSAGE_CONTENT_ATTRIBUTE, + MCP_PROMPT_RESULT_PREFIX, MCP_REQUEST_ARGUMENT, MCP_RESOURCE_URI_ATTRIBUTE, MCP_TOOL_RESULT_CONTENT_ATTRIBUTE, + MCP_TOOL_RESULT_PREFIX, } from './attributes'; /** @@ -22,16 +26,42 @@ const PII_ATTRIBUTES = new Set([ CLIENT_ADDRESS_ATTRIBUTE, CLIENT_PORT_ATTRIBUTE, MCP_LOGGING_MESSAGE_ATTRIBUTE, + MCP_PROMPT_RESULT_DESCRIPTION_ATTRIBUTE, + MCP_PROMPT_RESULT_MESSAGE_CONTENT_ATTRIBUTE, MCP_RESOURCE_URI_ATTRIBUTE, MCP_TOOL_RESULT_CONTENT_ATTRIBUTE, ]); /** - * Checks if an attribute key should be considered PII + * Checks if an attribute key should be considered PII. + * + * Returns true for: + * - Explicit PII attributes (client.address, client.port, mcp.logging.message, etc.) + * - All request arguments (mcp.request.argument.*) + * - Tool and prompt result content (mcp.tool.result.*, mcp.prompt.result.*) except metadata + * + * Preserves metadata attributes ending with _count, _error, or .is_error as they don't contain sensitive data. + * + * @param key - Attribute key to evaluate + * @returns true if the attribute should be filtered out (is PII), false if it should be preserved * @internal */ function isPiiAttribute(key: string): boolean { - return PII_ATTRIBUTES.has(key) || key.startsWith(`${MCP_REQUEST_ARGUMENT}.`); + if (PII_ATTRIBUTES.has(key)) { + return true; + } + + if (key.startsWith(`${MCP_REQUEST_ARGUMENT}.`)) { + return true; + } + + if (key.startsWith(`${MCP_TOOL_RESULT_PREFIX}.`) || key.startsWith(`${MCP_PROMPT_RESULT_PREFIX}.`)) { + if (!key.endsWith('_count') && !key.endsWith('_error') && !key.endsWith('.is_error')) { + return true; + } + } + + return false; } /** diff --git a/packages/core/src/integrations/mcp-server/resultExtraction.ts b/packages/core/src/integrations/mcp-server/resultExtraction.ts new file mode 100644 index 000000000000..34dc2be9d09c --- /dev/null +++ b/packages/core/src/integrations/mcp-server/resultExtraction.ts @@ -0,0 +1,126 @@ +/** + * Result extraction functions for MCP server instrumentation + * + * Handles extraction of attributes from tool and prompt execution results. + */ + +import { + MCP_PROMPT_RESULT_DESCRIPTION_ATTRIBUTE, + MCP_PROMPT_RESULT_MESSAGE_COUNT_ATTRIBUTE, + MCP_TOOL_RESULT_CONTENT_COUNT_ATTRIBUTE, + MCP_TOOL_RESULT_IS_ERROR_ATTRIBUTE, +} from './attributes'; +import { isValidContentItem } from './validation'; + +/** + * Build attributes for tool result content items + * @param content - Array of content items from tool result + * @returns Attributes extracted from each content item including type, text, mime type, URI, and resource info + */ +function buildAllContentItemAttributes(content: unknown[]): Record { + const attributes: Record = { + [MCP_TOOL_RESULT_CONTENT_COUNT_ATTRIBUTE]: content.length, + }; + + for (const [i, item] of content.entries()) { + if (!isValidContentItem(item)) { + continue; + } + + const prefix = content.length === 1 ? 'mcp.tool.result' : `mcp.tool.result.${i}`; + + const safeSet = (key: string, value: unknown): void => { + if (typeof value === 'string') { + attributes[`${prefix}.${key}`] = value; + } + }; + + safeSet('content_type', item.type); + safeSet('mime_type', item.mimeType); + safeSet('uri', item.uri); + safeSet('name', item.name); + + if (typeof item.text === 'string') { + attributes[`${prefix}.content`] = item.text; + } + + if (typeof item.data === 'string') { + attributes[`${prefix}.data_size`] = item.data.length; + } + + const resource = item.resource; + if (isValidContentItem(resource)) { + safeSet('resource_uri', resource.uri); + safeSet('resource_mime_type', resource.mimeType); + } + } + + return attributes; +} + +/** + * Extract tool result attributes for span instrumentation + * @param result - Tool execution result + * @returns Attributes extracted from tool result content + */ +export function extractToolResultAttributes(result: unknown): Record { + if (!isValidContentItem(result)) { + return {}; + } + + const attributes = Array.isArray(result.content) ? buildAllContentItemAttributes(result.content) : {}; + + if (typeof result.isError === 'boolean') { + attributes[MCP_TOOL_RESULT_IS_ERROR_ATTRIBUTE] = result.isError; + } + + return attributes; +} + +/** + * Extract prompt result attributes for span instrumentation + * @param result - Prompt execution result + * @returns Attributes extracted from prompt result + */ +export function extractPromptResultAttributes(result: unknown): Record { + const attributes: Record = {}; + if (!isValidContentItem(result)) { + return attributes; + } + + if (typeof result.description === 'string') { + attributes[MCP_PROMPT_RESULT_DESCRIPTION_ATTRIBUTE] = result.description; + } + + if (Array.isArray(result.messages)) { + attributes[MCP_PROMPT_RESULT_MESSAGE_COUNT_ATTRIBUTE] = result.messages.length; + + const messages = result.messages; + for (const [i, message] of messages.entries()) { + if (!isValidContentItem(message)) { + continue; + } + + const prefix = messages.length === 1 ? 'mcp.prompt.result' : `mcp.prompt.result.${i}`; + + const safeSet = (key: string, value: unknown): void => { + if (typeof value === 'string') { + const attrName = messages.length === 1 ? `${prefix}.message_${key}` : `${prefix}.${key}`; + attributes[attrName] = value; + } + }; + + safeSet('role', message.role); + + if (isValidContentItem(message.content)) { + const content = message.content; + if (typeof content.text === 'string') { + const attrName = messages.length === 1 ? `${prefix}.message_content` : `${prefix}.content`; + attributes[attrName] = content.text; + } + } + } + } + + return attributes; +} diff --git a/packages/core/src/integrations/mcp-server/sessionExtraction.ts b/packages/core/src/integrations/mcp-server/sessionExtraction.ts new file mode 100644 index 000000000000..90e235d4e544 --- /dev/null +++ b/packages/core/src/integrations/mcp-server/sessionExtraction.ts @@ -0,0 +1,202 @@ +/** + * Session and party info extraction functions for MCP server instrumentation + * + * Handles extraction of client/server info and session data from MCP messages. + */ + +import { + CLIENT_ADDRESS_ATTRIBUTE, + CLIENT_PORT_ATTRIBUTE, + MCP_PROTOCOL_VERSION_ATTRIBUTE, + MCP_SERVER_NAME_ATTRIBUTE, + MCP_SERVER_TITLE_ATTRIBUTE, + MCP_SERVER_VERSION_ATTRIBUTE, + MCP_SESSION_ID_ATTRIBUTE, + MCP_TRANSPORT_ATTRIBUTE, + NETWORK_PROTOCOL_VERSION_ATTRIBUTE, + NETWORK_TRANSPORT_ATTRIBUTE, +} from './attributes'; +import { + getClientInfoForTransport, + getProtocolVersionForTransport, + getSessionDataForTransport, +} from './sessionManagement'; +import type { ExtraHandlerData, JsonRpcRequest, MCPTransport, PartyInfo, SessionData } from './types'; +import { isValidContentItem } from './validation'; + +/** + * Extracts and validates PartyInfo from an unknown object + * @param obj - Unknown object that might contain party info + * @returns Validated PartyInfo object with only string properties + */ +function extractPartyInfo(obj: unknown): PartyInfo { + const partyInfo: PartyInfo = {}; + + if (isValidContentItem(obj)) { + if (typeof obj.name === 'string') { + partyInfo.name = obj.name; + } + if (typeof obj.title === 'string') { + partyInfo.title = obj.title; + } + if (typeof obj.version === 'string') { + partyInfo.version = obj.version; + } + } + + return partyInfo; +} + +/** + * Extracts session data from "initialize" requests + * @param request - JSON-RPC "initialize" request containing client info and protocol version + * @returns Session data extracted from request parameters including protocol version and client info + */ +export function extractSessionDataFromInitializeRequest(request: JsonRpcRequest): SessionData { + const sessionData: SessionData = {}; + if (isValidContentItem(request.params)) { + if (typeof request.params.protocolVersion === 'string') { + sessionData.protocolVersion = request.params.protocolVersion; + } + if (request.params.clientInfo) { + sessionData.clientInfo = extractPartyInfo(request.params.clientInfo); + } + } + return sessionData; +} + +/** + * Extracts session data from "initialize" response + * @param result - "initialize" response result containing server info and protocol version + * @returns Partial session data extracted from response including protocol version and server info + */ +export function extractSessionDataFromInitializeResponse(result: unknown): Partial { + const sessionData: Partial = {}; + if (isValidContentItem(result)) { + if (typeof result.protocolVersion === 'string') { + sessionData.protocolVersion = result.protocolVersion; + } + if (result.serverInfo) { + sessionData.serverInfo = extractPartyInfo(result.serverInfo); + } + } + return sessionData; +} + +/** + * Build client attributes from stored client info + * @param transport - MCP transport instance + * @returns Client attributes for span instrumentation + */ +export function getClientAttributes(transport: MCPTransport): Record { + const clientInfo = getClientInfoForTransport(transport); + const attributes: Record = {}; + + if (clientInfo?.name) { + attributes['mcp.client.name'] = clientInfo.name; + } + if (clientInfo?.title) { + attributes['mcp.client.title'] = clientInfo.title; + } + if (clientInfo?.version) { + attributes['mcp.client.version'] = clientInfo.version; + } + + return attributes; +} + +/** + * Build server attributes from stored server info + * @param transport - MCP transport instance + * @returns Server attributes for span instrumentation + */ +export function getServerAttributes(transport: MCPTransport): Record { + const serverInfo = getSessionDataForTransport(transport)?.serverInfo; + const attributes: Record = {}; + + if (serverInfo?.name) { + attributes[MCP_SERVER_NAME_ATTRIBUTE] = serverInfo.name; + } + if (serverInfo?.title) { + attributes[MCP_SERVER_TITLE_ATTRIBUTE] = serverInfo.title; + } + if (serverInfo?.version) { + attributes[MCP_SERVER_VERSION_ATTRIBUTE] = serverInfo.version; + } + + return attributes; +} + +/** + * Extracts client connection info from extra handler data + * @param extra - Extra handler data containing connection info + * @returns Client address and port information + */ +export function extractClientInfo(extra: ExtraHandlerData): { + address?: string; + port?: number; +} { + return { + address: + extra?.requestInfo?.remoteAddress || + extra?.clientAddress || + extra?.request?.ip || + extra?.request?.connection?.remoteAddress, + port: extra?.requestInfo?.remotePort || extra?.clientPort || extra?.request?.connection?.remotePort, + }; +} + +/** + * Extracts transport types based on transport constructor name + * @param transport - MCP transport instance + * @returns Transport type mapping for span attributes + */ +export function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } { + const transportName = transport.constructor?.name?.toLowerCase() || ''; + + if (transportName.includes('stdio')) { + return { mcpTransport: 'stdio', networkTransport: 'pipe' }; + } + + if (transportName.includes('streamablehttp') || transportName.includes('streamable')) { + return { mcpTransport: 'http', networkTransport: 'tcp' }; + } + + if (transportName.includes('sse')) { + return { mcpTransport: 'sse', networkTransport: 'tcp' }; + } + + return { mcpTransport: 'unknown', networkTransport: 'unknown' }; +} + +/** + * Build transport and network attributes + * @param transport - MCP transport instance + * @param extra - Optional extra handler data + * @returns Transport attributes for span instrumentation + */ +export function buildTransportAttributes( + transport: MCPTransport, + extra?: ExtraHandlerData, +): Record { + const sessionId = transport.sessionId; + const clientInfo = extra ? extractClientInfo(extra) : {}; + const { mcpTransport, networkTransport } = getTransportTypes(transport); + const clientAttributes = getClientAttributes(transport); + const serverAttributes = getServerAttributes(transport); + const protocolVersion = getProtocolVersionForTransport(transport); + + const attributes = { + ...(sessionId && { [MCP_SESSION_ID_ATTRIBUTE]: sessionId }), + ...(clientInfo.address && { [CLIENT_ADDRESS_ATTRIBUTE]: clientInfo.address }), + ...(clientInfo.port && { [CLIENT_PORT_ATTRIBUTE]: clientInfo.port }), + [MCP_TRANSPORT_ATTRIBUTE]: mcpTransport, + [NETWORK_TRANSPORT_ATTRIBUTE]: networkTransport, + [NETWORK_PROTOCOL_VERSION_ATTRIBUTE]: '2.0', + ...(protocolVersion && { [MCP_PROTOCOL_VERSION_ATTRIBUTE]: protocolVersion }), + ...clientAttributes, + ...serverAttributes, + }; + + return attributes; +} diff --git a/packages/core/src/integrations/mcp-server/sessionManagement.ts b/packages/core/src/integrations/mcp-server/sessionManagement.ts index 99ba2e0d8806..9d9c8b48f27d 100644 --- a/packages/core/src/integrations/mcp-server/sessionManagement.ts +++ b/packages/core/src/integrations/mcp-server/sessionManagement.ts @@ -16,7 +16,9 @@ const transportToSessionData = new WeakMap(); * @param sessionData - Session data to store */ export function storeSessionDataForTransport(transport: MCPTransport, sessionData: SessionData): void { - if (transport.sessionId) transportToSessionData.set(transport, sessionData); + if (transport.sessionId) { + transportToSessionData.set(transport, sessionData); + } } /** diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 3244ce73e49a..6943ac3e8850 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -8,12 +8,9 @@ import { getIsolationScope, withIsolationScope } from '../../currentScopes'; import { startInactiveSpan, withActiveSpan } from '../../tracing'; import { fill } from '../../utils/object'; -import { - extractSessionDataFromInitializeRequest, - extractSessionDataFromInitializeResponse, -} from './attributeExtraction'; import { cleanupPendingSpansForTransport, completeSpanWithResults, storeSpanForRequest } from './correlation'; import { captureError } from './errorCapture'; +import { extractSessionDataFromInitializeRequest, extractSessionDataFromInitializeResponse } from './sessionExtraction'; import { cleanupSessionDataForTransport, storeSessionDataForTransport, @@ -21,7 +18,7 @@ import { } from './sessionManagement'; import { buildMcpServerSpanConfig, createMcpNotificationSpan, createMcpOutgoingNotificationSpan } from './spans'; import type { ExtraHandlerData, MCPTransport } from './types'; -import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse } from './validation'; +import { isJsonRpcNotification, isJsonRpcRequest, isJsonRpcResponse, isValidContentItem } from './validation'; /** * Wraps transport.onmessage to create spans for incoming messages. @@ -93,9 +90,8 @@ export function wrapTransportSend(transport: MCPTransport): void { captureJsonRpcErrorResponse(message.error); } - if (message.result && typeof message.result === 'object') { - const result = message.result as Record; - if (result.protocolVersion || result.serverInfo) { + if (isValidContentItem(message.result)) { + if (message.result.protocolVersion || message.result.serverInfo) { try { const serverData = extractSessionDataFromInitializeResponse(message.result); updateSessionDataForTransport(this, serverData); diff --git a/packages/core/src/integrations/mcp-server/validation.ts b/packages/core/src/integrations/mcp-server/validation.ts index 21d257c01aeb..9ed21b290728 100644 --- a/packages/core/src/integrations/mcp-server/validation.ts +++ b/packages/core/src/integrations/mcp-server/validation.ts @@ -75,3 +75,12 @@ export function validateMcpServerInstance(instance: unknown): boolean { DEBUG_BUILD && debug.warn('Did not patch MCP server. Interface is incompatible.'); return false; } + +/** + * Check if the item is a valid content item + * @param item - The item to check + * @returns True if the item is a valid content item, false otherwise + */ +export function isValidContentItem(item: unknown): item is Record { + return item != null && typeof item === 'object'; +} diff --git a/packages/core/test/lib/integrations/mcp-server/piiFiltering.test.ts b/packages/core/test/lib/integrations/mcp-server/piiFiltering.test.ts index 14f803b28ccc..a86ccbd534d0 100644 --- a/packages/core/test/lib/integrations/mcp-server/piiFiltering.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/piiFiltering.test.ts @@ -124,7 +124,7 @@ describe('MCP Server PII Filtering', () => { setAttributes: vi.fn(), setStatus: vi.fn(), end: vi.fn(), - } as any; + } as unknown as ReturnType; startInactiveSpanSpy.mockReturnValueOnce(mockSpan); const toolCallRequest = { @@ -147,7 +147,7 @@ describe('MCP Server PII Filtering', () => { mockTransport.send?.(toolResponse); - // Tool result content should be filtered out + // Tool result content should be filtered out, but metadata should remain const setAttributesCall = mockSpan.setAttributes.mock.calls[0]?.[0]; expect(setAttributesCall).toBeDefined(); expect(setAttributesCall).not.toHaveProperty('mcp.tool.result.content'); @@ -163,6 +163,11 @@ describe('MCP Server PII Filtering', () => { 'client.port': 54321, 'mcp.request.argument.location': '"San Francisco"', 'mcp.tool.result.content': 'Weather data: 18°C', + 'mcp.tool.result.content_count': 1, + 'mcp.prompt.result.description': 'Code review prompt for sensitive analysis', + 'mcp.prompt.result.message_content': 'Please review this confidential code.', + 'mcp.prompt.result.message_count': 1, + 'mcp.resource.result.content': 'Sensitive resource content', 'mcp.logging.message': 'User requested weather', 'mcp.resource.uri': 'file:///private/docs/secret.txt', 'mcp.method.name': 'tools/call', // Non-PII should remain @@ -180,6 +185,16 @@ describe('MCP Server PII Filtering', () => { 'mcp.request.argument.location': '"San Francisco"', 'mcp.request.argument.units': '"celsius"', 'mcp.tool.result.content': 'Weather data: 18°C', + 'mcp.tool.result.content_count': 1, + 'mcp.prompt.result.description': 'Code review prompt for sensitive analysis', + 'mcp.prompt.result.message_count': 2, + 'mcp.prompt.result.0.role': 'user', + 'mcp.prompt.result.0.content': 'Sensitive prompt content', + 'mcp.prompt.result.1.role': 'assistant', + 'mcp.prompt.result.1.content': 'Another sensitive response', + 'mcp.resource.result.content_count': 1, + 'mcp.resource.result.uri': 'file:///private/file.txt', + 'mcp.resource.result.content': 'Sensitive resource content', 'mcp.logging.message': 'User requested weather', 'mcp.resource.uri': 'file:///private/docs/secret.txt', 'mcp.method.name': 'tools/call', // Non-PII should remain @@ -188,14 +203,37 @@ describe('MCP Server PII Filtering', () => { const result = filterMcpPiiFromSpanData(spanData, false); + // Client info should be filtered expect(result).not.toHaveProperty('client.address'); expect(result).not.toHaveProperty('client.port'); + + // Request arguments should be filtered expect(result).not.toHaveProperty('mcp.request.argument.location'); expect(result).not.toHaveProperty('mcp.request.argument.units'); + + // Specific PII content attributes should be filtered expect(result).not.toHaveProperty('mcp.tool.result.content'); + expect(result).not.toHaveProperty('mcp.prompt.result.description'); + + // Count attributes should remain as they don't contain sensitive content + expect(result).toHaveProperty('mcp.tool.result.content_count', 1); + expect(result).toHaveProperty('mcp.prompt.result.message_count', 2); + + // All tool and prompt result content should be filtered (including indexed attributes) + expect(result).not.toHaveProperty('mcp.prompt.result.0.role'); + expect(result).not.toHaveProperty('mcp.prompt.result.0.content'); + expect(result).not.toHaveProperty('mcp.prompt.result.1.role'); + expect(result).not.toHaveProperty('mcp.prompt.result.1.content'); + + expect(result).toHaveProperty('mcp.resource.result.content_count', 1); + expect(result).toHaveProperty('mcp.resource.result.uri', 'file:///private/file.txt'); + expect(result).toHaveProperty('mcp.resource.result.content', 'Sensitive resource content'); + + // Other PII attributes should be filtered expect(result).not.toHaveProperty('mcp.logging.message'); expect(result).not.toHaveProperty('mcp.resource.uri'); + // Non-PII attributes should remain expect(result).toHaveProperty('mcp.method.name', 'tools/call'); expect(result).toHaveProperty('mcp.session.id', 'test-session-123'); }); diff --git a/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts b/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts index 7b110a0b2756..5437d4a5a13a 100644 --- a/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/semanticConventions.test.ts @@ -434,5 +434,77 @@ describe('MCP Server Semantic Conventions', () => { expect(setStatusSpy).not.toHaveBeenCalled(); expect(endSpy).toHaveBeenCalled(); }); + + it('should instrument prompt call results and complete span with enriched attributes', async () => { + await wrappedMcpServer.connect(mockTransport); + + const setAttributesSpy = vi.fn(); + const setStatusSpy = vi.fn(); + const endSpy = vi.fn(); + const mockSpan = { + setAttributes: setAttributesSpy, + setStatus: setStatusSpy, + end: endSpy, + }; + startInactiveSpanSpy.mockReturnValueOnce( + mockSpan as unknown as ReturnType, + ); + + const promptCallRequest = { + jsonrpc: '2.0', + method: 'prompts/get', + id: 'req-prompt-result', + params: { + name: 'code-review', + arguments: { language: 'typescript', complexity: 'high' }, + }, + }; + + mockTransport.onmessage?.(promptCallRequest, {}); + + expect(startInactiveSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'prompts/get code-review', + op: 'mcp.server', + forceTransaction: true, + attributes: expect.objectContaining({ + 'mcp.method.name': 'prompts/get', + 'mcp.prompt.name': 'code-review', + 'mcp.request.id': 'req-prompt-result', + }), + }), + ); + + const promptResponse = { + jsonrpc: '2.0', + id: 'req-prompt-result', + result: { + description: 'Code review prompt for TypeScript with high complexity analysis', + messages: [ + { + role: 'user', + content: { + type: 'text', + text: 'Please review this TypeScript code for complexity and best practices.', + }, + }, + ], + }, + }; + + mockTransport.send?.(promptResponse); + + expect(setAttributesSpy).toHaveBeenCalledWith( + expect.objectContaining({ + 'mcp.prompt.result.description': 'Code review prompt for TypeScript with high complexity analysis', + 'mcp.prompt.result.message_count': 1, + 'mcp.prompt.result.message_role': 'user', + 'mcp.prompt.result.message_content': 'Please review this TypeScript code for complexity and best practices.', + }), + ); + + expect(setStatusSpy).not.toHaveBeenCalled(); + expect(endSpy).toHaveBeenCalled(); + }); }); }); diff --git a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts index 7f06eb886cdb..008942ac4099 100644 --- a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts @@ -4,7 +4,7 @@ import { wrapMcpServerWithSentry } from '../../../../src/integrations/mcp-server import { extractSessionDataFromInitializeRequest, extractSessionDataFromInitializeResponse, -} from '../../../../src/integrations/mcp-server/attributeExtraction'; +} from '../../../../src/integrations/mcp-server/sessionExtraction'; import { cleanupSessionDataForTransport, getClientInfoForTransport, From e2579bb7d533e8a695baf820c963cee2a9c85ddc Mon Sep 17 00:00:00 2001 From: Andrei <168741329+andreiborza@users.noreply.github.com> Date: Thu, 7 Aug 2025 11:44:40 +0200 Subject: [PATCH 02/10] ref: Make `lastReleasePos` checks for yarn changelog stricter (#17340) Commits like `meta: Re-organize changelog to add v8 page` tripped this up. Our changelog entries always follow `meta(changelog)` so we should just check for that. --- scripts/get-commit-list.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/get-commit-list.ts b/scripts/get-commit-list.ts index bceccfb317de..04c5932c12cd 100644 --- a/scripts/get-commit-list.ts +++ b/scripts/get-commit-list.ts @@ -3,7 +3,7 @@ import { execSync } from 'child_process'; function run(): void { const commits = execSync('git log --format="- %s"').toString().split('\n'); - const lastReleasePos = commits.findIndex(commit => /- meta(.*)changelog/i.test(commit)); + const lastReleasePos = commits.findIndex(commit => /- meta\(changelog\)/i.test(commit)); const newCommits = commits.splice(0, lastReleasePos).filter(commit => { // Filter out merge commits From 6829fda0e8405b3d042c86d2a661c0b256f9702a Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Thu, 7 Aug 2025 13:14:06 +0200 Subject: [PATCH 03/10] feat(sveltekit): Streamline build logs (#17306) This streamlines the build logs for sveltekit. For this, I aligned how we handle source maps setting with other SDKs, which is a bit different then how sveltekit did it. 1. Only show log messages when `debug: true` 2. Show short warning when `sourceMaps: false` and `debug: false`, more details when `debug: true` is set 3. Re-organize how `filesToDeleteAfterUpload` is set, based on other SDKs (?? I hope that makes sense....) --------- Co-authored-by: Abhijeet Prasad --- packages/sveltekit/src/vite/sourceMaps.ts | 185 ++++++------ .../test/vite/sentrySvelteKitPlugins.test.ts | 7 +- .../sveltekit/test/vite/sourceMaps.test.ts | 277 ++++++++++++++++-- 3 files changed, 331 insertions(+), 138 deletions(-) diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index 5559f5be4592..eb3b449144f8 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { consoleSandbox, escapeStringForRegex, uuid4 } from '@sentry/core'; +import { escapeStringForRegex, uuid4 } from '@sentry/core'; import { getSentryRelease } from '@sentry/node'; import type { SentryVitePluginOptions } from '@sentry/vite-plugin'; import { sentryVitePlugin } from '@sentry/vite-plugin'; @@ -27,6 +27,8 @@ type Sorcery = { // and we only want to generate a uuid once in case we have to fall back to it. const releaseName = detectSentryRelease(); +type FilesToDeleteAfterUpload = string | string[] | undefined; + /** * Creates a new Vite plugin that uses the unplugin-based Sentry Vite plugin to create * releases and upload source maps to Sentry. @@ -60,8 +62,12 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug }, }; - const { promise: filesToDeleteAfterUpload, resolve: resolveFilesToDeleteAfterUpload } = - createFilesToDeleteAfterUploadPromise(); + let _resolveFilesToDeleteAfterUpload: + | undefined + | ((value: FilesToDeleteAfterUpload | Promise) => void); + const filesToDeleteAfterUploadPromise = new Promise(resolve => { + _resolveFilesToDeleteAfterUpload = resolve; + }); const mergedOptions = { ...defaultPluginOptions, @@ -72,7 +78,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug }, sourcemaps: { ...options?.sourcemaps, - filesToDeleteAfterUpload, + filesToDeleteAfterUpload: filesToDeleteAfterUploadPromise, }, }; @@ -100,7 +106,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug ); // resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise - resolveFilesToDeleteAfterUpload(undefined); + _resolveFilesToDeleteAfterUpload?.(undefined); return sentryPlugins; } @@ -113,7 +119,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug ); // resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise - resolveFilesToDeleteAfterUpload(undefined); + _resolveFilesToDeleteAfterUpload?.(undefined); return sentryPlugins; } @@ -126,7 +132,7 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug ); // resolving filesToDeleteAfterUpload here, because we return the original deletion plugin which awaits the promise - resolveFilesToDeleteAfterUpload(undefined); + _resolveFilesToDeleteAfterUpload?.(undefined); return sentryPlugins; } @@ -153,64 +159,40 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug name: 'sentry-sveltekit-update-source-map-setting-plugin', apply: 'build', // only apply this plugin at build time config: async (config: UserConfig) => { - const settingKey = 'build.sourcemap'; - - const { updatedSourceMapSetting, previousSourceMapSetting } = getUpdatedSourceMapSetting(config); - - const userProvidedFilesToDeleteAfterUpload = await options?.sourcemaps?.filesToDeleteAfterUpload; - - if (previousSourceMapSetting === 'unset') { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log(`[Sentry] Enabled source map generation in the build options with \`${settingKey}: "hidden"\`.`); - }); - - if (userProvidedFilesToDeleteAfterUpload) { - resolveFilesToDeleteAfterUpload(userProvidedFilesToDeleteAfterUpload); - } else { - // Including all hidden (`.*`) directories by default so that folders like .vercel, - // .netlify, etc are also cleaned up. Additionally, we include the adapter output - // dir which could be a non-hidden directory, like `build` for the Node adapter. - const defaultFileDeletionGlob = ['./.*/**/*.map', `./${adapterOutputDir}/**/*.map`]; - - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] Automatically setting \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: [${defaultFileDeletionGlob - .map(file => `"${file}"`) - .join(', ')}]\` to delete generated source maps after they were uploaded to Sentry.`, - ); - }); + return { + ...config, + build: { + ...config.build, + sourcemap: _getUpdatedSourceMapSettings(config, options), + }, + }; + }, + }; - // In case we enabled source maps and users didn't specify a glob patter to delete, we set a default pattern: - resolveFilesToDeleteAfterUpload(defaultFileDeletionGlob); - } + const filesToDeleteAfterUploadConfigPlugin: Plugin = { + name: 'sentry-sveltekit-files-to-delete-after-upload-setting-plugin', + apply: 'build', // only apply this plugin at build time + config: (config: UserConfig) => { + const originalFilesToDeleteAfterUpload = options?.sourcemaps?.filesToDeleteAfterUpload; - return { - ...config, - build: { ...config.build, sourcemap: updatedSourceMapSetting }, - }; - } + if (typeof originalFilesToDeleteAfterUpload === 'undefined' && typeof config.build?.sourcemap === 'undefined') { + // Including all hidden (`.*`) directories by default so that folders like .vercel, + // .netlify, etc are also cleaned up. Additionally, we include the adapter output + // dir which could be a non-hidden directory, like `build` for the Node adapter. + const defaultFileDeletionGlob = ['./.*/**/*.map', `./${adapterOutputDir}/**/*.map`]; - if (previousSourceMapSetting === 'disabled') { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - `[Sentry] Parts of source map generation are currently disabled in your Vite configuration (\`${settingKey}: false\`). This setting is either a default setting or was explicitly set in your configuration. Sentry won't override this setting. Without source maps, code snippets on the Sentry Issues page will remain minified. To show unminified code, enable source maps in \`${settingKey}\` (e.g. by setting them to \`hidden\`).`, + debug && + // eslint-disable-next-line no-console + console.info( + `[Sentry] Automatically setting \`sourceMapsUploadOptions.sourcemaps.filesToDeleteAfterUpload: [${defaultFileDeletionGlob + .map(file => `"${file}"`) + .join(', ')}]\` to delete generated source maps after they were uploaded to Sentry.`, ); - }); - } else if (previousSourceMapSetting === 'enabled') { - if (mergedOptions?.debug) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.log( - `[Sentry] We discovered you enabled source map generation in your Vite configuration (\`${settingKey}\`). Sentry will keep this source map setting. This will un-minify the code snippet on the Sentry Issue page.`, - ); - }); - } - } - resolveFilesToDeleteAfterUpload(userProvidedFilesToDeleteAfterUpload); + _resolveFilesToDeleteAfterUpload?.(defaultFileDeletionGlob); + } else { + _resolveFilesToDeleteAfterUpload?.(originalFilesToDeleteAfterUpload); + } return config; }, @@ -390,18 +372,14 @@ export async function makeCustomSentryVitePlugins(options?: CustomSentryVitePlug return [ ...unchangedSentryVitePlugins, sourceMapSettingsPlugin, + filesToDeleteAfterUploadConfigPlugin, customReleaseManagementPlugin, customDebugIdUploadPlugin, customFileDeletionPlugin, ]; } -/** - * Whether the user enabled (true, 'hidden', 'inline') or disabled (false) source maps - */ -type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined; - -/** There are 3 ways to set up source map generation (https://github.com/getsentry/sentry-javascript/issues/13993) +/** There are 3 ways to set up source map generation (https://github.com/getsentry/sentry-j avascript/issues/13993) * * 1. User explicitly disabled source maps * - keep this setting (emit a warning that errors won't be unminified in Sentry) @@ -416,30 +394,50 @@ type UserSourceMapSetting = 'enabled' | 'disabled' | 'unset' | undefined; * * --> only exported for testing */ -export function getUpdatedSourceMapSetting(viteConfig: { - build?: { - sourcemap?: boolean | 'inline' | 'hidden'; - }; -}): { updatedSourceMapSetting: boolean | 'inline' | 'hidden'; previousSourceMapSetting: UserSourceMapSetting } { +export function _getUpdatedSourceMapSettings( + viteConfig: UserConfig, + sentryPluginOptions?: CustomSentryVitePluginOptions, +): boolean | 'inline' | 'hidden' { viteConfig.build = viteConfig.build || {}; - const originalSourcemapSetting = viteConfig.build.sourcemap; + const viteSourceMap = viteConfig?.build?.sourcemap; + let updatedSourceMapSetting = viteSourceMap; - if (originalSourcemapSetting === false) { - return { - previousSourceMapSetting: 'disabled', - updatedSourceMapSetting: originalSourcemapSetting, - }; - } + const settingKey = 'build.sourcemap'; + const debug = sentryPluginOptions?.debug; + + if (viteSourceMap === false) { + updatedSourceMapSetting = viteSourceMap; + + if (debug) { + // Longer debug message with more details + // eslint-disable-next-line no-console + console.warn( + `[Sentry] Source map generation is currently disabled in your Vite configuration (\`${settingKey}: false \`). This setting is either a default setting or was explicitly set in your configuration. Sentry won't override this setting. Without source maps, code snippets on the Sentry Issues page will remain minified. To show unminified code, enable source maps in \`${settingKey}\` (e.g. by setting them to \`hidden\`).`, + ); + } else { + // eslint-disable-next-line no-console + console.warn('[Sentry] Source map generation is disabled in your Vite configuration.'); + } + } else if (viteSourceMap && ['hidden', 'inline', true].includes(viteSourceMap)) { + updatedSourceMapSetting = viteSourceMap; + + debug && + // eslint-disable-next-line no-console + console.log( + `[Sentry] We discovered \`${settingKey}\` is set to \`${viteSourceMap.toString()}\`. Sentry will keep this source map setting. This will un-minify the code snippet on the Sentry Issue page.`, + ); + } else { + updatedSourceMapSetting = 'hidden'; - if (originalSourcemapSetting && ['hidden', 'inline', true].includes(originalSourcemapSetting)) { - return { previousSourceMapSetting: 'enabled', updatedSourceMapSetting: originalSourcemapSetting }; + debug && + // eslint-disable-next-line no-console + console.log( + `[Sentry] Enabled source map generation in the build options with \`${settingKey}: 'hidden'\`. The source maps will be deleted after they were uploaded to Sentry.`, + ); } - return { - previousSourceMapSetting: 'unset', - updatedSourceMapSetting: 'hidden', - }; + return updatedSourceMapSetting; } function getFiles(dir: string): string[] { @@ -475,22 +473,3 @@ function detectSentryRelease(): string { return release; } - -/** - * Creates a deferred promise that can be resolved/rejected by calling the - * `resolve` or `reject` function. - * Inspired by: https://stackoverflow.com/a/69027809 - */ -function createFilesToDeleteAfterUploadPromise(): { - promise: Promise; - resolve: (value: string | string[] | undefined) => void; - reject: (reason?: unknown) => void; -} { - let resolve!: (value: string | string[] | undefined) => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - return { resolve, reject, promise }; -} diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 36c79afffb06..d21bcf0d8d04 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -42,7 +42,7 @@ describe('sentrySvelteKit()', () => { expect(plugins).toBeInstanceOf(Array); // 1 auto instrument plugin + 5 source maps plugins - expect(plugins).toHaveLength(8); + expect(plugins).toHaveLength(9); }); it('returns the custom sentry source maps upload plugin, unmodified sourcemaps plugins and the auto-instrument plugin by default', async () => { @@ -56,6 +56,7 @@ describe('sentrySvelteKit()', () => { 'sentry-vite-release-injection-plugin', 'sentry-vite-debug-id-injection-plugin', 'sentry-sveltekit-update-source-map-setting-plugin', + 'sentry-sveltekit-files-to-delete-after-upload-setting-plugin', // custom release plugin: 'sentry-sveltekit-release-management-plugin', // custom source maps plugin: @@ -86,7 +87,7 @@ describe('sentrySvelteKit()', () => { it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => { const plugins = await getSentrySvelteKitPlugins({ autoInstrument: false }); const pluginNames = plugins.map(plugin => plugin.name); - expect(plugins).toHaveLength(7); + expect(plugins).toHaveLength(8); expect(pluginNames).not.toContain('sentry-upload-source-maps'); }); @@ -184,7 +185,7 @@ describe('sentrySvelteKit()', () => { // just to ignore the source maps plugin: autoUploadSourceMaps: false, }); - const plugin = plugins[0]; + const plugin = plugins[0]!; expect(plugin.name).toEqual('sentry-auto-instrumentation'); expect(makePluginSpy).toHaveBeenCalledWith({ diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index b85d4e4db2cb..97d223dc309b 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -1,7 +1,9 @@ +import { sentryVitePlugin } from '@sentry/vite-plugin'; import type { Plugin } from 'vite'; import * as vite from 'vite'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { getUpdatedSourceMapSetting, makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps'; +import type { ViteUserConfig } from 'vitest/config'; +import { _getUpdatedSourceMapSettings, makeCustomSentryVitePlugins } from '../../src/vite/sourceMaps'; const mockedViteDebugIdUploadPlugin = { name: 'sentry-vite-debug-id-upload-plugin', @@ -18,25 +20,21 @@ const mockedFileDeletionPlugin = { writeBundle: vi.fn(), }; -vi.mock('vite', async () => { - const original = (await vi.importActual('vite')) as any; +vi.mock('@sentry/vite-plugin', async () => { + const original = (await vi.importActual('@sentry/vite-plugin')) as any; return { ...original, - loadConfigFromFile: vi.fn(), + sentryVitePlugin: vi.fn(), }; }); -vi.mock('@sentry/vite-plugin', async () => { - const original = (await vi.importActual('@sentry/vite-plugin')) as any; +vi.mock('vite', async () => { + const original = (await vi.importActual('vite')) as any; return { ...original, - sentryVitePlugin: () => [ - mockedViteReleaseManagementPlugin, - mockedViteDebugIdUploadPlugin, - mockedFileDeletionPlugin, - ], + loadConfigFromFile: vi.fn(), }; }); @@ -65,6 +63,15 @@ async function getSentryViteSubPlugin(name: string): Promise } describe('makeCustomSentryVitePlugins()', () => { + beforeEach(() => { + // @ts-expect-error - this function exists! + sentryVitePlugin.mockReturnValue([ + mockedViteReleaseManagementPlugin, + mockedViteDebugIdUploadPlugin, + mockedFileDeletionPlugin, + ]); + }); + it('returns the custom sentry source maps plugin', async () => { const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); @@ -87,6 +94,7 @@ describe('makeCustomSentryVitePlugins()', () => { // @ts-expect-error - this global variable is set/accessed in src/vite/sourceMaps.ts globalThis._sentry_sourceMapSetting = undefined; }); + it('returns the custom sentry source maps plugin', async () => { const plugin = await getSentryViteSubPlugin('sentry-sveltekit-update-source-map-setting-plugin'); @@ -117,6 +125,8 @@ describe('makeCustomSentryVitePlugins()', () => { }); it('keeps source map generation settings when previously disabled', async () => { + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {}); + const originalConfig = { build: { sourcemap: false, assetsDir: 'assets' }, }; @@ -138,6 +148,10 @@ describe('makeCustomSentryVitePlugins()', () => { sourcemap: false, }, }); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + '[Sentry] Source map generation is disabled in your Vite configuration.', + ); }); it('enables source map generation with "hidden" when unset', async () => { @@ -201,8 +215,8 @@ describe('makeCustomSentryVitePlugins()', () => { throw new Error('test error'); }); - const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementationOnce(() => {}); - const consoleLogSpy = vi.spyOn(console, 'log').mockImplementationOnce(() => {}); + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); const plugin = await getSentryViteSubPlugin('sentry-sveltekit-debug-id-upload-plugin'); @@ -315,26 +329,225 @@ describe('makeCustomSentryVitePlugins()', () => { }); }); -describe('changeViteSourceMapSettings()', () => { - const cases = [ - { sourcemap: false, expectedSourcemap: false, expectedPrevious: 'disabled' }, - { sourcemap: 'hidden' as const, expectedSourcemap: 'hidden', expectedPrevious: 'enabled' }, - { sourcemap: 'inline' as const, expectedSourcemap: 'inline', expectedPrevious: 'enabled' }, - { sourcemap: true, expectedSourcemap: true, expectedPrevious: 'enabled' }, - { sourcemap: undefined, expectedSourcemap: 'hidden', expectedPrevious: 'unset' }, - ]; - - it.each(cases)( - 'handles vite source map setting `build.sourcemap: $sourcemap`', - async ({ sourcemap, expectedSourcemap, expectedPrevious }) => { - const viteConfig = { build: { sourcemap } }; - - const result = getUpdatedSourceMapSetting(viteConfig); - - expect(result).toEqual({ - updatedSourceMapSetting: expectedSourcemap, - previousSourceMapSetting: expectedPrevious, +describe('_getUpdatedSourceMapSettings', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + describe('when sourcemap is false', () => { + it('should keep sourcemap as false and show short warning when debug is disabled', () => { + const result = _getUpdatedSourceMapSettings({ build: { sourcemap: false } }); + + expect(result).toBe(false); + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledWith( + '[Sentry] Source map generation is disabled in your Vite configuration.', + ); + }); + + it('should keep sourcemap as false and show long warning when debug is enabled', () => { + const result = _getUpdatedSourceMapSettings({ build: { sourcemap: false } }, { debug: true }); + + expect(result).toBe(false); + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('[Sentry] Source map generation is currently disabled in your Vite configuration'), + ); + // eslint-disable-next-line no-console + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'This setting is either a default setting or was explicitly set in your configuration.', + ), + ); + }); + }); + + describe('when sourcemap is explicitly set to valid values', () => { + it.each([ + ['hidden', 'hidden'], + ['inline', 'inline'], + [true, true], + ] as ('inline' | 'hidden' | boolean)[][])('should keep sourcemap as %s when set to %s', (input, expected) => { + const result = _getUpdatedSourceMapSettings({ build: { sourcemap: input } }, { debug: true }); + + expect(result).toBe(expected); + // eslint-disable-next-line no-console + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining(`[Sentry] We discovered \`build.sourcemap\` is set to \`${input.toString()}\``), + ); + }); + }); + + describe('when sourcemap is undefined or invalid', () => { + it.each([[undefined], ['invalid'], ['something'], [null]])( + 'should set sourcemap to hidden when value is %s', + input => { + const result = _getUpdatedSourceMapSettings({ build: { sourcemap: input as any } }, { debug: true }); + + expect(result).toBe('hidden'); + // eslint-disable-next-line no-console + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + "[Sentry] Enabled source map generation in the build options with `build.sourcemap: 'hidden'`", + ), + ); + }, + ); + + it('should set sourcemap to hidden when build config is empty', () => { + const result = _getUpdatedSourceMapSettings({}, { debug: true }); + + expect(result).toBe('hidden'); + // eslint-disable-next-line no-console + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + "[Sentry] Enabled source map generation in the build options with `build.sourcemap: 'hidden'`", + ), + ); + }); + }); +}); + +describe('deleteFilesAfterUpload', () => { + it('works with defauts', async () => { + const viteConfig: ViteUserConfig = {}; + + vi.mock('@sentry/vite-plugin', async () => { + const original = (await vi.importActual('@sentry/vite-plugin')) as any; + + return { + ...original, + sentryVitePlugin: vi.fn(original.sentryVitePlugin), + }; + }); + + const plugins = await makeCustomSentryVitePlugins({ + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + }); + + // @ts-expect-error this function exists! + const mergedOptions = sentryVitePlugin.mock.calls[0][0]; + + expect(mergedOptions).toEqual({ + _metaOptions: { + telemetry: { + metaFramework: 'sveltekit', + }, + }, + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + release: { + name: expect.any(String), + }, + sourcemaps: { + filesToDeleteAfterUpload: expect.any(Promise), + }, + }); + + const sourceMapSettingPlugin = plugins.find( + plugin => plugin.name === 'sentry-sveltekit-update-source-map-setting-plugin', + )!; + + // @ts-expect-error this function exists! + const sourceMapSettingConfig = await sourceMapSettingPlugin.config(viteConfig); + expect(sourceMapSettingConfig).toEqual({ build: { sourcemap: 'hidden' } }); + + const filesToDeleteAfterUploadSettingPlugin = plugins.find( + plugin => plugin.name === 'sentry-sveltekit-files-to-delete-after-upload-setting-plugin', + )!; + + // call this to ensure the filesToDeleteAfterUpload setting is resolved + // @ts-expect-error this function exists! + await filesToDeleteAfterUploadSettingPlugin.config(viteConfig); + + await expect(mergedOptions.sourcemaps.filesToDeleteAfterUpload).resolves.toEqual([ + './.*/**/*.map', + './.svelte-kit/output/**/*.map', + ]); + }); + + it.each([ + [['blub/'], undefined, 'hidden', ['blub/']], + [['blub/'], false, false, ['blub/']], + [undefined, 'hidden' as const, 'hidden', undefined], + [undefined, false, false, undefined], + [undefined, true, true, undefined], + [['/blub/'], true, true, ['/blub/']], + ])( + 'works with filesToDeleteAfterUpload: %j & sourcemap: %s', + async (filesToDeleteAfterUpload, sourcemap, sourcemapExpected, filesToDeleteAfterUploadExpected) => { + const viteConfig: ViteUserConfig = { + build: { + sourcemap, + }, + }; + + vi.mock('@sentry/vite-plugin', async () => { + const original = (await vi.importActual('@sentry/vite-plugin')) as any; + + return { + ...original, + sentryVitePlugin: vi.fn(original.sentryVitePlugin), + }; + }); + + const plugins = await makeCustomSentryVitePlugins({ + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + sourcemaps: { + filesToDeleteAfterUpload, + }, + }); + + // @ts-expect-error this function exists! + const mergedOptions = sentryVitePlugin.mock.calls[0][0]; + + expect(mergedOptions).toEqual({ + _metaOptions: { + telemetry: { + metaFramework: 'sveltekit', + }, + }, + authToken: 'token', + org: 'org', + project: 'project', + adapter: 'other', + release: { + name: expect.any(String), + }, + sourcemaps: { + filesToDeleteAfterUpload: expect.any(Promise), + }, }); + + const sourceMapSettingPlugin = plugins.find( + plugin => plugin.name === 'sentry-sveltekit-update-source-map-setting-plugin', + )!; + + // @ts-expect-error this function exists! + const sourceMapSettingConfig = await sourceMapSettingPlugin.config(viteConfig); + expect(sourceMapSettingConfig).toEqual({ build: { sourcemap: sourcemapExpected } }); + + const filesToDeleteAfterUploadSettingPlugin = plugins.find( + plugin => plugin.name === 'sentry-sveltekit-files-to-delete-after-upload-setting-plugin', + )!; + + // call this to ensure the filesToDeleteAfterUpload setting is resolved + // @ts-expect-error this function exists! + await filesToDeleteAfterUploadSettingPlugin.config(viteConfig); + + await expect(mergedOptions.sourcemaps.filesToDeleteAfterUpload).resolves.toEqual( + filesToDeleteAfterUploadExpected, + ); }, ); }); From 08fb9328fecbb83428b177fcd8f2032226cf52c0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 7 Aug 2025 13:22:55 +0200 Subject: [PATCH 04/10] fix(react-router): Ensure source map upload fails silently if Sentry CLI fails (#17081) Sentry CLI's `cli.releases.uploadSourceMaps` method previously never rejected when the actual CLI binary execution exited with an error code. In CLI 2.49.0 and 2.50.0 I added a new execution mode variant (`rejectOnError`) which continues to pipe stdio to the process (the RR SDKs' upload script) but now also rejects on error. This patch bumps Sentry CLI and configures it to actually reject now. We already catch the rejection and fail silently. Nothing changed here. --- .../src/vite/buildEnd/handleOnBuildEnd.ts | 6 ++- .../vite/buildEnd/handleOnBuildEnd.test.ts | 7 +++ yarn.lock | 43 ++++++++++++++++++- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts b/packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts index 5937d156bbba..959578b6d644 100644 --- a/packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts +++ b/packages/react-router/src/vite/buildEnd/handleOnBuildEnd.ts @@ -59,7 +59,10 @@ export const sentryOnBuildEnd: BuildEndHook = async ({ reactRouterConfig, viteCo if (sourceMapsUploadOptions?.enabled ?? (true && viteConfig.build.sourcemap !== false)) { // inject debugIds try { - await cliInstance.execute(['sourcemaps', 'inject', reactRouterConfig.buildDirectory], debug); + await cliInstance.execute( + ['sourcemaps', 'inject', reactRouterConfig.buildDirectory], + debug ? 'rejectOnError' : false, + ); } catch (error) { // eslint-disable-next-line no-console console.error('[Sentry] Could not inject debug ids', error); @@ -73,6 +76,7 @@ export const sentryOnBuildEnd: BuildEndHook = async ({ reactRouterConfig, viteCo paths: [reactRouterConfig.buildDirectory], }, ], + live: 'rejectOnError', }); } catch (error) { // eslint-disable-next-line no-console diff --git a/packages/react-router/test/vite/buildEnd/handleOnBuildEnd.test.ts b/packages/react-router/test/vite/buildEnd/handleOnBuildEnd.test.ts index 21d8616e5e44..29bb5c0527d6 100644 --- a/packages/react-router/test/vite/buildEnd/handleOnBuildEnd.test.ts +++ b/packages/react-router/test/vite/buildEnd/handleOnBuildEnd.test.ts @@ -147,8 +147,10 @@ describe('sentryOnBuildEnd', () => { await sentryOnBuildEnd(config); + expect(mockSentryCliInstance.releases.uploadSourceMaps).toHaveBeenCalledTimes(1); expect(mockSentryCliInstance.releases.uploadSourceMaps).toHaveBeenCalledWith('undefined', { include: [{ paths: ['/build'] }], + live: 'rejectOnError', }); }); @@ -249,6 +251,8 @@ describe('sentryOnBuildEnd', () => { mockSentryCliInstance.execute.mockRejectedValueOnce(new Error('Injection failed')); await sentryOnBuildEnd(defaultConfig); + expect(mockSentryCliInstance.execute).toHaveBeenCalledTimes(1); + expect(mockSentryCliInstance.execute).toHaveBeenCalledWith(['sourcemaps', 'inject', '/build'], false); expect(consoleSpy).toHaveBeenCalledWith('[Sentry] Could not inject debug ids', expect.any(Error)); consoleSpy.mockRestore(); @@ -282,6 +286,9 @@ describe('sentryOnBuildEnd', () => { expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('[Sentry] Automatically setting')); expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Deleting asset after upload:')); + // rejectOnError is used in debug mode to pipe debug id injection output from the CLI to this process's stdout + expect(mockSentryCliInstance.execute).toHaveBeenCalledWith(['sourcemaps', 'inject', '/build'], 'rejectOnError'); + consoleSpy.mockRestore(); }); diff --git a/yarn.lock b/yarn.lock index 7fe57741e388..a79397ae7beb 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6924,7 +6924,7 @@ mitt "^3.0.0" "@sentry-internal/test-utils@link:dev-packages/test-utils": - version "10.1.0" + version "10.2.0" dependencies: express "^4.21.1" @@ -6966,41 +6966,81 @@ magic-string "0.30.8" unplugin "1.0.1" +"@sentry/cli-darwin@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.49.0.tgz#290657e5840b360cb8ca25c8a78f8c0f15c66b03" + integrity sha512-bgowyDeFuXbjkGq1ZKqcWhmzgfBe7oKIXYWJOOps4+32QfG+YsrdNnottHS01td3bzrJq0QnHj8H12fA81DqrA== + "@sentry/cli-darwin@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-darwin/-/cli-darwin-2.50.2.tgz#fcf924fcc02cfa54748ff07a380334e533635c74" integrity sha512-0Pjpl0vQqKhwuZm19z6AlEF+ds3fJg1KWabv8WzGaSc/fwxMEwjFwOZj+IxWBJPV578cXXNvB39vYjjpCH8j7A== +"@sentry/cli-linux-arm64@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.49.0.tgz#a732004d7131f7e7b44f6a64abdccc36efb35d52" + integrity sha512-dqxsDUd76aDm03fUwUOs5BR7RHLpSb2EH/B1hlWm0mFvo9uY907XxW9wDFx/qDpCdmpC0aF+lF/lOBOrG9B5Fg== + "@sentry/cli-linux-arm64@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.50.2.tgz#ac9e6dba42095832bac8084abab4b86fdd2956f3" integrity sha512-03Cj215M3IdoHAwevCxm5oOm9WICFpuLR05DQnODFCeIUsGvE1pZsc+Gm0Ky/ZArq2PlShBJTpbHvXbCUka+0w== +"@sentry/cli-linux-arm@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.49.0.tgz#73719561510df3369e05e9a4898b4e43b8753e4c" + integrity sha512-RBDIjIGmNsFw+a6vAt6m3D7ROKsMEB9i3u+UuIRxk0/DyHTcfVWxnK/ScPXGILM6PxQ2XOBfOKad0mmiDHBzZA== + "@sentry/cli-linux-arm@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-arm/-/cli-linux-arm-2.50.2.tgz#835acd53ca83f6be9fc0d3d85a3cd4c694051bce" integrity sha512-jzFwg9AeeuFAFtoCcyaDEPG05TU02uOy1nAX09c1g7FtsyQlPcbhI94JQGmnPzdRjjDmORtwIUiVZQrVTkDM7w== +"@sentry/cli-linux-i686@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.49.0.tgz#8d1bb1378251a3aa995cc4b56bd352fa12a84b66" + integrity sha512-gDAd5/vJbEhd4Waud0Cd8ZRqLEagDlOvWwNH3KB694EiHJUwzRSiTA1YUVMYGI8Z9UyEA1sKxARwm2Trv99BxA== + "@sentry/cli-linux-i686@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-i686/-/cli-linux-i686-2.50.2.tgz#72f0e4bc1c515754aa11225efce711a24fb53524" integrity sha512-J+POvB34uVyHbIYF++Bc/OCLw+gqKW0H/y/mY7rRZCiocgpk266M4NtsOBl6bEaurMx1D+BCIEjr4nc01I/rqA== +"@sentry/cli-linux-x64@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.49.0.tgz#7bf58fb7005c89fdde4e1262d5ed35e23065aceb" + integrity sha512-mbohGvPNhHjUciYNXzkt9TYUebTmxeAp9v9JfLSb/Soz6fubKwEHhpRJuz1zASxVWIR4PuqkePchqN5zhcLC0A== + "@sentry/cli-linux-x64@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-linux-x64/-/cli-linux-x64-2.50.2.tgz#d06f8ffd65871b1373a0d2228ab254d9456a615c" integrity sha512-81yQVRLj8rnuHoYcrM7QbOw8ubA3weiMdPtTxTim1s6WExmPgnPTKxLCr9xzxGJxFdYo3xIOhtf5JFpUX/3j4A== +"@sentry/cli-win32-arm64@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-arm64/-/cli-win32-arm64-2.49.0.tgz#2bf6dd911acbe3ddb02eec0afb4301bb8fb25b53" + integrity sha512-3zwvsp61EPpSuGpGdXY4JelVJmNEjoj4vn5m6EFoOtk7OUI5/VFqqR4wchjy9Hjm3Eh6MB5K+KTKXs4W2p18ng== + "@sentry/cli-win32-arm64@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-win32-arm64/-/cli-win32-arm64-2.50.2.tgz#4bd7a140367c17f77d621903cfe0914232108657" integrity sha512-QjentLGvpibgiZlmlV9ifZyxV73lnGH6pFZWU5wLeRiaYKxWtNrrHpVs+HiWlRhkwQ0mG1/S40PGNgJ20DJ3gA== +"@sentry/cli-win32-i686@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.49.0.tgz#32e31472ae6c5f69e538a4061d651937fcb8f14a" + integrity sha512-2oWaNl6z0BaOCAjM1Jxequfgjod3XO6wothxow4kA8e9+43JLhgarSdpwJPgQjcVyxjygwQ3/jKPdUFh0qNOmg== + "@sentry/cli-win32-i686@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-win32-i686/-/cli-win32-i686-2.50.2.tgz#1eb997cf780c396446cdd8e63c6d4309894465e8" integrity sha512-UkBIIzkQkQ1UkjQX8kHm/+e7IxnEhK6CdgSjFyNlxkwALjDWHJjMztevqAPz3kv4LdM6q1MxpQ/mOqXICNhEGg== +"@sentry/cli-win32-x64@2.49.0": + version "2.49.0" + resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.49.0.tgz#86aab38cb41f885914d7c99ceaab7b6ce52c72c6" + integrity sha512-dR4ulyrA6ZT7x7cg4Rwm0tcHf4TZz5QO6t1W1jX6uJ9n/U0bOSqSFZHNf/RryiUzQE1g8LBthOYyKGMkET6T8w== + "@sentry/cli-win32-x64@2.50.2": version "2.50.2" resolved "https://registry.yarnpkg.com/@sentry/cli-win32-x64/-/cli-win32-x64-2.50.2.tgz#1d0c106125b6dc87f3a598ac02519c699f17a6c0" @@ -28567,7 +28607,6 @@ stylus@0.59.0, stylus@^0.59.0: sucrase@^3.27.0, sucrase@^3.35.0, sucrase@getsentry/sucrase#es2020-polyfills: version "3.36.0" - uid fd682f6129e507c00bb4e6319cc5d6b767e36061 resolved "https://codeload.github.com/getsentry/sucrase/tar.gz/fd682f6129e507c00bb4e6319cc5d6b767e36061" dependencies: "@jridgewell/gen-mapping" "^0.3.2" From f25a3273f49dac6f9367cdaad159393e32a540d7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 7 Aug 2025 14:39:07 +0200 Subject: [PATCH 05/10] fix(browser): Handle data urls in errors caught by `globalHandlersIntegration` (#17216) [#17218](https://github.com/getsentry/sentry-javascript/pull/17218) adjusted data URI stack line parsing for most errors that go through the stack parsers (fully for node, line truncation in general). This patch now applies a similar logic to `globalHandlersIntegration` which in some conditions applies a stack frame that doesn't go through the same stack parser. --- .../globalHandlers/dataUrls/subject.js | 11 +++++ .../globalHandlers/dataUrls/test.ts | 41 +++++++++++++++++++ .../src/integrations/globalhandlers.ts | 19 ++++++++- 3 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/subject.js b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/subject.js new file mode 100644 index 000000000000..0c22e79964df --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/subject.js @@ -0,0 +1,11 @@ +const workerCode = ` + self.addEventListener('message', (event) => { + if (event.data.type === 'error') { + throw new Error('Error thrown in worker'); + } + }); +`; + +const worker = new Worker(`data:text/javascript;base64,${btoa(workerCode)}`); + +worker.postMessage({ type: 'error' }); diff --git a/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/test.ts b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/test.ts new file mode 100644 index 000000000000..e890ea128069 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/globalHandlers/dataUrls/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers'; + +/** + * Tests a special case where the `globalHandlersIntegration` itself creates a stack frame instead of using + * stack parsers. This is necessary because we don't always get an `error` object passed to `window.onerror`. + * @see `globalhandlers.ts#_enhanceEventWithInitialFrame` + */ +sentryTest('detects and handles data urls on first stack frame', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const errorEventPromise = waitForErrorRequest(page, e => { + return !!e.exception?.values; + }); + + await page.goto(url); + + const errorEvent = envelopeRequestParser(await errorEventPromise); + + expect(errorEvent?.exception?.values?.[0]).toEqual({ + mechanism: { + handled: false, + synthetic: true, + type: 'auto.browser.global_handlers.onerror', + }, + stacktrace: { + frames: [ + { + colno: expect.any(Number), // webkit reports different colno than chromium + filename: '', + function: '?', + in_app: true, + lineno: 4, + }, + ], + }, + type: 'Error', + value: expect.stringMatching(/(Uncaught )?Error: Error thrown in worker/), // webikt throws without "Uncaught " + }); +}); diff --git a/packages/browser/src/integrations/globalhandlers.ts b/packages/browser/src/integrations/globalhandlers.ts index ebfdff6d2f58..2aa29731a9b0 100644 --- a/packages/browser/src/integrations/globalhandlers.ts +++ b/packages/browser/src/integrations/globalhandlers.ts @@ -171,7 +171,7 @@ function _enhanceEventWithInitialFrame( const colno = column; const lineno = line; - const filename = isString(url) && url.length > 0 ? url : getLocationHref(); + const filename = getFilenameFromUrl(url) ?? getLocationHref(); // event.exception.values[0].stacktrace.frames if (ev0sf.length === 0) { @@ -199,3 +199,20 @@ function getOptions(): { stackParser: StackParser; attachStacktrace?: boolean } }; return options; } + +function getFilenameFromUrl(url: string | undefined): string | undefined { + if (!isString(url) || url.length === 0) { + return undefined; + } + + // stack frame urls can be data urls, for example when initializing a Worker with a base64 encoded script + // in this case we just show the data prefix and mime type to avoid too long raw data urls + if (url.startsWith('data:')) { + const match = url.match(/^data:([^;]+)/); + const mimeType = match ? match[1] : 'text/javascript'; + const isBase64 = url.includes('base64,'); + return ``; + } + + return url.slice(0, 1024); +} From b4aad4931d78e78e36e242aa6361e8e0bf7d7f59 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 7 Aug 2025 17:24:50 +0200 Subject: [PATCH 06/10] feat(bun): Export `skipOpenTelemetrySetup` option (#17349) As raised in #16969, the Bun SDK doesn't currently export `skipOpenTelemetrySetup` in the SDK init options type, altghough the option can be correctly passed to the underlying `NodeClient`. This patch exports the option now. --- packages/bun/src/types.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/bun/src/types.ts b/packages/bun/src/types.ts index 45b15f80e517..755eb3e48a0a 100644 --- a/packages/bun/src/types.ts +++ b/packages/bun/src/types.ts @@ -23,6 +23,18 @@ export interface BaseBunOptions { /** Sets an optional server name (device name) */ serverName?: string; + /** + * If this is set to true, the SDK will not set up OpenTelemetry automatically. + * In this case, you _have_ to ensure to set it up correctly yourself, including: + * * The `SentrySpanProcessor` + * * The `SentryPropagator` + * * The `SentryContextManager` + * * The `SentrySampler` + * + * If you are registering your own OpenTelemetry Loader Hooks (or `import-in-the-middle` hooks), it is also recommended to set the `registerEsmLoaderHooks` option to false. + */ + skipOpenTelemetrySetup?: boolean; + /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; } From 735c1d8f143212f2e96fdc175c452f96ca2ca582 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 7 Aug 2025 18:25:31 +0200 Subject: [PATCH 07/10] fix(browser): Improve navigation vs. redirect detection (#17275) - Increase the threshold of inactivity between potential navigations from 300ms to 1.5s - Change how we treat interactions: - Previously, we'd take interactions happening _before_ the last root span was started into account - Now, we reset the interaction timestamp when we start a new navigation _root_ span. So from that moment on, we'll consider every new navigation span a redirect iff: - no interaction happened within 1.5s (from root span start) - the new span was started within 1.5s - there is still an active root span --- .../navigation-aborting-pageload/init.js | 2 +- .../keypress-early/init.js | 23 ------ .../keypress-early/test.ts | 42 ---------- .../multiple-redirects/init.js | 36 +++++++++ .../template.html | 0 .../multiple-redirects/test.ts | 78 +++++++++++++++++++ .../navigation-navigation-click/init.js | 27 +++++++ .../navigation-navigation-click/template.html | 8 ++ .../navigation-navigation-click/test.ts | 47 +++++++++++ .../init.js | 7 ++ .../template.html | 1 + .../navigation-navigation-keypress/test.ts | 54 +++++++++++++ .../navigation-redirect/init.js | 20 +++++ .../template.html | 0 .../test.ts | 4 +- .../navigation-redirect/opt-out/test.ts | 2 +- .../init.js | 0 .../subject.js | 0 .../template.html | 0 .../test.ts | 6 +- .../init.js | 0 .../subject.js | 0 .../template.html | 7 ++ .../test.ts | 2 +- .../init.js | 2 +- .../test.ts | 2 +- .../init.js | 0 .../test.ts | 8 +- .../src/tracing/browserTracingIntegration.ts | 12 ++- .../tracing/browserTracingIntegration.test.ts | 4 +- 30 files changed, 310 insertions(+), 84 deletions(-) delete mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/init.js delete mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/init.js rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click => multiple-redirects}/template.html (100%) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/test.ts rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click-early => navigation-navigation-keypress}/init.js (74%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{keypress-early => navigation-navigation-keypress}/template.html (72%) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/init.js rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click-early => navigation-redirect}/template.html (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click-early => navigation-redirect}/test.ts (85%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click => pageload-navigation-click}/init.js (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click => pageload-navigation-click}/subject.js (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{keypress => pageload-navigation-click}/template.html (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{late => pageload-navigation-click}/test.ts (83%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{keypress => pageload-navigation-keypress}/init.js (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{keypress => pageload-navigation-keypress}/subject.js (100%) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/template.html rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{keypress => pageload-navigation-keypress}/test.ts (92%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{late => pageload-navigation-time}/init.js (96%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{click => pageload-navigation-time}/test.ts (91%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{immediately => pageload-redirect}/init.js (100%) rename dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/{immediately => pageload-redirect}/test.ts (85%) diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js index ad357eee8cc6..06caf2c2c239 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js @@ -4,7 +4,7 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserTracingIntegration({ idleTimeout: 2000 })], + integrations: [Sentry.browserTracingIntegration({ idleTimeout: 2000, detectRedirects: false })], tracesSampleRate: 1, }); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/init.js deleted file mode 100644 index 83abe7de1b7a..000000000000 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/init.js +++ /dev/null @@ -1,23 +0,0 @@ -import * as Sentry from '@sentry/browser'; - -window.Sentry = Sentry; - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [Sentry.browserTracingIntegration()], - tracesSampleRate: 1, - debug: true, -}); - -document.getElementById('btn1').addEventListener('click', () => { - // Trigger navigation later than click, so the last click is more than 300ms ago - setTimeout(() => { - window.history.pushState({}, '', '/sub-page'); - - // then trigger redirect inside of this navigation, which should be detected as a redirect - // because the last click was more than 300ms ago - setTimeout(() => { - window.history.pushState({}, '', '/sub-page-redirect'); - }, 100); - }, 400); -}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/test.ts deleted file mode 100644 index 7f1da4860719..000000000000 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/test.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { expect } from '@playwright/test'; -import { sentryTest } from '../../../../../utils/fixtures'; -import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; - -sentryTest( - 'should create a navigation.redirect span if a keypress happened more than 300ms before navigation', - async ({ getLocalTestUrl, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } - - const url = await getLocalTestUrl({ testDir: __dirname }); - - const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); - const navigationRequestPromise = waitForTransactionRequest( - page, - event => event.contexts?.trace?.op === 'navigation', - ); - - await page.goto(url); - - await pageloadRequestPromise; - - // Now trigger navigation, and then a redirect in the navigation - await page.focus('#btn1'); - await page.keyboard.press('Enter'); - - const navigationRequest = envelopeRequestParser(await navigationRequestPromise); - - expect(navigationRequest.contexts?.trace?.op).toBe('navigation'); - expect(navigationRequest.transaction).toEqual('/sub-page'); - - const spans = navigationRequest.spans || []; - - expect(spans).toContainEqual( - expect.objectContaining({ - op: 'navigation.redirect', - description: '/sub-page-redirect', - }), - ); - }, -); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/init.js new file mode 100644 index 000000000000..484a12682a2d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/init.js @@ -0,0 +1,36 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, +}); + +window.history.pushState({}, '', '/sub-page-redirect-1'); + +setTimeout(() => { + window.history.pushState({}, '', '/sub-page-redirect-2'); +}, 400); + +setTimeout(() => { + window.history.pushState({}, '', '/sub-page-redirect-3'); +}, 800); + +document.getElementById('btn1').addEventListener('click', () => { + window.history.pushState({}, '', '/next-page'); +}); + +setTimeout(() => { + document.getElementById('btn1').click(); + // 1s is still within the 1.5s time window, but the click should trigger a new navigation root span +}, 1000); + +setTimeout(() => { + window.history.pushState({}, '', '/next-page-redirect-1'); +}, 1100); + +setTimeout(() => { + window.history.pushState({}, '', '/next-page-redirect-2'); +}, 2000); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/template.html similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/template.html rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/template.html diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/test.ts new file mode 100644 index 000000000000..58be1f6ff9a4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/multiple-redirects/test.ts @@ -0,0 +1,78 @@ +import { expect } from '@playwright/test'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'creates a pageload and navigation root spans each with multiple navigation.redirect childspans', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + const navigationRequestPromise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/next-page', + ); + + await page.goto(url); + + const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise); + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + + expect(pageloadRequest.contexts?.trace?.op).toBe('pageload'); + + expect(pageloadRequest.contexts?.trace?.data).toMatchObject({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', + [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + ['sentry.idle_span_finish_reason']: 'cancelled', + }); + + expect(pageloadRequest.request).toEqual({ + headers: { + 'User-Agent': expect.any(String), + }, + url: 'http://sentry-test.io/index.html', + }); + + const spans = pageloadRequest.spans || []; + + const redirectSpans = spans.filter(span => span.op === 'navigation.redirect'); + expect(redirectSpans).toHaveLength(3); + + redirectSpans.forEach(redirectSpan => { + expect(redirectSpan?.timestamp).toEqual(redirectSpan?.start_timestamp); + expect(redirectSpan).toEqual({ + data: { + 'sentry.op': 'navigation.redirect', + 'sentry.origin': 'auto.navigation.browser', + 'sentry.source': 'url', + }, + description: expect.stringContaining('/sub-page-redirect-'), + op: 'navigation.redirect', + origin: 'auto.navigation.browser', + parent_span_id: pageloadRequest.contexts!.trace!.span_id, + span_id: expect.any(String), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: expect.any(String), + }); + }); + + expect(navigationRequest.contexts?.trace?.op).toBe('navigation'); + expect(navigationRequest.transaction).toEqual('/next-page'); + + // 2 subsequent redirects belonging to the navigation root span + expect(navigationRequest.spans?.filter(span => span.op === 'navigation.redirect')).toHaveLength(2); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/init.js new file mode 100644 index 000000000000..c3e90ce95430 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/init.js @@ -0,0 +1,27 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, + debug: true, +}); + +document.getElementById('btn1').addEventListener('click', () => { + window.history.pushState({}, '', '/sub-page'); + + // then trigger redirect inside of this navigation, which should not be detected as a redirect + // because the last click was less than 1.5s ago + setTimeout(() => { + document.getElementById('btn2').click(); + }, 100); +}); + +document.getElementById('btn2').addEventListener('click', () => { + setTimeout(() => { + // navigation happens ~1100ms after the last navigation + window.history.pushState({}, '', '/sub-page-2'); + }, 1000); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/template.html new file mode 100644 index 000000000000..d892beb72de0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/template.html @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/test.ts new file mode 100644 index 000000000000..a88987209bcd --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-click/test.ts @@ -0,0 +1,47 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'creates navigation root span if click happened within 1.5s of the last navigation', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + const navigationRequestPromise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page', + ); + const navigation2RequestPromise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page-2', + ); + + await page.goto(url); + + await pageloadRequestPromise; + + // Now trigger navigation (since no span is active), and then a redirect in the navigation, with + await page.click('#btn1'); + + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + const navigation2Request = envelopeRequestParser(await navigation2RequestPromise); + + expect(navigationRequest.contexts?.trace?.op).toBe('navigation'); + expect(navigationRequest.transaction).toEqual('/sub-page'); + + const spans = (navigationRequest.spans || []).filter(s => s.op === 'navigation.redirect'); + + expect(spans).toHaveLength(0); + + expect(navigation2Request.contexts?.trace?.op).toBe('navigation'); + expect(navigation2Request.transaction).toEqual('/sub-page-2'); + + const spans2 = (navigation2Request.spans || []).filter(s => s.op === 'navigation.redirect'); + expect(spans2).toHaveLength(0); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/init.js similarity index 74% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/init.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/init.js index 83abe7de1b7a..f9d8db46d3d9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/init.js @@ -21,3 +21,10 @@ document.getElementById('btn1').addEventListener('click', () => { }, 100); }, 400); }); + +document.getElementById('btn2').addEventListener('click', () => { + // Trigger navigation later than click, so the last click is more than 300ms ago + setTimeout(() => { + window.history.pushState({}, '', '/sub-page-2'); + }, 400); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/template.html similarity index 72% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/template.html rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/template.html index d364e6680b41..d18d7bb98e5c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress-early/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/template.html @@ -4,4 +4,5 @@ + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/test.ts new file mode 100644 index 000000000000..fe152feda462 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-navigation-keypress/test.ts @@ -0,0 +1,54 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../../utils/fixtures'; +import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; + +sentryTest( + 'creates a navigation root span if a keypress happened within the last 1.5s', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); + const navigationRequestPromise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page', + ); + + const navigationRequest2Promise = waitForTransactionRequest( + page, + event => event.contexts?.trace?.op === 'navigation' && event.transaction === '/sub-page-2', + ); + + await page.goto(url); + + await pageloadRequestPromise; + + await page.focus('#btn1'); + await page.keyboard.press('Enter'); + + await page.waitForTimeout(500); + + await page.focus('#btn2'); + await page.keyboard.press('Enter'); + + const navigationRequest = envelopeRequestParser(await navigationRequestPromise); + const navigationRequest2 = envelopeRequestParser(await navigationRequest2Promise); + + expect(navigationRequest.contexts?.trace?.op).toBe('navigation'); + expect(navigationRequest.transaction).toEqual('/sub-page'); + + const redirectSpans = navigationRequest.spans?.filter(span => span.op === 'navigation.redirect') || []; + expect(redirectSpans).toHaveLength(1); + + expect(redirectSpans[0].description).toEqual('/sub-page-redirect'); + + expect(navigationRequest2.contexts?.trace?.op).toBe('navigation'); + expect(navigationRequest2.transaction).toEqual('/sub-page-2'); + + const redirectSpans2 = navigationRequest2.spans?.filter(span => span.op === 'navigation.redirect') || []; + expect(redirectSpans2).toHaveLength(0); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/init.js new file mode 100644 index 000000000000..5cf45a1f0b06 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/init.js @@ -0,0 +1,20 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracesSampleRate: 1, + debug: true, +}); + +document.getElementById('btn1').addEventListener('click', () => { + window.history.pushState({}, '', '/sub-page'); + + // then trigger redirect inside of this navigation, which should be detected as a redirect + // because the last navigation was less than 1.5s ago + setTimeout(() => { + window.history.pushState({}, '', '/sub-page-redirect'); + }, 750); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/template.html similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/template.html rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/template.html diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/test.ts similarity index 85% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/test.ts rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/test.ts index 97cbc67c8af8..71fe35c8bcc9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click-early/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/navigation-redirect/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( - 'should create a navigation.redirect span if a click happened more than 300ms before navigation', + 'creates a navigation root span and redirect child span if no click happened within the last 1.5s', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); @@ -21,7 +21,7 @@ sentryTest( await pageloadRequestPromise; - // Now trigger navigation, and then a redirect in the navigation, with + // Now trigger navigation (since no span is active), and then a redirect in the navigation, with await page.click('#btn1'); const navigationRequest = envelopeRequestParser(await navigationRequestPromise); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/test.ts index e96e9e650122..a25cc01f086f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( - 'should not create a navigation.redirect span if `detectRedirects` is set to false', + "doesn't create a navigation.redirect span if `detectRedirects` is set to false", async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/init.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/init.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/init.js diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/subject.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/subject.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/subject.js diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/template.html similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/template.html rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/template.html diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/test.ts similarity index 83% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/test.ts rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/test.ts index f1108cdbc1c5..771b07afaba6 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-click/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( - 'should not create a navigation.redirect span if redirect happened more than 300ms after pageload', + "doesn't create a navigation.redirect span if a click happened before navigation", async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); @@ -23,9 +23,9 @@ sentryTest( // Ensure a navigation span is sent, too await navigationRequestPromise; - const spans = pageloadRequest.spans || []; + const pageloadTxnSpans = pageloadRequest.spans || []; - expect(spans).not.toContainEqual( + expect(pageloadTxnSpans).not.toContainEqual( expect.objectContaining({ op: 'navigation.redirect', }), diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/init.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/init.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/init.js diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/subject.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/subject.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/subject.js diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/template.html b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/template.html new file mode 100644 index 000000000000..316bec83030d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/template.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/test.ts similarity index 92% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/test.ts rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/test.ts index 8213d2723b08..e77e08936466 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/keypress/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-keypress/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( - 'should not create a navigation.redirect span if a keypress happened before navigation', + "doesn't create a navigation.redirect span if a keypress happened before navigation", async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/init.js similarity index 96% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/init.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/init.js index 686f72903a89..f317a550e2a7 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/late/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/init.js @@ -11,4 +11,4 @@ Sentry.init({ // trigger redirect later setTimeout(() => { window.history.pushState({}, '', '/sub-page'); -}, 400); +}, 1550); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/test.ts similarity index 91% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/test.ts rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/test.ts index 4a5cb9acd73b..6b5925f52cd1 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/click/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-navigation-time/test.ts @@ -3,7 +3,7 @@ import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; sentryTest( - 'should not create a navigation.redirect span if a click happened before navigation', + "doesn't create a navigation.redirect span if redirect happened more than 1.5s after pageload", async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/immediately/init.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-redirect/init.js similarity index 100% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/immediately/init.js rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-redirect/init.js diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/immediately/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-redirect/test.ts similarity index 85% rename from dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/immediately/test.ts rename to dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-redirect/test.ts index f2b3e885f6ce..1fa0a081fa85 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/immediately/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/pageload-redirect/test.ts @@ -8,7 +8,7 @@ import { import { sentryTest } from '../../../../../utils/fixtures'; import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; -sentryTest('should create a pageload transaction with navigation.redirect span', async ({ getLocalTestUrl, page }) => { +sentryTest('creates a pageload root span with navigation.redirect childspan', async ({ getLocalTestUrl, page }) => { if (shouldSkipTracingTest()) { sentryTest.skip(); } @@ -46,9 +46,9 @@ sentryTest('should create a pageload transaction with navigation.redirect span', }), ); - const navigationSpan = spans.find(span => span.op === 'navigation.redirect'); - expect(navigationSpan?.timestamp).toEqual(navigationSpan?.start_timestamp); - expect(navigationSpan).toEqual({ + const redirectSpan = spans.find(span => span.op === 'navigation.redirect'); + expect(redirectSpan?.timestamp).toEqual(redirectSpan?.start_timestamp); + expect(redirectSpan).toEqual({ data: { 'sentry.op': 'navigation.redirect', 'sentry.origin': 'auto.navigation.browser', diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 2a38f7afe7be..d2224f141797 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -504,6 +504,11 @@ export const browserTracingIntegration = ((_options: Partial never consider this a redirect const startTimestamp = spanData.start_timestamp; if (now - startTimestamp > REDIRECT_THRESHOLD) { return false; } - // A click happened in the last 300ms? + // A click happened in the last REDIRECT_THRESHOLD seconds? // --> never consider this a redirect if (lastInteractionTimestamp && now - lastInteractionTimestamp <= REDIRECT_THRESHOLD) { return false; diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index e39da8cdda39..84aea8b13c9e 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -154,7 +154,7 @@ describe('browserTracingIntegration', () => { expect(spanIsSampled(span!)).toBe(false); }); - it('starts navigation when URL changes after > 300ms', () => { + it('starts navigation when URL changes after > 1.5s', () => { const client = new BrowserClient( getDefaultBrowserClientOptions({ tracesSampleRate: 1, @@ -187,7 +187,7 @@ describe('browserTracingIntegration', () => { const dom = new JSDOM(undefined, { url: 'https://example.com/test' }); Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true }); - vi.advanceTimersByTime(400); + vi.advanceTimersByTime(1600); WINDOW.history.pushState({}, '', '/test'); expect(span!.isRecording()).toBe(false); From ccc7d32fc7a6a21ca541a3b562549a4fae17b3d1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 8 Aug 2025 09:56:45 +0200 Subject: [PATCH 08/10] ref(core): Improve event `mechanism` for supabase integration (#17286) see #17212 --- packages/core/src/integrations/supabase.ts | 23 +++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/core/src/integrations/supabase.ts b/packages/core/src/integrations/supabase.ts index 360f31142652..39dedadc9e04 100644 --- a/packages/core/src/integrations/supabase.ts +++ b/packages/core/src/integrations/supabase.ts @@ -11,6 +11,7 @@ import { setHttpStatus, SPAN_STATUS_ERROR, SPAN_STATUS_OK, startSpan } from '../ import type { IntegrationFn } from '../types-hoist/integration'; import { debug } from '../utils/debug-logger'; import { isPlainObject } from '../utils/is'; +import { addExceptionMechanism } from '../utils/misc'; const AUTH_OPERATIONS_TO_INSTRUMENT = [ 'reauthenticate', @@ -236,6 +237,7 @@ function instrumentAuthOperation(operation: AuthOperationFn, isAdmin = false): A captureException(res.error, { mechanism: { handled: false, + type: 'auto.db.supabase.auth', }, }); } else { @@ -252,6 +254,7 @@ function instrumentAuthOperation(operation: AuthOperationFn, isAdmin = false): A captureException(err, { mechanism: { handled: false, + type: 'auto.db.supabase.auth', }, }); @@ -408,7 +411,7 @@ function instrumentPostgRESTFilterBuilder(PostgRESTFilterBuilder: PostgRESTFilte err.details = res.error.details; } - const supabaseContext: Record = {}; + const supabaseContext: Record = {}; if (queryItems.length) { supabaseContext.query = queryItems; } @@ -416,10 +419,19 @@ function instrumentPostgRESTFilterBuilder(PostgRESTFilterBuilder: PostgRESTFilte supabaseContext.body = body; } - captureException(err, { - contexts: { - supabase: supabaseContext, - }, + captureException(err, scope => { + scope.addEventProcessor(e => { + addExceptionMechanism(e, { + handled: false, + type: 'auto.db.supabase.postgres', + }); + + return e; + }); + + scope.setContext('supabase', supabaseContext); + + return scope; }); } @@ -448,6 +460,7 @@ function instrumentPostgRESTFilterBuilder(PostgRESTFilterBuilder: PostgRESTFilte return res; }, (err: Error) => { + // TODO: shouldn't we capture this error? if (span) { setHttpStatus(span, 500); span.end(); From ca4fba675257852dba903168072ac089074a4455 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 8 Aug 2025 09:54:18 +0100 Subject: [PATCH 09/10] fix(react): Add support for React Router sub-routes from `handle` (#17277) --- .../react-router-7-lazy-routes/.gitignore | 29 ++ .../react-router-7-lazy-routes/.npmrc | 2 + .../react-router-7-lazy-routes/package.json | 53 +++ .../playwright.config.mjs | 7 + .../public/index.html | 24 ++ .../src/globals.d.ts | 5 + .../react-router-7-lazy-routes/src/index.tsx | 74 +++++ .../src/pages/Index.tsx | 14 + .../src/pages/InnerLazyRoutes.tsx | 37 +++ .../src/react-app-env.d.ts | 1 + .../start-event-proxy.mjs | 6 + .../tests/transactions.test.ts | 56 ++++ .../react-router-7-lazy-routes/tsconfig.json | 20 ++ .../react/src/reactrouterv6-compat-utils.tsx | 156 +++++++++ packages/react/src/types.ts | 2 + .../test/reactrouterv6-compat-utils.test.tsx | 306 +++++++++++++++++- 16 files changed, 790 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore new file mode 100644 index 000000000000..84634c973eeb --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.gitignore @@ -0,0 +1,29 @@ +# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. + +# dependencies +/node_modules +/.pnp +.pnp.js + +# testing +/coverage + +# production +/build + +# misc +.DS_Store +.env.local +.env.development.local +.env.test.local +.env.production.local + +npm-debug.log* +yarn-debug.log* +yarn-error.log* + +/test-results/ +/playwright-report/ +/playwright/.cache/ + +!*.d.ts diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json new file mode 100644 index 000000000000..1c38ac1468ec --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/package.json @@ -0,0 +1,53 @@ +{ + "name": "react-router-7-lazy-routes", + "version": "0.1.0", + "private": true, + "dependencies": { + "@sentry/react": "latest || *", + "@types/react": "18.0.0", + "@types/react-dom": "18.0.0", + "express": "4.20.0", + "react": "18.2.0", + "react-dom": "18.2.0", + "react-router-dom": "7.6.0", + "react-scripts": "5.0.1", + "typescript": "~5.0.0" + }, + "scripts": { + "build": "react-scripts build", + "start": "serve -s build", + "test": "playwright test", + "clean": "npx rimraf node_modules pnpm-lock.yaml", + "test:build": "pnpm install && npx playwright install && pnpm build", + "test:build-ts3.8": "pnpm install && pnpm add typescript@3.8 && npx playwright install && pnpm build", + "test:build-canary": "pnpm install && pnpm add react@canary react-dom@canary && npx playwright install && pnpm build", + "test:assert": "pnpm test" + }, + "eslintConfig": { + "extends": [ + "react-app", + "react-app/jest" + ] + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "devDependencies": { + "@playwright/test": "~1.53.2", + "@sentry-internal/test-utils": "link:../../../test-utils", + "serve": "14.0.1", + "npm-run-all2": "^6.2.0" + }, + "volta": { + "extends": "../../package.json" + } +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs new file mode 100644 index 000000000000..31f2b913b58b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/playwright.config.mjs @@ -0,0 +1,7 @@ +import { getPlaywrightConfig } from '@sentry-internal/test-utils'; + +const config = getPlaywrightConfig({ + startCommand: `pnpm start`, +}); + +export default config; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html new file mode 100644 index 000000000000..39da76522bea --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/public/index.html @@ -0,0 +1,24 @@ + + + + + + + + React App + + + +
+ + + diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts new file mode 100644 index 000000000000..ffa61ca49acc --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/globals.d.ts @@ -0,0 +1,5 @@ +interface Window { + recordedTransactions?: string[]; + capturedExceptionId?: string; + sentryReplayId?: string; +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx new file mode 100644 index 000000000000..4570c23d06f5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/index.tsx @@ -0,0 +1,74 @@ +import * as Sentry from '@sentry/react'; +import React from 'react'; +import ReactDOM from 'react-dom/client'; +import { + Navigate, + PatchRoutesOnNavigationFunction, + RouterProvider, + createBrowserRouter, + createRoutesFromChildren, + matchRoutes, + useLocation, + useNavigationType, +} from 'react-router-dom'; +import Index from './pages/Index'; + + +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.REACT_APP_E2E_TEST_DSN, + integrations: [ + Sentry.reactRouterV7BrowserTracingIntegration({ + useEffect: React.useEffect, + useLocation, + useNavigationType, + createRoutesFromChildren, + matchRoutes, + trackFetchStreamPerformance: true, + enableAsyncRouteHandlers: true, + }), + ], + // We recommend adjusting this value in production, or using tracesSampler + // for finer control + tracesSampleRate: 1.0, + release: 'e2e-test', + + tunnel: 'http://localhost:3031', +}); + +const sentryCreateBrowserRouter = Sentry.wrapCreateBrowserRouterV7(createBrowserRouter); + +const router = sentryCreateBrowserRouter( + [ + { + path: '/', + element: , + }, + { + path: '/lazy', + handle: { + lazyChildren: () => import('./pages/InnerLazyRoutes').then(module => module.someMoreNestedRoutes), + }, + }, + { + path: '/static', + element: <>Hello World, + }, + { + path: '*', + element: , + }, + ], + { + async patchRoutesOnNavigation({ matches, patch }: Parameters[0]) { + const leafRoute = matches[matches.length - 1]?.route; + if (leafRoute?.id && leafRoute?.handle?.lazyChildren) { + const children = await leafRoute.handle.lazyChildren(); + patch(leafRoute.id, children); + } + }, + }, +); + +const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); +root.render(); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx new file mode 100644 index 000000000000..e24b1b7cbff7 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/Index.tsx @@ -0,0 +1,14 @@ +import * as React from 'react'; +import { Link } from 'react-router-dom'; + +const Index = () => { + return ( + <> + + navigate + + + ); +}; + +export default Index; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx new file mode 100644 index 000000000000..e8385e54dab5 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/pages/InnerLazyRoutes.tsx @@ -0,0 +1,37 @@ +import React from 'react'; + +export const someMoreNestedRoutes = [ + { + path: 'inner', + children: [ + { + index: true, + element: <>Level 1, + }, + { + path: ':id', + children: [ + { + index: true, + element: <>Level 1 ID, + }, + { + path: ':anotherId', + children: [ + { + index: true, + element: <>Level 1 ID Another ID, + }, + { + path: ':someAnotherId', + element:
+ Rendered +
, + }, + ], + }, + ], + }, + ], + }, +]; diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts new file mode 100644 index 000000000000..6431bc5fc6b2 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/src/react-app-env.d.ts @@ -0,0 +1 @@ +/// diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs new file mode 100644 index 000000000000..5e5ac71f169b --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/start-event-proxy.mjs @@ -0,0 +1,6 @@ +import { startEventProxyServer } from '@sentry-internal/test-utils'; + +startEventProxyServer({ + port: 3031, + proxyServerName: 'react-router-7-lazy-routes', +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts new file mode 100644 index 000000000000..74a1fcff1faa --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/transactions.test.ts @@ -0,0 +1,56 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; + + +test('Creates a pageload transaction with parameterized route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'pageload' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + await page.goto('/lazy/inner/1/2/3'); + const event = await transactionPromise; + + + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + + await expect(lazyRouteContent).toBeVisible(); + + // Validate the transaction event + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('pageload'); +}); + +test('Creates a navigation transaction inside a lazy route', async ({ page }) => { + const transactionPromise = waitForTransaction('react-router-7-lazy-routes', async transactionEvent => { + return ( + !!transactionEvent?.transaction && + transactionEvent.contexts?.trace?.op === 'navigation' && + transactionEvent.transaction === '/lazy/inner/:id/:anotherId/:someAnotherId' + ); + }); + + await page.goto('/'); + + // Check if the navigation link exists + const navigationLink = page.locator('id=navigation'); + await expect(navigationLink).toBeVisible(); + + // Click the navigation link to navigate to the lazy route + await navigationLink.click(); + const event = await transactionPromise; + + // Check if the lazy route content is rendered + const lazyRouteContent = page.locator('id=innermost-lazy-route'); + + await expect(lazyRouteContent).toBeVisible(); + + // Validate the transaction event + expect(event.transaction).toBe('/lazy/inner/:id/:anotherId/:someAnotherId'); + expect(event.type).toBe('transaction'); + expect(event.contexts?.trace?.op).toBe('navigation'); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json new file mode 100644 index 000000000000..4cc95dc2689a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tsconfig.json @@ -0,0 +1,20 @@ +{ + "compilerOptions": { + "target": "es2018", + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": true, + "esModuleInterop": true, + "allowSyntheticDefaultImports": true, + "strict": true, + "forceConsistentCasingInFileNames": true, + "noFallthroughCasesInSwitch": true, + "module": "esnext", + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "noEmit": true, + "jsx": "react" + }, + "include": ["src", "tests"] +} diff --git a/packages/react/src/reactrouterv6-compat-utils.tsx b/packages/react/src/reactrouterv6-compat-utils.tsx index c711d9f3d613..f364facca1db 100644 --- a/packages/react/src/reactrouterv6-compat-utils.tsx +++ b/packages/react/src/reactrouterv6-compat-utils.tsx @@ -10,6 +10,7 @@ import { } from '@sentry/browser'; import type { Client, Integration, Span, TransactionSource } from '@sentry/core'; import { + addNonEnumerableProperty, debug, getActiveSpan, getClient, @@ -46,6 +47,7 @@ let _useNavigationType: UseNavigationType; let _createRoutesFromChildren: CreateRoutesFromChildren; let _matchRoutes: MatchRoutes; let _stripBasename: boolean = false; +let _enableAsyncRouteHandlers: boolean = false; const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet(); @@ -55,7 +57,20 @@ export interface ReactRouterOptions { useNavigationType: UseNavigationType; createRoutesFromChildren: CreateRoutesFromChildren; matchRoutes: MatchRoutes; + /** + * Whether to strip the basename from the pathname when creating transactions. + * + * This is useful for applications that use a basename in their routing setup. + * @default false + */ stripBasename?: boolean; + /** + * Enables support for async route handlers. + * + * This allows Sentry to track and instrument routes dynamically resolved from async handlers. + * @default false + */ + enableAsyncRouteHandlers?: boolean; } type V6CompatibleVersion = '6' | '7'; @@ -63,6 +78,130 @@ type V6CompatibleVersion = '6' | '7'; // Keeping as a global variable for cross-usage in multiple functions const allRoutes = new Set(); +/** + * Adds resolved routes as children to the parent route. + * Prevents duplicate routes by checking if they already exist. + */ +function addResolvedRoutesToParent(resolvedRoutes: RouteObject[], parentRoute: RouteObject): void { + const existingChildren = parentRoute.children || []; + + const newRoutes = resolvedRoutes.filter( + newRoute => + !existingChildren.some( + existing => + existing === newRoute || + (newRoute.path && existing.path === newRoute.path) || + (newRoute.id && existing.id === newRoute.id), + ), + ); + + if (newRoutes.length > 0) { + parentRoute.children = [...existingChildren, ...newRoutes]; + } +} + +/** + * Handles the result of an async handler function call. + */ +function handleAsyncHandlerResult(result: unknown, route: RouteObject, handlerKey: string): void { + if ( + result && + typeof result === 'object' && + 'then' in result && + typeof (result as Promise).then === 'function' + ) { + (result as Promise) + .then((resolvedRoutes: unknown) => { + if (Array.isArray(resolvedRoutes)) { + processResolvedRoutes(resolvedRoutes, route); + } + }) + .catch((e: unknown) => { + DEBUG_BUILD && debug.warn(`Error resolving async handler '${handlerKey}' for route`, route, e); + }); + } else if (Array.isArray(result)) { + processResolvedRoutes(result, route); + } +} + +/** + * Processes resolved routes by adding them to allRoutes and checking for nested async handlers. + */ +function processResolvedRoutes(resolvedRoutes: RouteObject[], parentRoute?: RouteObject): void { + resolvedRoutes.forEach(child => { + allRoutes.add(child); + // Only check for async handlers if the feature is enabled + if (_enableAsyncRouteHandlers) { + checkRouteForAsyncHandler(child); + } + }); + + if (parentRoute) { + // If a parent route is provided, add the resolved routes as children to the parent route + addResolvedRoutesToParent(resolvedRoutes, parentRoute); + } + + // After processing lazy routes, check if we need to update an active pageload transaction + const activeRootSpan = getActiveRootSpan(); + if (activeRootSpan && spanToJSON(activeRootSpan).op === 'pageload') { + const location = WINDOW.location; + if (location) { + // Re-run the pageload transaction update with the newly loaded routes + updatePageloadTransaction( + activeRootSpan, + { pathname: location.pathname }, + Array.from(allRoutes), + undefined, + undefined, + Array.from(allRoutes), + ); + } + } +} + +/** + * Creates a proxy wrapper for an async handler function. + */ +function createAsyncHandlerProxy( + originalFunction: (...args: unknown[]) => unknown, + route: RouteObject, + handlerKey: string, +): (...args: unknown[]) => unknown { + const proxy = new Proxy(originalFunction, { + apply(target: (...args: unknown[]) => unknown, thisArg, argArray) { + const result = target.apply(thisArg, argArray); + handleAsyncHandlerResult(result, route, handlerKey); + return result; + }, + }); + + addNonEnumerableProperty(proxy, '__sentry_proxied__', true); + + return proxy; +} + +/** + * Recursively checks a route for async handlers and sets up Proxies to add discovered child routes to allRoutes when called. + */ +export function checkRouteForAsyncHandler(route: RouteObject): void { + // Set up proxies for any functions in the route's handle + if (route.handle && typeof route.handle === 'object') { + for (const key of Object.keys(route.handle)) { + const maybeFn = route.handle[key]; + if (typeof maybeFn === 'function' && !(maybeFn as { __sentry_proxied__?: boolean }).__sentry_proxied__) { + route.handle[key] = createAsyncHandlerProxy(maybeFn, route, key); + } + } + } + + // Recursively check child routes + if (Array.isArray(route.children)) { + for (const child of route.children) { + checkRouteForAsyncHandler(child); + } + } +} + /** * Creates a wrapCreateBrowserRouter function that can be used with all React Router v6 compatible versions. */ @@ -85,6 +224,13 @@ export function createV6CompatibleWrapCreateBrowserRouter< return function (routes: RouteObject[], opts?: Record & { basename?: string }): TRouter { addRoutesToAllRoutes(routes); + // Check for async handlers that might contain sub-route declarations (only if enabled) + if (_enableAsyncRouteHandlers) { + for (const route of routes) { + checkRouteForAsyncHandler(route); + } + } + const router = createRouterFunction(routes, opts); const basename = opts?.basename; @@ -164,6 +310,13 @@ export function createV6CompatibleWrapCreateMemoryRouter< ): TRouter { addRoutesToAllRoutes(routes); + // Check for async handlers that might contain sub-route declarations (only if enabled) + if (_enableAsyncRouteHandlers) { + for (const route of routes) { + checkRouteForAsyncHandler(route); + } + } + const router = createRouterFunction(routes, opts); const basename = opts?.basename; @@ -230,6 +383,7 @@ export function createReactRouterV6CompatibleTracingIntegration( createRoutesFromChildren, matchRoutes, stripBasename, + enableAsyncRouteHandlers = false, instrumentPageLoad = true, instrumentNavigation = true, } = options; @@ -245,6 +399,7 @@ export function createReactRouterV6CompatibleTracingIntegration( _matchRoutes = matchRoutes; _createRoutesFromChildren = createRoutesFromChildren; _stripBasename = stripBasename || false; + _enableAsyncRouteHandlers = enableAsyncRouteHandlers; }, afterAllSetup(client) { integration.afterAllSetup(client); @@ -542,6 +697,7 @@ function getNormalizedName( } let pathBuilder = ''; + if (branches) { for (const branch of branches) { const route = branch.route; diff --git a/packages/react/src/types.ts b/packages/react/src/types.ts index 19aacffc5ac3..c25ee5df1ae3 100644 --- a/packages/react/src/types.ts +++ b/packages/react/src/types.ts @@ -13,6 +13,7 @@ export type Location = { export interface NonIndexRouteObject { caseSensitive?: boolean; children?: RouteObject[]; + handle?: Record; element?: React.ReactNode | null; errorElement?: React.ReactNode | null; index?: any; @@ -22,6 +23,7 @@ export interface NonIndexRouteObject { export interface IndexRouteObject { caseSensitive?: boolean; children?: undefined; + handle?: Record; element?: React.ReactNode | null; errorElement?: React.ReactNode | null; index: any; diff --git a/packages/react/test/reactrouterv6-compat-utils.test.tsx b/packages/react/test/reactrouterv6-compat-utils.test.tsx index 193b4aaab223..1f10d89c8558 100644 --- a/packages/react/test/reactrouterv6-compat-utils.test.tsx +++ b/packages/react/test/reactrouterv6-compat-utils.test.tsx @@ -1,5 +1,6 @@ -import { describe, expect } from 'vitest'; -import { getNumberOfUrlSegments } from '../src/reactrouterv6-compat-utils'; +import { describe, expect, it, test } from 'vitest'; +import { checkRouteForAsyncHandler, getNumberOfUrlSegments } from '../src/reactrouterv6-compat-utils'; +import type { RouteObject } from '../src/types'; describe('getNumberOfUrlSegments', () => { test.each([ @@ -11,3 +12,304 @@ describe('getNumberOfUrlSegments', () => { expect(getNumberOfUrlSegments(input)).toEqual(output); }); }); + +describe('checkRouteForAsyncHandler', () => { + it('should not create nested proxies when called multiple times on the same route', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + }, + }; + + checkRouteForAsyncHandler(route); + checkRouteForAsyncHandler(route); + + const proxiedHandler = route.handle?.lazyChildren; + expect(typeof proxiedHandler).toBe('function'); + expect(proxiedHandler).not.toBe(mockHandler); + + expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + const proxyHandler = (proxiedHandler as any)?.__sentry_proxied__; + expect(proxyHandler).toBe(true); + }); + + it('should handle routes without handle property', () => { + const route: RouteObject = { + path: '/test', + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with non-function handle properties', () => { + const route: RouteObject = { + path: '/test', + handle: { + someData: 'not a function', + }, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with null/undefined handle properties', () => { + const route: RouteObject = { + path: '/test', + handle: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with mixed function and non-function handle properties', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + someData: 'not a function', + anotherData: 123, + }, + }; + + checkRouteForAsyncHandler(route); + + const proxiedHandler = route.handle?.lazyChildren; + expect(typeof proxiedHandler).toBe('function'); + expect(proxiedHandler).not.toBe(mockHandler); + expect((proxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Non-function properties should remain unchanged + expect(route.handle?.someData).toBe('not a function'); + expect(route.handle?.anotherData).toBe(123); + }); + + it('should handle nested routes with async handlers', () => { + const parentHandler = () => Promise.resolve([]); + const childHandler = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/parent', + handle: { + lazyChildren: parentHandler, + }, + children: [ + { + path: '/child', + handle: { + lazyChildren: childHandler, + }, + }, + ], + }; + + checkRouteForAsyncHandler(route); + + // Check parent handler is proxied + const proxiedParentHandler = route.handle?.lazyChildren; + expect(typeof proxiedParentHandler).toBe('function'); + expect(proxiedParentHandler).not.toBe(parentHandler); + expect((proxiedParentHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Check child handler is proxied + const proxiedChildHandler = route.children?.[0]?.handle?.lazyChildren; + expect(typeof proxiedChildHandler).toBe('function'); + expect(proxiedChildHandler).not.toBe(childHandler); + expect((proxiedChildHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should handle deeply nested routes', () => { + const level1Handler = () => Promise.resolve([]); + const level2Handler = () => Promise.resolve([]); + const level3Handler = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/level1', + handle: { + lazyChildren: level1Handler, + }, + children: [ + { + path: '/level2', + handle: { + lazyChildren: level2Handler, + }, + children: [ + { + path: '/level3', + handle: { + lazyChildren: level3Handler, + }, + }, + ], + }, + ], + }; + + checkRouteForAsyncHandler(route); + + // Check all handlers are proxied + expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe( + true, + ); + expect( + (route.children?.[0]?.children?.[0]?.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__, + ).toBe(true); + }); + + it('should handle routes with multiple async handlers', () => { + const handler1 = () => Promise.resolve([]); + const handler2 = () => Promise.resolve([]); + const handler3 = () => Promise.resolve([]); + + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: handler1, + asyncLoader: handler2, + dataLoader: handler3, + }, + }; + + checkRouteForAsyncHandler(route); + + // Check all handlers are proxied + expect((route.handle?.lazyChildren as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.handle?.asyncLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + expect((route.handle?.dataLoader as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should not re-proxy already proxied functions', () => { + const mockHandler = () => Promise.resolve([]); + const route: RouteObject = { + path: '/test', + handle: { + lazyChildren: mockHandler, + }, + }; + + // First call should proxy the function + checkRouteForAsyncHandler(route); + const firstProxiedHandler = route.handle?.lazyChildren; + expect(firstProxiedHandler).not.toBe(mockHandler); + expect((firstProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + + // Second call should not create a new proxy + checkRouteForAsyncHandler(route); + const secondProxiedHandler = route.handle?.lazyChildren; + expect(secondProxiedHandler).toBe(firstProxiedHandler); // Should be the same proxy + expect((secondProxiedHandler as { __sentry_proxied__?: boolean }).__sentry_proxied__).toBe(true); + }); + + it('should handle routes with empty children array', () => { + const route: RouteObject = { + path: '/test', + children: [], + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with undefined children', () => { + const route: RouteObject = { + path: '/test', + children: undefined, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with null children', () => { + const route: RouteObject = { + path: '/test', + children: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with non-array children', () => { + const route: RouteObject = { + path: '/test', + children: 'not an array' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is not an object', () => { + const route: RouteObject = { + path: '/test', + handle: 'not an object' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is null', () => { + const route: RouteObject = { + path: '/test', + handle: null as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is undefined', () => { + const route: RouteObject = { + path: '/test', + handle: undefined as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a function', () => { + const route: RouteObject = { + path: '/test', + handle: (() => {}) as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a string', () => { + const route: RouteObject = { + path: '/test', + handle: 'string handle' as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a number', () => { + const route: RouteObject = { + path: '/test', + handle: 42 as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is a boolean', () => { + const route: RouteObject = { + path: '/test', + handle: true as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); + + it('should handle routes with handle that is an array', () => { + const route: RouteObject = { + path: '/test', + handle: [] as any, + }; + + expect(() => checkRouteForAsyncHandler(route)).not.toThrow(); + }); +}); From f0661fc0de9b796a6f1f5f4eea329a1916d53124 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 8 Aug 2025 11:32:40 +0200 Subject: [PATCH 10/10] meta(changelog): Update changelog for 10.3.0 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f56fe6262be4..50c7c9202486 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 10.3.0 + +- feat(core): MCP Server - Capture prompt results from prompt function calls (#17284) +- feat(bun): Export `skipOpenTelemetrySetup` option ([#17349](https://github.com/getsentry/sentry-javascript/pull/17349)) +- feat(sveltekit): Streamline build logs ([#17306](https://github.com/getsentry/sentry-javascript/pull/17306)) +- fix(browser): Handle data urls in errors caught by `globalHandlersIntegration` ([#17216](https://github.com/getsentry/sentry-javascript/pull/17216)) +- fix(browser): Improve navigation vs. redirect detection ([#17275](https://github.com/getsentry/sentry-javascript/pull/17275)) +- fix(react-router): Ensure source map upload fails silently if Sentry CLI fails ([#17081](https://github.com/getsentry/sentry-javascript/pull/17081)) +- fix(react): Add support for React Router sub-routes from `handle` ([#17277](https://github.com/getsentry/sentry-javascript/pull/17277)) + ## 10.2.0 ### Important Changes