Skip to content

feat(v8): Remove requestData deprecations #10626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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