diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 4b2f15eb3be4..f69fa113721a 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -84,19 +84,27 @@ async function instrumentRequest( } addNonEnumerableProperty(locals, '__sentry_wrapped__', true); - const { method, headers } = ctx.request; + const isDynamicPageRequest = checkIsDynamicPageRequest(ctx); + + const { method, headers } = isDynamicPageRequest + ? ctx.request + : // headers can only be accessed in dynamic routes. Accessing `ctx.request.headers` in a static route + // will make the server log a warning. + { method: ctx.request.method, headers: undefined }; return continueTrace( { - sentryTrace: headers.get('sentry-trace') || undefined, - baggage: headers.get('baggage'), + sentryTrace: headers?.get('sentry-trace') || undefined, + baggage: headers?.get('baggage'), }, async () => { - // We store this on the current scope, not isolation scope, - // because we may have multiple requests nested inside each other - getCurrentScope().setSDKProcessingMetadata({ request: ctx.request }); + getCurrentScope().setSDKProcessingMetadata({ + // We store the request on the current scope, not isolation scope, + // because we may have multiple requests nested inside each other + request: isDynamicPageRequest ? ctx.request : { method, url: ctx.request.url }, + }); - if (options.trackClientIp) { + if (options.trackClientIp && isDynamicPageRequest) { getCurrentScope().setUser({ ip_address: ctx.clientAddress }); } @@ -277,3 +285,16 @@ function tryDecodeUrl(url: string): string | undefined { return undefined; } } + +/** + * Checks if the incoming request is a request for a dynamic (server-side rendered) page. + * We can check this by looking at the middleware's `clientAddress` context property because accessing + * this prop in a static route will throw an error which we can conveniently catch. + */ +function checkIsDynamicPageRequest(context: Parameters[0]): boolean { + try { + return context.clientAddress != null; + } catch { + return false; + } +} diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index bf96f6ef9046..2bb3886b5dd8 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -149,55 +149,117 @@ describe('sentryMiddleware', () => { }); }); - it('attaches client IP if `trackClientIp=true`', async () => { - const middleware = handleRequest({ trackClientIp: true }); - const ctx = { - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, - clientAddress: '192.168.0.1', - params: {}, - url: new URL('https://myDomain.io/users/'), - }; - const next = vi.fn(() => nextResult); + describe('track client IP address', () => { + it('attaches client IP if `trackClientIp=true` when handling dynamic page requests', async () => { + const middleware = handleRequest({ trackClientIp: true }); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + clientAddress: '192.168.0.1', + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); - // @ts-expect-error, a partial ctx object is fine here - await middleware(ctx, next); + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); - expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' }); + expect(setUserMock).toHaveBeenCalledWith({ ip_address: '192.168.0.1' }); + }); + + it("doesn't attach a client IP if `trackClientIp=true` when handling static page requests", async () => { + const middleware = handleRequest({ trackClientIp: true }); + + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + get clientAddress() { + throw new Error('clientAddress.get() should not be called in static page requests'); + }, + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + + const next = vi.fn(() => nextResult); + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); + + expect(setUserMock).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledTimes(1); + }); }); - it('attaches request as SDK processing metadata', async () => { - const middleware = handleRequest({}); - const ctx = { - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, - clientAddress: '192.168.0.1', - params: {}, - url: new URL('https://myDomain.io/users/'), - }; - const next = vi.fn(() => nextResult); + describe('request data', () => { + it('attaches request as SDK processing metadata in dynamic page requests', async () => { + const middleware = handleRequest({}); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + clientAddress: '192.168.0.1', + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); - // @ts-expect-error, a partial ctx object is fine here - await middleware(ctx, next); + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); - expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ - request: { - method: 'GET', - url: '/users', - headers: new Headers({ - 'some-header': 'some-value', - }), - }, + expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + }); + expect(next).toHaveBeenCalledTimes(1); + }); + + it("doesn't attach request headers as processing metadata for static page requests", async () => { + const middleware = handleRequest({}); + const ctx = { + request: { + method: 'GET', + url: '/users', + headers: new Headers({ + 'some-header': 'some-value', + }), + }, + get clientAddress() { + throw new Error('clientAddress.get() should not be called in static page requests'); + }, + params: {}, + url: new URL('https://myDomain.io/users/'), + }; + const next = vi.fn(() => nextResult); + + // @ts-expect-error, a partial ctx object is fine here + await middleware(ctx, next); + + expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({ + request: { + method: 'GET', + url: '/users', + }, + }); + expect(next).toHaveBeenCalledTimes(1); }); });