Skip to content

fix(node): Allow ParseRequestOptions to be passed to request handler #5287

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.

Sign up for GitHub

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 1 commit into from
Jun 22, 2022
Merged

fix(node): Allow ParseRequestOptions to be passed to request handler #5287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 20 additions & 4 deletions packages/node/src/handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import * as domain from 'domain';
import * as http from 'http';

import { NodeClient } from './client';
// TODO (v8 / XXX) Remove these imports
import type { ParseRequestOptions } from './requestDataDeprecated';
import { parseRequest } from './requestDataDeprecated';
import { addRequestDataToEvent, extractRequestData, flush, isAutoSessionTrackingEnabled } from './sdk';

/**
Expand Down Expand Up @@ -72,9 +75,12 @@ export function tracingHandler(): (
};
}

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

/**
* Express compatible request handler.
Expand All @@ -96,11 +102,21 @@ export function requestHandler(
scope.setSession();
}
}

return function sentryRequestMiddleware(
req: http.IncomingMessage,
res: http.ServerResponse,
next: (error?: any) => void,
): void {
// TODO (v8 / XXX) Remove this shim and just use `addRequestDataToEvent`
let backwardsCompatibleEventProcessor: (event: Event) => Event;
if (options && 'include' in options) {
backwardsCompatibleEventProcessor = (event: Event) => addRequestDataToEvent(event, req, options);
} else {
// eslint-disable-next-line deprecation/deprecation
backwardsCompatibleEventProcessor = (event: Event) => parseRequest(event, req, options as ParseRequestOptions);
}

if (options && options.flushTimeout && options.flushTimeout > 0) {
// eslint-disable-next-line @typescript-eslint/unbound-method
const _end = res.end;
Expand All @@ -124,7 +140,7 @@ export function requestHandler(
const currentHub = getCurrentHub();

currentHub.configureScope(scope => {
scope.addEventProcessor((event: Event) => addRequestDataToEvent(event, req, options));
scope.addEventProcessor(backwardsCompatibleEventProcessor);
const client = currentHub.getClient<NodeClient>();
if (isAutoSessionTrackingEnabled(client)) {
const scope = currentHub.getScope();
Expand Down
10 changes: 8 additions & 2 deletions packages/serverless/src/gcpfunction/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ import { domainify, getActiveDomain, proxyFunction } from './../utils';
import { HttpFunction, WrapperOptions } from './general';

// TODO (v8 / #5257): Remove this whole old/new business and just use the new stuff
type ParseRequestOptions = AddRequestDataToEventOptions['include'] & {
serverName?: boolean;
version?: boolean;
};

interface OldHttpFunctionWrapperOptions extends WrapperOptions {
/**
* @deprecated Use `addRequestDataToEventOptions` instead.
*/
parseRequestOptions: AddRequestDataToEventOptions;
parseRequestOptions: ParseRequestOptions;
}
interface NewHttpFunctionWrapperOptions extends WrapperOptions {
addRequestDataToEventOptions: AddRequestDataToEventOptions;
Expand Down Expand Up @@ -58,7 +63,8 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr

const options: HttpFunctionWrapperOptions = {
flushTimeout: 2000,
addRequestDataToEventOptions: parseRequestOptions ? parseRequestOptions : {},
// TODO (v8 / xxx): Remove this line, since `addRequestDataToEventOptions` will be included in the spread of `wrapOptions`
addRequestDataToEventOptions: parseRequestOptions ? { include: parseRequestOptions } : {},
...wrapOptions,
};
return (req, res) => {
Expand Down