Skip to content

Commit 9851d64

Browse files
s1gr1dclaude
andauthored
fix(astro): Fall back to dataCollection.userInfo for trackClientIp (#24091)
`handleRequest` hard-defaulted `trackClientIp` to `false`, so the global `dataCollection.userInfo` could never switch client IP collection on. An integration option is only meant to win when the user actually sets it. It is now `options.trackClientIp ?? dataCollection.userInfo`. Heads up, this changes the default. Astro apps that leave both alone start reporting `user.ip_address`, matching `userInfo`'s documented default of `true` and what the other server SDKs do. Fixes #24086 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 64d9de9 commit 9851d64

3 files changed

Lines changed: 76 additions & 24 deletions

File tree

‎MIGRATION.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,12 @@ User IP address inference, which was previously gated on `sendDefaultPii`, is no
246246
`dataCollection.userInfo`. An explicit `requestDataIntegration({ include: { ip: true } })` overrides
247247
`dataCollection.userInfo: false` for data collected by that integration.
248248

249+
#### Astro client IP
250+
251+
`trackClientIp` no longer defaults to `false`. When you leave it unset, `handleRequest` now follows
252+
`dataCollection.userInfo`, which defaults to `true`, so Astro apps that set neither option start
253+
reporting `user.ip_address`. Pass `trackClientIp: false` to keep the v10 behaviour.
254+
249255
#### Remix action form data
250256

251257
`captureActionFormDataKeys` is an integration-level override, so it no longer requires

‎packages/astro/src/server/middleware.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type MiddlewareOptions = {
5353
*
5454
* Only set this to `true` if you're fine with collecting potentially personally identifiable information (PII).
5555
*
56-
* @default false (recommended)
56+
* @default `dataCollection.userInfo` (`true` unless disabled)
5757
*/
5858
trackClientIp?: boolean;
5959
};
@@ -78,10 +78,7 @@ type AstroLocalsWithSentry = Record<string, unknown> & {
7878
};
7979

8080
export const handleRequest: (options?: MiddlewareOptions) => MiddlewareHandler = options => {
81-
const handlerOptions = {
82-
trackClientIp: false,
83-
...options,
84-
};
81+
const handlerOptions = { ...options };
8582

8683
return async (ctx, next) => {
8784
// If no Sentry client exists, just bail
@@ -209,7 +206,8 @@ async function instrumentRequestStartHttpServerSpan(
209206
normalizedRequest: winterCGRequestToRequestData(request),
210207
});
211208

212-
if (options.trackClientIp) {
209+
// The integration option wins when set; otherwise `dataCollection.userInfo` decides.
210+
if (options.trackClientIp ?? client.getDataCollectionOptions().userInfo) {
213211
isolationScope.setUser({ ip_address: ctx.clientAddress });
214212
}
215213

‎packages/astro/test/server/middleware.test.ts‎

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,29 @@ describe('sentryMiddleware', () => {
4646
});
4747
const setSDKProcessingMetadataMock = vi.fn();
4848

49+
const DATA_COLLECTION_DEFAULTS = {
50+
userInfo: false,
51+
cookies: true,
52+
httpHeaders: { request: true, response: true },
53+
httpBodies: [],
54+
urlQueryParams: true,
55+
graphQL: { document: true, variables: true },
56+
genAI: { inputs: true, outputs: true },
57+
databaseQueryData: true,
58+
stackFrameVariables: true,
59+
frameContextLines: 5,
60+
};
61+
62+
function mockClientWith(dataCollection: Partial<typeof DATA_COLLECTION_DEFAULTS>): void {
63+
vi.spyOn(SentryNode, 'getClient').mockImplementation(
64+
() =>
65+
({
66+
getOptions: () => ({}),
67+
getDataCollectionOptions: () => ({ ...DATA_COLLECTION_DEFAULTS, ...dataCollection }),
68+
}) as unknown as Client,
69+
);
70+
}
71+
4972
beforeEach(() => {
5073
vi.spyOn(SentryNode, 'getCurrentScope').mockImplementation(() => {
5174
return {
@@ -56,24 +79,7 @@ describe('sentryMiddleware', () => {
5679
} as any;
5780
});
5881
vi.spyOn(SentryNode, 'getActiveSpan').mockImplementation(getSpanMock);
59-
vi.spyOn(SentryNode, 'getClient').mockImplementation(
60-
() =>
61-
({
62-
getOptions: () => ({}),
63-
getDataCollectionOptions: () => ({
64-
userInfo: false,
65-
cookies: true,
66-
httpHeaders: { request: true, response: true },
67-
httpBodies: [],
68-
urlQueryParams: true,
69-
graphQL: { document: true, variables: true },
70-
genAI: { inputs: true, outputs: true },
71-
databaseQueryData: true,
72-
stackFrameVariables: true,
73-
frameContextLines: 5,
74-
}),
75-
}) as unknown as Client,
76-
);
82+
mockClientWith({ userInfo: false });
7783
vi.spyOn(SentryNode, 'getTraceMetaTags').mockImplementation(
7884
() => `
7985
<meta name="sentry-trace" content="123">
@@ -308,6 +314,48 @@ describe('sentryMiddleware', () => {
308314
});
309315
});
310316

317+
it('attaches the client IP when `trackClientIp` is unset and `dataCollection.userInfo` is on', async () => {
318+
mockClientWith({ userInfo: true });
319+
const middleware = handleRequest();
320+
const ctx = {
321+
...DYNAMIC_REQUEST_CONTEXT,
322+
};
323+
324+
// @ts-expect-error, a partial ctx object is fine here
325+
await middleware(ctx, async () => {
326+
expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBe('192.168.0.1');
327+
return nextResult;
328+
});
329+
});
330+
331+
it('does not attach a client IP when `trackClientIp` is unset and `dataCollection.userInfo` is off', async () => {
332+
mockClientWith({ userInfo: false });
333+
const middleware = handleRequest();
334+
const ctx = {
335+
...DYNAMIC_REQUEST_CONTEXT,
336+
};
337+
338+
// @ts-expect-error, a partial ctx object is fine here
339+
await middleware(ctx, async () => {
340+
expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBeUndefined();
341+
return nextResult;
342+
});
343+
});
344+
345+
it('lets `trackClientIp=false` win over `dataCollection.userInfo`', async () => {
346+
mockClientWith({ userInfo: true });
347+
const middleware = handleRequest({ trackClientIp: false });
348+
const ctx = {
349+
...DYNAMIC_REQUEST_CONTEXT,
350+
};
351+
352+
// @ts-expect-error, a partial ctx object is fine here
353+
await middleware(ctx, async () => {
354+
expect(SentryCore.getIsolationScope().getScopeData().user?.ip_address).toBeUndefined();
355+
return nextResult;
356+
});
357+
});
358+
311359
it("doesn't attach a client IP if `trackClientIp=true` when handling static page requests", async () => {
312360
const middleware = handleRequest({ trackClientIp: true });
313361

0 commit comments

Comments
 (0)