ref: Set normalizedRequest instead of request in various places (#14305)

mydea · web-flow · commit 0aadc9b900ac · 2024-11-20T10:03:20.000+01:00
The request data integration prefers this over `request`, we want to get rid of `request` in v9. Updates the following: - [x] astro/src/server/middleware.ts - [x] bun/src/integrations/bunserver.ts - [x] cloudflare/src/scope-utils.ts - [ ] google-cloud-serverless/src/gcpfunction/http.ts - [x] nextjs/src/common/captureRequestError.ts - [x] nextjs/src/common/withServerActionInstrumentation.ts - [x] nextjs/src/common/wrapGenerationFunctionWithSentry.ts - [ ] nextjs/src/common/wrapMiddlewareWithSentry.ts - [x] nextjs/src/common/wrapRouteHandlerWithSentry.ts - [x] nextjs/src/common/wrapServerComponentWithSentry.ts - [ ] nextjs/src/common/pages-router-instrumentation/_error.ts - [ ] nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts - [ ] nextjs/src/common/utils/wrapperUtils.ts - [x] nextjs/src/edge/wrapApiHandlerWithSentry.ts - [ ] remix/src/utils/errors.ts - [ ] remix/src/utils/instrumentServer.ts - [x] sveltekit/src/server/handle.ts Part of #14298
diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/server-components.test.ts
@@ -33,17 +33,12 @@ test('Sends a transaction for a request to app router', async ({ page }) => {
     trace_id: expect.any(String),
   });
 
-  expect(transactionEvent).toEqual(
-    expect.objectContaining({
-      request: {
-        cookies: {},
-        headers: expect.any(Object),
-        url: expect.any(String),
-      },
+  expect(transactionEvent.request).toEqual({
+    cookies: {},
+    headers: expect.objectContaining({
+      'user-agent': expect.any(String),
     }),
-  );
-
-  expect(Object.keys(transactionEvent.request?.headers!).length).toBeGreaterThan(0);
+  });
 
   // The transaction should not contain any spans with the same name as the transaction
   // e.g. "GET /server-component/parameter/[...parameters]"
diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts
@@ -12,9 +12,10 @@ import {
   startSpan,
   withIsolationScope,
 } from '@sentry/node';
-import type { Scope, SpanAttributes } from '@sentry/types';
+import type { RequestEventData, Scope, SpanAttributes } from '@sentry/types';
 import {
   addNonEnumerableProperty,
+  extractQueryParamsFromUrl,
   logger,
   objectify,
   stripUrlQueryAndFragment,
@@ -111,7 +112,13 @@ async function instrumentRequest(
       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 ? winterCGRequestToRequestData(request) : { method, url: request.url },
+        normalizedRequest: (isDynamicPageRequest
+          ? winterCGRequestToRequestData(request)
+          : {
+              method,
+              url: request.url,
+              query_string: extractQueryParamsFromUrl(request.url),
+            }) satisfies RequestEventData,
       });
 
       if (options.trackClientIp && isDynamicPageRequest) {
diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts
@@ -221,7 +221,7 @@ describe('sentryMiddleware', () => {
       await middleware(ctx, next);
 
       expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({
-        request: {
+        normalizedRequest: {
           method: 'GET',
           url: '/users',
           headers: {
@@ -254,7 +254,7 @@ describe('sentryMiddleware', () => {
       await middleware(ctx, next);
 
       expect(setSDKProcessingMetadataMock).toHaveBeenCalledWith({
-        request: {
+        normalizedRequest: {
           method: 'GET',
           url: '/users',
         },
diff --git a/packages/bun/src/integrations/bunserver.ts b/packages/bun/src/integrations/bunserver.ts
@@ -9,8 +9,8 @@ import {
   startSpan,
   withIsolationScope,
 } from '@sentry/core';
-import type { IntegrationFn, SpanAttributes } from '@sentry/types';
-import { getSanitizedUrlString, parseUrl } from '@sentry/utils';
+import type { IntegrationFn, RequestEventData, SpanAttributes } from '@sentry/types';
+import { extractQueryParamsFromUrl, getSanitizedUrlString, parseUrl } from '@sentry/utils';
 
 const INTEGRATION_NAME = 'BunServer';
 
@@ -76,11 +76,12 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
         const url = getSanitizedUrlString(parsedUrl);
 
         isolationScope.setSDKProcessingMetadata({
-          request: {
+          normalizedRequest: {
             url,
             method: request.method,
             headers: request.headers.toJSON(),
-          },
+            query_string: extractQueryParamsFromUrl(url),
+          } satisfies RequestEventData,
         });
 
         return continueTrace(
diff --git a/packages/cloudflare/src/scope-utils.ts b/packages/cloudflare/src/scope-utils.ts
@@ -25,5 +25,5 @@ export function addCultureContext(scope: Scope, cf: IncomingRequestCfProperties)
  * Set request data on scope
  */
 export function addRequest(scope: Scope, request: Request): void {
-  scope.setSDKProcessingMetadata({ request: winterCGRequestToRequestData(request) });
+  scope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(request) });
 }
diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts
@@ -109,7 +109,7 @@ describe('withSentry', () => {
         },
       );
 
-      expect(sentryEvent.sdkProcessingMetadata?.request).toEqual({
+      expect(sentryEvent.sdkProcessingMetadata?.normalizedRequest).toEqual({
         headers: {},
         url: 'https://example.com/',
         method: 'GET',
diff --git a/packages/nextjs/src/common/captureRequestError.ts b/packages/nextjs/src/common/captureRequestError.ts
@@ -1,4 +1,6 @@
 import { captureException, withScope } from '@sentry/core';
+import type { RequestEventData } from '@sentry/types';
+import { headersToDict } from '@sentry/utils';
 
 type RequestInfo = {
   path: string;
@@ -18,10 +20,10 @@ type ErrorContext = {
 export function captureRequestError(error: unknown, request: RequestInfo, errorContext: ErrorContext): void {
   withScope(scope => {
     scope.setSDKProcessingMetadata({
-      request: {
-        headers: request.headers,
+      normalizedRequest: {
+        headers: headersToDict(request.headers),
         method: request.method,
-      },
+      } satisfies RequestEventData,
     });
 
     scope.setContext('nextjs', {
diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts
@@ -9,6 +9,7 @@ import {
   startSpan,
   withIsolationScope,
 } from '@sentry/core';
+import type { RequestEventData } from '@sentry/types';
 import { logger, vercelWaitUntil } from '@sentry/utils';
 
 import { DEBUG_BUILD } from './debug-build';
@@ -89,9 +90,9 @@ async function withServerActionInstrumentationImplementation<A extends (...args:
 
     isolationScope.setTransactionName(`serverAction/${serverActionName}`);
     isolationScope.setSDKProcessingMetadata({
-      request: {
+      normalizedRequest: {
         headers: fullHeadersObject,
-      },
+      } satisfies RequestEventData,
     });
 
     return continueTrace(
diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts
@@ -14,7 +14,7 @@ import {
   withIsolationScope,
   withScope,
 } from '@sentry/core';
-import type { WebFetchHeaders } from '@sentry/types';
+import type { RequestEventData, WebFetchHeaders } from '@sentry/types';
 import { propagationContextFromHeaders, uuid4, winterCGHeadersToDict } from '@sentry/utils';
 
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
@@ -68,9 +68,9 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
           scope.setTransactionName(`${componentType}.${generationFunctionIdentifier} (${componentRoute})`);
 
           isolationScope.setSDKProcessingMetadata({
-            request: {
+            normalizedRequest: {
               headers: headersDict,
-            },
+            } satisfies RequestEventData,
           });
 
           const activeSpan = getActiveSpan();
diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts
@@ -36,7 +36,7 @@ export function wrapMiddlewareWithSentry<H extends EdgeRouteHandler>(
 
         if (req instanceof Request) {
           isolationScope.setSDKProcessingMetadata({
-            request: winterCGRequestToRequestData(req),
+            normalizedRequest: winterCGRequestToRequestData(req),
           });
           spanName = `middleware ${req.method} ${new URL(req.url).pathname}`;
           spanSource = 'url';
diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
@@ -13,7 +13,7 @@ import {
   withIsolationScope,
   withScope,
 } from '@sentry/core';
-
+import type { RequestEventData } from '@sentry/types';
 import type { RouteHandlerContext } from './types';
 
 import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils';
@@ -64,10 +64,10 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
               );
               scope.setPropagationContext(incomingPropagationContext);
               scope.setSDKProcessingMetadata({
-                request: {
+                normalizedRequest: {
                   method,
                   headers: completeHeadersDict,
-                },
+                } satisfies RequestEventData,
               });
             }
 
diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts
@@ -13,6 +13,7 @@ import {
   withIsolationScope,
   withScope,
 } from '@sentry/core';
+import type { RequestEventData } from '@sentry/types';
 import { propagationContextFromHeaders, uuid4, vercelWaitUntil, winterCGHeadersToDict } from '@sentry/utils';
 
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
@@ -49,9 +50,9 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
       const headersDict = context.headers ? winterCGHeadersToDict(context.headers) : undefined;
 
       isolationScope.setSDKProcessingMetadata({
-        request: {
+        normalizedRequest: {
           headers: headersDict,
-        },
+        } satisfies RequestEventData,
       });
 
       return withIsolationScope(isolationScope, () => {
diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts
@@ -32,7 +32,7 @@ export function wrapApiHandlerWithSentry<H extends EdgeRouteHandler>(
 
         if (req instanceof Request) {
           isolationScope.setSDKProcessingMetadata({
-            request: winterCGRequestToRequestData(req),
+            normalizedRequest: winterCGRequestToRequestData(req),
           });
           currentScope.setTransactionName(`${req.method} ${parameterizedRoute}`);
         } else {
diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts
@@ -8,8 +8,10 @@ import { getRequestInfo } from '@opentelemetry/instrumentation-http';
 import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core';
 import type { PolymorphicRequest, RequestEventData, SanitizedRequestData, Scope } from '@sentry/types';
 import {
+  extractQueryParamsFromUrl,
   getBreadcrumbLogLevelFromHttpStatusCode,
   getSanitizedUrlString,
+  headersToDict,
   logger,
   parseUrl,
   stripUrlQueryAndFragment,
@@ -145,7 +147,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
         const normalizedRequest: RequestEventData = {
           url: absoluteUrl,
           method: request.method,
-          query_string: extractQueryParams(request),
+          query_string: extractQueryParamsFromUrl(request.url || ''),
           headers: headersToDict(request.headers),
           cookies,
         };
@@ -443,36 +445,3 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope):
     // ignore errors if we can't patch stuff
   }
 }
-
-function extractQueryParams(req: IncomingMessage): string | undefined {
-  // req.url is path and query string
-  if (!req.url) {
-    return;
-  }
-
-  try {
-    // The `URL` constructor can't handle internal URLs of the form `/some/path/here`, so stick a dummy protocol and
-    // hostname as the base. Since the point here is just to grab the query string, it doesn't matter what we use.
-    const queryParams = new URL(req.url, 'http://dogs.are.great').search.slice(1);
-    return queryParams.length ? queryParams : undefined;
-  } catch {
-    return undefined;
-  }
-}
-
-function headersToDict(reqHeaders: Record<string, string | string[] | undefined>): Record<string, string> {
-  const headers: Record<string, string> = Object.create(null);
-
-  try {
-    Object.entries(reqHeaders).forEach(([key, value]) => {
-      if (typeof value === 'string') {
-        headers[key] = value;
-      }
-    });
-  } catch (e) {
-    DEBUG_BUILD &&
-      logger.warn('Sentry failed extracting headers from a request object. If you see this, please file an issue.');
-  }
-
-  return headers;
-}
diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts
@@ -131,7 +131,9 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
     return withIsolationScope(isolationScope => {
       // We only call continueTrace in the initial top level request to avoid
       // creating a new root span for the sub request.
-      isolationScope.setSDKProcessingMetadata({ request: winterCGRequestToRequestData(input.event.request.clone()) });
+      isolationScope.setSDKProcessingMetadata({
+        normalizedRequest: winterCGRequestToRequestData(input.event.request.clone()),
+      });
       return continueTrace(getTracePropagationData(input.event), () => instrumentHandle(input, options));
     });
   };
@@ -167,7 +169,9 @@ async function instrumentHandle(
         name: routeName,
       },
       async (span?: Span) => {
-        getCurrentScope().setSDKProcessingMetadata({ request: winterCGRequestToRequestData(event.request.clone()) });
+        getCurrentScope().setSDKProcessingMetadata({
+          normalizedRequest: winterCGRequestToRequestData(event.request.clone()),
+        });
         const res = await resolve(event, {
           transformPageChunk: addSentryCodeToPage(options),
         });
diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts
@@ -73,6 +73,8 @@ export {
   extractRequestData,
   winterCGHeadersToDict,
   winterCGRequestToRequestData,
+  extractQueryParamsFromUrl,
+  headersToDict,
 } from './requestdata';
 export type {
   AddRequestDataToEventOptions,
diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts
@@ -416,18 +416,58 @@ export function winterCGHeadersToDict(winterCGHeaders: WebFetchHeaders): Record<
   return headers;
 }
 
+/**
+ * Convert common request headers to a simple dictionary.
+ */
+export function headersToDict(reqHeaders: Record<string, string | string[] | undefined>): Record<string, string> {
+  const headers: Record<string, string> = Object.create(null);
+
+  try {
+    Object.entries(reqHeaders).forEach(([key, value]) => {
+      if (typeof value === 'string') {
+        headers[key] = value;
+      }
+    });
+  } catch (e) {
+    DEBUG_BUILD &&
+      logger.warn('Sentry failed extracting headers from a request object. If you see this, please file an issue.');
+  }
+
+  return headers;
+}
+
 /**
  * Converts a `Request` object that implements the `Web Fetch API` (https://developer.mozilla.org/en-US/docs/Web/API/Headers) into the format that the `RequestData` integration understands.
  */
-export function winterCGRequestToRequestData(req: WebFetchRequest): PolymorphicRequest {
+export function winterCGRequestToRequestData(req: WebFetchRequest): RequestEventData {
   const headers = winterCGHeadersToDict(req.headers);
+
   return {
     method: req.method,
     url: req.url,
+    query_string: extractQueryParamsFromUrl(req.url),
     headers,
+    // TODO: Can we extract body data from the request?
   };
 }
 
+/** Extract the query params from an URL. */
+export function extractQueryParamsFromUrl(url: string): string | undefined {
+  // url is path and query string
+  if (!url) {
+    return;
+  }
+
+  try {
+    // The `URL` constructor can't handle internal URLs of the form `/some/path/here`, so stick a dummy protocol and
+    // hostname as the base. Since the point here is just to grab the query string, it doesn't matter what we use.
+    const queryParams = new URL(url, 'http://dogs.are.great').search.slice(1);
+    return queryParams.length ? queryParams : undefined;
+  } catch {
+    return undefined;
+  }
+}
+
 function extractNormalizedRequestData(
   normalizedRequest: RequestEventData,
   { include }: { include: string[] },

Original file line number	Diff line number	Diff line change
`@@ -25,5 +25,5 @@ export function addCultureContext(scope: Scope, cf: IncomingRequestCfProperties)`
`25`	`25`	`* Set request data on scope`
`26`	`26`	`*/`
`27`	`27`	`export function addRequest(scope: Scope, request: Request): void {`
`28`		`- scope.setSDKProcessingMetadata({ request: winterCGRequestToRequestData(request) });`
	`28`	`+ scope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(request) });`
`29`	`29`	`}`