drop it like it's hot

Author: mydea

packages/astro/src/index.server.ts

@@ -31,7 +31,6 @@ export {
   cron,
   dataloaderIntegration,
   dedupeIntegration,
-  DEFAULT_USER_INCLUDES,
   defaultStackParser,
   endSession,
   expressErrorHandler,

packages/aws-serverless/src/index.ts

@@ -42,7 +42,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  DEFAULT_USER_INCLUDES,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/bun/src/index.ts

@@ -18,7 +18,6 @@ export type {
   Thread,
   User,
 } from '@sentry/core';
-export type { AddRequestDataToEventOptions } from '@sentry/core';
 
 export {
   addEventProcessor,
@@ -64,7 +63,6 @@ export {
   flush,
   close,
   getSentryRelease,
-  DEFAULT_USER_INCLUDES,
   createGetModuleFromFilename,
   anrIntegration,
   disableAnrDetectionForCallback,

packages/cloudflare/src/index.ts

@@ -18,7 +18,6 @@ export type {
   Thread,
   User,
 } from '@sentry/core';
-export type { AddRequestDataToEventOptions } from '@sentry/core';
 
 export type { CloudflareOptions } from './client';
 

packages/core/src/index.ts

@@ -3,7 +3,6 @@ export type { AsyncContextStrategy } from './asyncContext/types';
 export type { Carrier } from './carrier';
 export type { OfflineStore, OfflineTransportOptions } from './transports/offline';
 export type { ServerRuntimeClientOptions } from './server-runtime-client';
-export type { RequestDataIntegrationOptions } from './integrations/requestdata';
 export type { IntegrationIndex } from './integration';
 
 export * from './tracing';

packages/core/src/integrations/requestdata.ts

@@ -1,88 +1,49 @@
 import { defineIntegration } from '../integration';
-import type { IntegrationFn } from '../types-hoist';
-import { type AddRequestDataToEventOptions, addNormalizedRequestDataToEvent } from '../utils-hoist/requestdata';
+import type { Event, IntegrationFn, RequestEventData } from '../types-hoist';
+import { parseCookie } from '../utils-hoist/cookie';
+import { getClientIPAddress, ipHeaderNames } from '../utils-hoist/vendor/getIpAddress';
 
-export type RequestDataIntegrationOptions = {
-  /**
-   * Controls what data is pulled from the request and added to the event
-   */
-  include?: {
-    cookies?: boolean;
-    data?: boolean;
-    headers?: boolean;
-    ip?: boolean;
-    query_string?: boolean;
-    url?: boolean;
-    user?:
-      | boolean
-      | {
-          id?: boolean;
-          username?: boolean;
-          email?: boolean;
-        };
-  };
+interface RequestDataIncludeOptions {
+  cookies?: boolean;
+  data?: boolean;
+  headers?: boolean;
+  ip?: boolean;
+  query_string?: boolean;
+  url?: boolean;
+}
 
+type RequestDataIntegrationOptions = {
   /**
-   * Whether to identify transactions by parameterized path, parameterized path with method, or handler name.
-   * @deprecated This option does not do anything anymore, and will be removed in v9.
+   * Controls what data is pulled from the request and added to the event.
    */
-  transactionNamingScheme?: 'path' | 'methodPath' | 'handler';
+  include?: RequestDataIncludeOptions;
 };
 
-const DEFAULT_OPTIONS = {
-  include: {
-    cookies: true,
-    data: true,
-    headers: true,
-    ip: false,
-    query_string: true,
-    url: true,
-    user: {
-      id: true,
-      username: true,
-      email: true,
-    },
-  },
-  transactionNamingScheme: 'methodPath' as const,
-};
+const DEFAULT_INCLUDE = {
+  cookies: true,
+  data: true,
+  headers: true,
+  ip: false,
+  query_string: true,
+  url: true,
+} satisfies RequestDataIncludeOptions;
 
 const INTEGRATION_NAME = 'RequestData';
 
 const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) => {
-  const _options: Required<RequestDataIntegrationOptions> = {
-    ...DEFAULT_OPTIONS,
-    ...options,
-    include: {
-      ...DEFAULT_OPTIONS.include,
-      ...options.include,
-      user:
-        options.include && typeof options.include.user === 'boolean'
-          ? options.include.user
-          : {
-              ...DEFAULT_OPTIONS.include.user,
-              // Unclear why TS still thinks `options.include.user` could be a boolean at this point
-              ...((options.include || {}).user as Record<string, boolean>),
-            },
-    },
+  const include = {
+    ...DEFAULT_INCLUDE,
+    ...options.include,
   };
 
   return {
     name: INTEGRATION_NAME,
     processEvent(event) {
-      // Note: In the long run, most of the logic here should probably move into the request data utility functions. For
-      // the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed.
-      // (TL;DR: Those functions touch many parts of the repo in many different ways, and need to be cleaned up. Once
-      // that's happened, it will be easier to add this logic in without worrying about unexpected side effects.)
-
       const { sdkProcessingMetadata = {} } = event;
       const { normalizedRequest, ipAddress } = sdkProcessingMetadata;
 
-      const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options);
-
-      // If this is set, it takes precedence over the plain request object
       if (normalizedRequest) {
-        // TODO: user???
-        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, addRequestDataOptions);
+        addNormalizedRequestDataToEvent(event, normalizedRequest, { ipAddress }, include);
         return event;
       }
 
@@ -97,42 +58,75 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
  */
 export const requestDataIntegration = defineIntegration(_requestDataIntegration);
 
-/** Convert this integration's options to match what `addRequestDataToEvent` expects */
-/** TODO: Can possibly be deleted once https://github.com/getsentry/sentry-javascript/issues/5718 is fixed */
-function convertReqDataIntegrationOptsToAddReqDataOpts(
-  integrationOptions: Required<RequestDataIntegrationOptions>,
-): AddRequestDataToEventOptions {
-  const {
-    include: { ip, user, ...requestOptions },
-  } = integrationOptions;
-
-  const requestIncludeKeys: string[] = ['method'];
-  for (const [key, value] of Object.entries(requestOptions)) {
-    if (value) {
-      requestIncludeKeys.push(key);
+/**
+ * Add already normalized request data to an event.
+ * This mutates the passed in event.
+ */
+function addNormalizedRequestDataToEvent(
+  event: Event,
+  req: RequestEventData,
+  // Data that should not go into `event.request` but is somehow related to requests
+  additionalData: { ipAddress?: string },
+  include: RequestDataIncludeOptions,
+): void {
+  event.request = {
+    ...event.request,
+    ...extractNormalizedRequestData(req, include),
+  };
+
+  if (include.ip) {
+    const ip = (req.headers && getClientIPAddress(req.headers)) || additionalData.ipAddress;
+    if (ip) {
+      event.user = {
+        ...event.user,
+        ip_address: ip,
+      };
     }
   }
+}
 
-  let addReqDataUserOpt;
-  if (user === undefined) {
-    addReqDataUserOpt = true;
-  } else if (typeof user === 'boolean') {
-    addReqDataUserOpt = user;
-  } else {
-    const userIncludeKeys: string[] = [];
-    for (const [key, value] of Object.entries(user)) {
-      if (value) {
-        userIncludeKeys.push(key);
-      }
+function extractNormalizedRequestData(
+  normalizedRequest: RequestEventData,
+  include: RequestDataIncludeOptions,
+): RequestEventData {
+  const requestData: RequestEventData = {};
+  const headers = { ...normalizedRequest.headers };
+
+  if (include.headers) {
+    requestData.headers = headers;
+
+    // Remove the Cookie header in case cookie data should not be included in the event
+    if (!include.cookies) {
+      delete (headers as { cookie?: string }).cookie;
+    }
+
+    // Remove IP headers in case IP data should not be included in the event
+    if (!include.ip) {
+      ipHeaderNames.forEach(ipHeaderName => {
+        // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
+        delete (headers as Record<string, unknown>)[ipHeaderName];
+      });
     }
-    addReqDataUserOpt = userIncludeKeys;
   }
 
-  return {
-    include: {
-      ip,
-      user: addReqDataUserOpt,
-      request: requestIncludeKeys.length !== 0 ? requestIncludeKeys : undefined,
-    },
-  };
+  requestData.method = normalizedRequest.method;
+
+  if (include.url) {
+    requestData.url = normalizedRequest.url;
+  }
+
+  if (include.cookies) {
+    const cookies = normalizedRequest.cookies || (headers && headers.cookie ? parseCookie(headers.cookie) : undefined);
+    requestData.cookies = cookies || {};
+  }
+
+  if (include.query_string) {
+    requestData.query_string = normalizedRequest.query_string;
+  }
+
+  if (include.data) {
+    requestData.data = normalizedRequest.data;
+  }
+
+  return requestData;
 }

packages/core/src/utils-hoist/index.ts

@@ -66,17 +66,13 @@ export { basename, dirname, isAbsolute, join, normalizePath, relative, resolve }
 export { makePromiseBuffer } from './promisebuffer';
 export type { PromiseBuffer } from './promisebuffer';
 
-// TODO: Remove requestdata export once equivalent integration is used everywhere
 export {
-  DEFAULT_USER_INCLUDES,
-  addNormalizedRequestDataToEvent,
   winterCGHeadersToDict,
   winterCGRequestToRequestData,
   httpRequestToRequestData,
   extractQueryParamsFromUrl,
   headersToDict,
 } from './requestdata';
-export type { AddRequestDataToEventOptions } from './requestdata';
 
 export { severityLevelFromString } from './severity';
 export {