ref: Refactor remaining usage of request to normalizedRequest (#14401)

This includes exporting `httpRequestToRequestEventData` from
`@sentry/node`, which we can reuse in a few places.

This also allows us to remove a bunch of stuff from remix, because
actually we can just use `winterCGRequestToRequestData` instead of the
custom implementation we have there. at least type wise, this should all
work, let's see if any test complains...

Closes #14298

Author: mydea

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

@@ -73,6 +73,7 @@ export {
   extractRequestData,
   winterCGHeadersToDict,
   winterCGRequestToRequestData,
+  httpRequestToRequestData,
   extractQueryParamsFromUrl,
   headersToDict,
 } from './requestdata';

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

@@ -14,6 +14,7 @@ import { DEBUG_BUILD } from './debug-build';
 import { isPlainObject, isString } from './is';
 import { logger } from './logger';
 import { normalize } from './normalize';
+import { dropUndefinedKeys } from './object';
 import { truncate } from './string';
 import { stripUrlQueryAndFragment } from './url';
 import { getClientIPAddress, ipHeaderNames } from './vendor/getIpAddress';
@@ -451,6 +452,43 @@ export function winterCGRequestToRequestData(req: WebFetchRequest): RequestEvent
   };
 }
 
+/**
+ * Convert a HTTP request object to RequestEventData to be passed as normalizedRequest.
+ * Instead of allowing `PolymorphicRequest` to be passed,
+ * we want to be more specific and generally require a http.IncomingMessage-like object.
+ */
+export function httpRequestToRequestData(request: {
+  method?: string;
+  url?: string;
+  headers?: {
+    [key: string]: string | string[] | undefined;
+  };
+  protocol?: string;
+  socket?: unknown;
+}): RequestEventData {
+  const headers = request.headers || {};
+  const host = headers.host || '<no host>';
+  const protocol = request.socket && (request.socket as { encrypted?: boolean }).encrypted ? 'https' : 'http';
+  const originalUrl = request.url || '';
+  const absoluteUrl = originalUrl.startsWith(protocol) ? originalUrl : `${protocol}://${host}${originalUrl}`;
+
+  // This is non-standard, but may be sometimes set
+  // It may be overwritten later by our own body handling
+  const data = (request as PolymorphicRequest).body || undefined;
+
+  // This is non-standard, but may be set on e.g. Next.js or Express requests
+  const cookies = (request as PolymorphicRequest).cookies;
+
+  return dropUndefinedKeys({
+    url: absoluteUrl,
+    method: request.method,
+    query_string: extractQueryParamsFromUrl(originalUrl),
+    headers: headersToDict(headers),
+    cookies,
+    data,
+  });
+}
+
 /** Extract the query params from an URL. */
 export function extractQueryParamsFromUrl(url: string): string | undefined {
   // url is path and query string

packages/google-cloud-serverless/src/gcpfunction/http.ts

@@ -2,11 +2,13 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   handleCallbackErrors,
+  httpRequestToRequestData,
+  isString,
+  logger,
   setHttpStatus,
+  stripUrlQueryAndFragment,
 } from '@sentry/core';
-import { isString, logger, stripUrlQueryAndFragment } from '@sentry/core';
 import { captureException, continueTrace, flush, getCurrentScope, startSpanManual } from '@sentry/node';
-
 import { DEBUG_BUILD } from '../debug-build';
 import { domainify, markEventUnhandled, proxyFunction } from '../utils';
 import type { HttpFunction, WrapperOptions } from './general';
@@ -44,6 +46,9 @@ function _wrapHttpFunction(fn: HttpFunction, options: Partial<WrapperOptions>):
     const baggage = req.headers?.baggage;
 
     return continueTrace({ sentryTrace, baggage }, () => {
+      const normalizedRequest = httpRequestToRequestData(req);
+      getCurrentScope().setSDKProcessingMetadata({ normalizedRequest });
+
       return startSpanManual(
         {
           name: `${reqMethod} ${reqUrl}`,
@@ -54,10 +59,6 @@ function _wrapHttpFunction(fn: HttpFunction, options: Partial<WrapperOptions>):
           },
         },
         span => {
-          getCurrentScope().setSDKProcessingMetadata({
-            request: req,
-          });
-
           // eslint-disable-next-line @typescript-eslint/unbound-method
           const _end = res.end;
           // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/google-cloud-serverless/test/gcpfunction/http.test.ts

@@ -176,11 +176,12 @@ describe('GCPFunction', () => {
     expect(defaultIntegrations).toContain('RequestData');
 
     expect(mockScope.setSDKProcessingMetadata).toHaveBeenCalledWith({
-      request: {
+      normalizedRequest: {
         method: 'POST',
-        url: '/path?q=query',
+        url: 'http://hostname/path?q=query',
         headers: { host: 'hostname', 'content-type': 'application/json' },
-        body: { foo: 'bar' },
+        query_string: 'q=query',
+        data: { foo: 'bar' },
       },
     });
   });

packages/nextjs/src/common/pages-router-instrumentation/_error.ts

@@ -1,4 +1,4 @@
-import { captureException, withScope } from '@sentry/core';
+import { captureException, httpRequestToRequestData, withScope } from '@sentry/core';
 import { vercelWaitUntil } from '@sentry/core';
 import type { NextPageContext } from 'next';
 import { flushSafelyWithTimeout } from '../utils/responseEnd';
@@ -38,7 +38,8 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP
 
   withScope(scope => {
     if (req) {
-      scope.setSDKProcessingMetadata({ request: req });
+      const normalizedRequest = httpRequestToRequestData(req);
+      scope.setSDKProcessingMetadata({ normalizedRequest });
     }
 
     // If third-party libraries (or users themselves) throw something falsy, we want to capture it as a message (which

packages/nextjs/src/common/pages-router-instrumentation/wrapApiHandlerWithSentry.ts

@@ -3,6 +3,7 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   captureException,
   continueTrace,
+  httpRequestToRequestData,
   setHttpStatus,
   startSpanManual,
   withIsolationScope,
@@ -65,8 +66,9 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz
             },
             () => {
               const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
+              const normalizedRequest = httpRequestToRequestData(req);
 
-              isolationScope.setSDKProcessingMetadata({ request: req });
+              isolationScope.setSDKProcessingMetadata({ normalizedRequest });
               isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`);
 
               return startSpanManual(

packages/nextjs/src/common/utils/wrapperUtils.ts

@@ -6,6 +6,7 @@ import {
   getIsolationScope,
   getRootSpan,
   getTraceData,
+  httpRequestToRequestData,
 } from '@sentry/core';
 import { TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL } from '../span-attributes-with-logic-attached';
 
@@ -61,10 +62,9 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
     this: unknown,
     ...args: Parameters<F>
   ): Promise<{ data: ReturnType<F>; sentryTrace?: string; baggage?: string }> {
+    const normalizedRequest = httpRequestToRequestData(req);
     getCurrentScope().setTransactionName(`${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`);
-    getIsolationScope().setSDKProcessingMetadata({
-      request: req,
-    });
+    getIsolationScope().setSDKProcessingMetadata({ normalizedRequest });
 
     const span = getActiveSpan();
 

packages/nextjs/src/common/wrapMiddlewareWithSentry.ts

@@ -8,9 +8,10 @@ import {
   handleCallbackErrors,
   setCapturedScopesOnSpan,
   startSpan,
+  vercelWaitUntil,
+  winterCGRequestToRequestData,
   withIsolationScope,
 } from '@sentry/core';
-import { vercelWaitUntil, winterCGRequestToRequestData } from '@sentry/core';
 import type { TransactionSource } from '@sentry/types';
 import type { EdgeRouteHandler } from '../edge/types';
 import { flushSafelyWithTimeout } from './utils/responseEnd';

packages/node/src/integrations/http/SentryHttpInstrumentation.ts

@@ -5,17 +5,19 @@ import { VERSION } from '@opentelemetry/core';
 import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
 import { getRequestInfo } from '@opentelemetry/instrumentation-http';
-import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core';
 import {
-  extractQueryParamsFromUrl,
+  addBreadcrumb,
   getBreadcrumbLogLevelFromHttpStatusCode,
+  getClient,
+  getIsolationScope,
   getSanitizedUrlString,
-  headersToDict,
+  httpRequestToRequestData,
   logger,
   parseUrl,
   stripUrlQueryAndFragment,
+  withIsolationScope,
 } from '@sentry/core';
-import type { PolymorphicRequest, RequestEventData, SanitizedRequestData, Scope } from '@sentry/types';
+import type { RequestEventData, SanitizedRequestData, Scope } from '@sentry/types';
 import { DEBUG_BUILD } from '../../debug-build';
 import type { NodeClient } from '../../sdk/client';
 import { getRequestUrl } from '../../utils/getRequestUrl';
@@ -131,26 +133,10 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
 
         instrumentation._diag.debug('http instrumentation for incoming request');
 
-        const request = args[0] as http.IncomingMessage;
-
         const isolationScope = getIsolationScope().clone();
+        const request = args[0] as http.IncomingMessage;
 
-        const headers = request.headers;
-        const host = headers.host || '<no host>';
-        const protocol = request.socket && (request.socket as { encrypted?: boolean }).encrypted ? 'https' : 'http';
-        const originalUrl = request.url || '';
-        const absoluteUrl = originalUrl.startsWith(protocol) ? originalUrl : `${protocol}://${host}${originalUrl}`;
-
-        // This is non-standard, but may be set on e.g. Next.js or Express requests
-        const cookies = (request as PolymorphicRequest).cookies;
-
-        const normalizedRequest: RequestEventData = {
-          url: absoluteUrl,
-          method: request.method,
-          query_string: extractQueryParamsFromUrl(request.url || ''),
-          headers: headersToDict(request.headers),
-          cookies,
-        };
+        const normalizedRequest = httpRequestToRequestData(request);
 
         patchRequestToCaptureBody(request, isolationScope);
 

packages/remix/src/utils/errors.ts

@@ -1,13 +1,20 @@
 import type { AppData, DataFunctionArgs, EntryContext, HandleDocumentRequestFunction } from '@remix-run/node';
-import { captureException, getClient, handleCallbackErrors } from '@sentry/core';
-import { addExceptionMechanism, isPrimitive, logger, objectify } from '@sentry/core';
-import type { Span } from '@sentry/types';
+import {
+  addExceptionMechanism,
+  captureException,
+  getClient,
+  handleCallbackErrors,
+  isPrimitive,
+  logger,
+  objectify,
+  winterCGRequestToRequestData,
+} from '@sentry/core';
+import type { RequestEventData, Span } from '@sentry/types';
 import { DEBUG_BUILD } from './debug-build';
 import type { RemixOptions } from './remixOptions';
 import { storeFormDataKeys } from './utils';
 import { extractData, isResponse, isRouteErrorResponse } from './vendor/response';
 import type { DataFunction, RemixRequest } from './vendor/types';
-import { normalizeRemixRequest } from './web-fetch';
 
 /**
  * Captures an exception happened in the Remix server.
@@ -41,24 +48,18 @@ export async function captureRemixServerException(
     return;
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  let normalizedRequest: Record<string, unknown> = request as unknown as any;
+  let normalizedRequest: RequestEventData = {};
 
   try {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    normalizedRequest = normalizeRemixRequest(request as unknown as any);
+    normalizedRequest = winterCGRequestToRequestData(request);
   } catch (e) {
     DEBUG_BUILD && logger.warn('Failed to normalize Remix request');
   }
 
   const objectifiedErr = objectify(err);
 
   captureException(isResponse(objectifiedErr) ? await extractResponseError(objectifiedErr) : objectifiedErr, scope => {
-    scope.setSDKProcessingMetadata({
-      request: {
-        ...normalizedRequest,
-      },
-    });
+    scope.setSDKProcessingMetadata({ normalizedRequest });
 
     scope.addEventProcessor(event => {
       addExceptionMechanism(event, {

packages/remix/src/utils/instrumentServer.ts

@@ -11,11 +11,12 @@ import {
   spanToJSON,
   spanToTraceHeader,
   startSpan,
+  winterCGRequestToRequestData,
   withIsolationScope,
 } from '@sentry/core';
 import { dynamicSamplingContextToSentryBaggageHeader, fill, isNodeEnv, loadModule, logger } from '@sentry/core';
 import { continueTrace, getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry';
-import type { TransactionSource, WrappedFunction } from '@sentry/types';
+import type { RequestEventData, TransactionSource, WrappedFunction } from '@sentry/types';
 import type { Span } from '@sentry/types';
 
 import { DEBUG_BUILD } from './debug-build';
@@ -39,7 +40,6 @@ import type {
   ServerRoute,
   ServerRouteManifest,
 } from './vendor/types';
-import { normalizeRemixRequest } from './web-fetch';
 
 let FUTURE_FLAGS: FutureConfig | undefined;
 
@@ -296,10 +296,10 @@ function wrapRequestHandler(
     return withIsolationScope(async isolationScope => {
       const options = getClient()?.getOptions();
 
-      let normalizedRequest: Record<string, unknown> = request;
+      let normalizedRequest: RequestEventData = {};
 
       try {
-        normalizedRequest = normalizeRemixRequest(request);
+        normalizedRequest = winterCGRequestToRequestData(request);
       } catch (e) {
         DEBUG_BUILD && logger.warn('Failed to normalize Remix request');
       }
@@ -311,11 +311,7 @@ function wrapRequestHandler(
         isolationScope.setTransactionName(name);
       }
 
-      isolationScope.setSDKProcessingMetadata({
-        request: {
-          ...normalizedRequest,
-        },
-      });
+      isolationScope.setSDKProcessingMetadata({ normalizedRequest });
 
       if (!options || !hasTracingEnabled(options)) {
         return origRequestHandler.call(this, request, loadContext);