Skip to content

Commit

Permalink
feat(v8): Remove requestData deprecations (#10626)
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad authored Feb 15, 2024
1 parent 551aa86 commit 4ef3bd5
Show file tree
Hide file tree
Showing 12 changed files with 507 additions and 859 deletions.
18 changes: 4 additions & 14 deletions packages/core/src/integrations/requestdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,16 @@ const DEFAULT_OPTIONS = {
email: true,
},
},
transactionNamingScheme: 'methodPath',
transactionNamingScheme: 'methodPath' as const,
};

const INTEGRATION_NAME = 'RequestData';

const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) => {
const _addRequestData = addRequestDataToEvent;
const _options: Required<RequestDataIntegrationOptions> = {
...DEFAULT_OPTIONS,
...options,
include: {
// @ts-expect-error It's mad because `method` isn't a known `include` key. (It's only here and not set by default in
// `addRequestDataToEvent` for legacy reasons. TODO (v8): Change that.)
method: true,
...DEFAULT_OPTIONS.include,
...options.include,
user:
Expand Down Expand Up @@ -95,15 +91,9 @@ const _requestDataIntegration = ((options: RequestDataIntegrationOptions = {}) =
return event;
}

// The Express request handler takes a similar `include` option to that which can be passed to this integration.
// If passed there, we store it in `sdkProcessingMetadata`. TODO(v8): Force express and GCP people to use this
// integration, so that all of this passing and conversion isn't necessary
const addRequestDataOptions =
sdkProcessingMetadata.requestDataOptionsFromExpressHandler ||
sdkProcessingMetadata.requestDataOptionsFromGCPWrapper ||
convertReqDataIntegrationOptsToAddReqDataOpts(_options);
const addRequestDataOptions = convertReqDataIntegrationOptsToAddReqDataOpts(_options);

const processedEvent = _addRequestData(event, req, addRequestDataOptions);
const processedEvent = addRequestDataToEvent(event, req, addRequestDataOptions);

// Transaction events already have the right `transaction` value
if (event.type === 'transaction' || transactionNamingScheme === 'handler') {
Expand Down Expand Up @@ -185,7 +175,7 @@ function convertReqDataIntegrationOptsToAddReqDataOpts(
include: { ip, user, ...requestOptions },
} = integrationOptions;

const requestIncludeKeys: string[] = [];
const requestIncludeKeys: string[] = ['method'];
for (const [key, value] of Object.entries(requestOptions)) {
if (value) {
requestIncludeKeys.push(key);
Expand Down
49 changes: 3 additions & 46 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import type { Span } from '@sentry/types';
import type { AddRequestDataToEventOptions } from '@sentry/utils';
import {
addRequestDataToTransaction,
dropUndefinedKeys,
extractPathForTransaction,
extractRequestData,
isString,
Expand All @@ -29,8 +28,6 @@ import {

import type { NodeClient } from './client';
import { DEBUG_BUILD } from './debug-build';
// TODO (v8 / XXX) Remove this import
import type { ParseRequestOptions } from './requestDataDeprecated';
import { isAutoSessionTrackingEnabled } from './sdk';

/**
Expand Down Expand Up @@ -115,37 +112,9 @@ export function tracingHandler(): (
};
}

export type RequestHandlerOptions =
// TODO (v8 / XXX) Remove ParseRequestOptions type and eslint override
// eslint-disable-next-line deprecation/deprecation
(ParseRequestOptions | AddRequestDataToEventOptions) & {
flushTimeout?: number;
};

/**
* Backwards compatibility shim which can be removed in v8. Forces the given options to follow the
* `AddRequestDataToEventOptions` interface.
*
* TODO (v8): Get rid of this, and stop passing `requestDataOptionsFromExpressHandler` to `setSDKProcessingMetadata`.
*/
function convertReqHandlerOptsToAddReqDataOpts(
reqHandlerOptions: RequestHandlerOptions = {},
): AddRequestDataToEventOptions | undefined {
let addRequestDataOptions: AddRequestDataToEventOptions | undefined;

if ('include' in reqHandlerOptions) {
addRequestDataOptions = { include: reqHandlerOptions.include };
} else {
// eslint-disable-next-line deprecation/deprecation
const { ip, request, transaction, user } = reqHandlerOptions as ParseRequestOptions;

if (ip || request || transaction || user) {
addRequestDataOptions = { include: dropUndefinedKeys({ ip, request, transaction, user }) };
}
}

return addRequestDataOptions;
}
export type RequestHandlerOptions = AddRequestDataToEventOptions & {
flushTimeout?: number;
};

/**
* Express compatible request handler.
Expand All @@ -154,9 +123,6 @@ function convertReqHandlerOptsToAddReqDataOpts(
export function requestHandler(
options?: RequestHandlerOptions,
): (req: http.IncomingMessage, res: http.ServerResponse, next: (error?: any) => void) => void {
// TODO (v8): Get rid of this
const requestDataOptions = convertReqHandlerOptsToAddReqDataOpts(options);

const client = getClient<NodeClient>();
// Initialise an instance of SessionFlusher on the client when `autoSessionTracking` is enabled and the
// `requestHandler` middleware is used indicating that we are running in SessionAggregates mode
Expand Down Expand Up @@ -193,8 +159,6 @@ export function requestHandler(
const scope = getCurrentScope();
scope.setSDKProcessingMetadata({
request: req,
// TODO (v8): Stop passing this
requestDataOptionsFromExpressHandler: requestDataOptions,
});

const client = getClient<NodeClient>();
Expand Down Expand Up @@ -372,7 +336,6 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
}

if (isThenable(maybePromiseResult)) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Promise.resolve(maybePromiseResult).then(
nextResult => {
captureIfError(nextResult as any);
Expand All @@ -389,9 +352,3 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
return maybePromiseResult;
};
}

// TODO (v8 / #5257): Remove this
// eslint-disable-next-line deprecation/deprecation
export type { ParseRequestOptions, ExpressRequest } from './requestDataDeprecated';
// eslint-disable-next-line deprecation/deprecation
export { parseRequest, extractRequestData } from './requestDataDeprecated';
54 changes: 0 additions & 54 deletions packages/node/src/requestDataDeprecated.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable max-lines */
import {
endSession,
functionToStringIntegration,
Expand Down
1 change: 0 additions & 1 deletion packages/node/test/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ describe('requestHandler', () => {
const scope = getCurrentScope();
expect((scope as any)._sdkProcessingMetadata).toEqual({
request: req,
requestDataOptionsFromExpressHandler: requestHandlerOptions,
});
});
});
Expand Down
112 changes: 0 additions & 112 deletions packages/node/test/integrations/requestdata.test.ts

This file was deleted.

Loading

0 comments on commit 4ef3bd5

Please sign in to comment.