diff --git a/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts index d5f07ebe239a..3faacdb30090 100644 --- a/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/astro-4/tests/errors.server.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForError } from '@sentry-internal/test-utils'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; test.describe('server-side errors', () => { test('captures SSR error', async ({ page }) => { @@ -7,9 +7,26 @@ test.describe('server-side errors', () => { return errorEvent?.exception?.values?.[0]?.value === "Cannot read properties of undefined (reading 'x')"; }); + const transactionEventPromise = waitForTransaction('astro-4', transactionEvent => { + return transactionEvent.transaction === 'GET /ssr-error'; + }); + await page.goto('/ssr-error'); const errorEvent = await errorEventPromise; + const transactionEvent = await transactionEventPromise; + + expect(transactionEvent).toMatchObject({ + transaction: 'GET /ssr-error', + spans: [], + }); + + const traceId = transactionEvent.contexts?.trace?.trace_id; + const spanId = transactionEvent.contexts?.trace?.span_id; + + expect(traceId).toMatch(/[a-f0-9]{32}/); + expect(spanId).toMatch(/[a-f0-9]{16}/); + expect(transactionEvent.contexts?.trace?.parent_span_id).toBeUndefined(); expect(errorEvent).toMatchObject({ contexts: { @@ -20,8 +37,8 @@ test.describe('server-side errors', () => { os: expect.any(Object), runtime: expect.any(Object), trace: { - span_id: '', //TODO: This is a bug! We should expect.stringMatching(/[a-f0-9]{16}/) instead of '' - trace_id: expect.stringMatching(/[a-f0-9]{32}/), + span_id: spanId, + trace_id: traceId, }, }, environment: 'qa', @@ -69,18 +86,50 @@ test.describe('server-side errors', () => { const errorEventPromise = waitForError('astro-4', errorEvent => { return errorEvent?.exception?.values?.[0]?.value === 'Endpoint Error'; }); + const transactionEventApiPromise = waitForTransaction('astro-4', transactionEvent => { + return transactionEvent.transaction === 'GET /endpoint-error/api'; + }); + const transactionEventEndpointPromise = waitForTransaction('astro-4', transactionEvent => { + return transactionEvent.transaction === 'GET /endpoint-error'; + }); await page.goto('/endpoint-error'); await page.getByText('Get Data').click(); const errorEvent = await errorEventPromise; + const transactionEventApi = await transactionEventApiPromise; + const transactionEventEndpoint = await transactionEventEndpointPromise; + + expect(transactionEventEndpoint).toMatchObject({ + transaction: 'GET /endpoint-error', + spans: [], + }); + + const traceId = transactionEventEndpoint.contexts?.trace?.trace_id; + const endpointSpanId = transactionEventApi.contexts?.trace?.span_id; + + expect(traceId).toMatch(/[a-f0-9]{32}/); + expect(endpointSpanId).toMatch(/[a-f0-9]{16}/); + + expect(transactionEventApi).toMatchObject({ + transaction: 'GET /endpoint-error/api', + spans: [], + }); + + const spanId = transactionEventApi.contexts?.trace?.span_id; + const parentSpanId = transactionEventApi.contexts?.trace?.parent_span_id; + + expect(spanId).toMatch(/[a-f0-9]{16}/); + // TODO: This is incorrect, for whatever reason, it should be the endpointSpanId ideally + expect(parentSpanId).toMatch(/[a-f0-9]{16}/); + expect(parentSpanId).not.toEqual(endpointSpanId); expect(errorEvent).toMatchObject({ contexts: { trace: { - parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), - span_id: expect.stringMatching(/[a-f0-9]{16}/), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), + parent_span_id: parentSpanId, + span_id: spanId, + trace_id: traceId, }, }, exception: { diff --git a/package.json b/package.json index 39139d5faba0..099771f7be38 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "clean:build": "lerna run clean", "clean:caches": "yarn rimraf eslintcache .nxcache && yarn jest --clearCache", "clean:deps": "lerna clean --yes && rm -rf node_modules && yarn", - "clean:tarballs": "rimraf -g **/*.tgz", + "clean:tarballs": "rimraf {packages,dev-packages}/*/*.tgz", "clean:all": "run-s clean:build clean:tarballs clean:caches clean:deps", "fix": "run-s fix:biome fix:prettier fix:lerna", "fix:lerna": "lerna run fix", diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 781a5a75b7a9..14b48706c2cf 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -155,49 +155,50 @@ async function instrumentRequest( op: 'http.server', }, async span => { - const originalResponse = await next(); - - if (originalResponse.status) { - setHttpStatus(span, originalResponse.status); - } - - const client = getClient(); - const contentType = originalResponse.headers.get('content-type'); - - const isPageloadRequest = contentType && contentType.startsWith('text/html'); - if (!isPageloadRequest || !client) { - return originalResponse; - } - - // Type case necessary b/c the body's ReadableStream type doesn't include - // the async iterator that is actually available in Node - // We later on use the async iterator to read the body chunks - // see https://github.com/microsoft/TypeScript/issues/39051 - const originalBody = originalResponse.body as NodeJS.ReadableStream | null; - if (!originalBody) { - return originalResponse; + try { + const originalResponse = await next(); + if (originalResponse.status) { + setHttpStatus(span, originalResponse.status); + } + + const client = getClient(); + const contentType = originalResponse.headers.get('content-type'); + + const isPageloadRequest = contentType && contentType.startsWith('text/html'); + if (!isPageloadRequest || !client) { + return originalResponse; + } + + // Type case necessary b/c the body's ReadableStream type doesn't include + // the async iterator that is actually available in Node + // We later on use the async iterator to read the body chunks + // see https://github.com/microsoft/TypeScript/issues/39051 + const originalBody = originalResponse.body as NodeJS.ReadableStream | null; + if (!originalBody) { + return originalResponse; + } + + const decoder = new TextDecoder(); + + const newResponseStream = new ReadableStream({ + start: async controller => { + for await (const chunk of originalBody) { + const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); + const modifiedHtml = addMetaTagToHead(html); + controller.enqueue(new TextEncoder().encode(modifiedHtml)); + } + controller.close(); + }, + }); + + return new Response(newResponseStream, originalResponse); + } catch (e) { + sendErrorToSentry(e); + throw e; } - - const decoder = new TextDecoder(); - - const newResponseStream = new ReadableStream({ - start: async controller => { - for await (const chunk of originalBody) { - const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); - } - controller.close(); - }, - }); - - return new Response(newResponseStream, originalResponse); }, ); return res; - } catch (e) { - sendErrorToSentry(e); - throw e; } finally { vercelWaitUntil( (async () => { diff --git a/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts b/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts index d2aa470213f7..7c1d94daa7d8 100644 --- a/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts +++ b/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts @@ -17,6 +17,7 @@ export function generateSpanContextForPropagationContext(propagationContext: Pro const spanContext: SpanContext = { traceId: propagationContext.traceId, + // TODO: Do not create an invalid span context here spanId: propagationContext.parentSpanId || '', isRemote: true, traceFlags: propagationContext.sampled ? TraceFlags.SAMPLED : TraceFlags.NONE,