Skip to content

Commit af2c002

Browse files
marshall-leemarshall-lee
authored
Wrap google cloud functions with a Proxy(). (#3035)
* Wrap google cloud functions with a Proxy(). Firebase wrappers such as `onRequest`, `onCreate`, etc return callable handlers with some important additional data in properties which is being used by `firebase-cli` while deploying the functions. So when we wrap such functions naively, these properties are not visible anymore. So I decided to circumvent it with a `new Proxy(fn, { apply: ... })` trick. This proxy object will redirect all property accesses to the original function but calling behavior will be re-defined. Also added some workarounds for firebase emulator (`firebase emulators:start`) - Need to wrap `fn.__emulator_func` too. See code comments. - Need to create a new domain because unlike `functions-framework`, firebase emulator doesn't create a domain. Fixes #3023. Also this commit removes a dependency on `@google-cloud/functions-framework` package since it's used only for importing types but these types are also needed for TypeScript users and we cannot force them to install an entired functions-framework as a dependency. Instead, I moved all the types from it right in this package and also add a *runtime* dependency `@types/express`. Fixes #2997. * Move @types/aws-lambda to dependencies. It's not enough to include it only in `devDependencies` because without it TypeScript users are getting type errors.
1 parent 9a23684 commit af2c002

File tree

7 files changed

+154
-42
lines changed

7 files changed

+154
-42
lines changed

‎packages/serverless/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
"@sentry/node": "5.27.3",
2121
"@sentry/types": "5.27.3",
2222
"@sentry/utils": "5.27.3",
23+
"@types/aws-lambda": "^8.10.62",
24+
"@types/express": "^4.17.2",
2325
"tslib": "^1.9.3"
2426
},
2527
"devDependencies": {
@@ -28,7 +30,6 @@
2830
"@google-cloud/functions-framework": "^1.7.1",
2931
"@google-cloud/pubsub": "^2.5.0",
3032
"@sentry-internal/eslint-config-sdk": "5.27.3",
31-
"@types/aws-lambda": "^8.10.62",
3233
"@types/node": "^14.6.4",
3334
"aws-sdk": "^2.765.0",
3435
"eslint": "7.6.0",

‎packages/serverless/src/gcpfunction/cloud_events.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
2-
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
3-
import {
4-
CloudEventFunction,
5-
CloudEventFunctionWithCallback,
6-
} from '@google-cloud/functions-framework/build/src/functions';
71
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
82
import { logger } from '@sentry/utils';
93

10-
import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
4+
import { domainify, getActiveDomain, proxyFunction } from '../utils';
5+
import {
6+
CloudEventFunction,
7+
CloudEventFunctionWithCallback,
8+
configureScopeWithContext,
9+
WrapperOptions,
10+
} from './general';
1111

1212
export type CloudEventFunctionWrapperOptions = WrapperOptions;
1313

@@ -21,6 +21,14 @@ export type CloudEventFunctionWrapperOptions = WrapperOptions;
2121
export function wrapCloudEventFunction(
2222
fn: CloudEventFunction | CloudEventFunctionWithCallback,
2323
wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
24+
): CloudEventFunctionWithCallback {
25+
return proxyFunction(fn, f => domainify(_wrapCloudEventFunction(f, wrapOptions)));
26+
}
27+
28+
/** */
29+
function _wrapCloudEventFunction(
30+
fn: CloudEventFunction | CloudEventFunctionWithCallback,
31+
wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
2432
): CloudEventFunctionWithCallback {
2533
const options: CloudEventFunctionWrapperOptions = {
2634
flushTimeout: 2000,
@@ -41,7 +49,7 @@ export function wrapCloudEventFunction(
4149
scope.setSpan(transaction);
4250
});
4351

44-
const activeDomain = getActiveDomain();
52+
const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
4553

4654
activeDomain.on('error', captureException);
4755

‎packages/serverless/src/gcpfunction/events.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
2-
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
3-
import { EventFunction, EventFunctionWithCallback } from '@google-cloud/functions-framework/build/src/functions';
41
import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
52
import { logger } from '@sentry/utils';
63

7-
import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
4+
import { domainify, getActiveDomain, proxyFunction } from '../utils';
5+
import { configureScopeWithContext, EventFunction, EventFunctionWithCallback, WrapperOptions } from './general';
86

97
export type EventFunctionWrapperOptions = WrapperOptions;
108

@@ -18,6 +16,14 @@ export type EventFunctionWrapperOptions = WrapperOptions;
1816
export function wrapEventFunction(
1917
fn: EventFunction | EventFunctionWithCallback,
2018
wrapOptions: Partial<EventFunctionWrapperOptions> = {},
19+
): EventFunctionWithCallback {
20+
return proxyFunction(fn, f => domainify(_wrapEventFunction(f, wrapOptions)));
21+
}
22+
23+
/** */
24+
function _wrapEventFunction(
25+
fn: EventFunction | EventFunctionWithCallback,
26+
wrapOptions: Partial<EventFunctionWrapperOptions> = {},
2127
): EventFunctionWithCallback {
2228
const options: EventFunctionWrapperOptions = {
2329
flushTimeout: 2000,
@@ -38,7 +44,7 @@ export function wrapEventFunction(
3844
scope.setSpan(transaction);
3945
});
4046

41-
const activeDomain = getActiveDomain();
47+
const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
4248

4349
activeDomain.on('error', captureException);
4450

‎packages/serverless/src/gcpfunction/general.ts‎

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,48 @@
1-
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
2-
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
3-
import { Context } from '@google-cloud/functions-framework/build/src/functions';
41
import { Scope } from '@sentry/node';
52
import { Context as SentryContext } from '@sentry/types';
6-
import * as domain from 'domain';
3+
import { Request, Response } from 'express'; // eslint-disable-line import/no-extraneous-dependencies
74
import { hostname } from 'os';
85

6+
export interface HttpFunction {
7+
(req: Request, res: Response): any; // eslint-disable-line @typescript-eslint/no-explicit-any
8+
}
9+
10+
export interface EventFunction {
11+
(data: Record<string, any>, context: Context): any; // eslint-disable-line @typescript-eslint/no-explicit-any
12+
}
13+
14+
export interface EventFunctionWithCallback {
15+
(data: Record<string, any>, context: Context, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
16+
}
17+
18+
export interface CloudEventFunction {
19+
(cloudevent: CloudEventsContext): any; // eslint-disable-line @typescript-eslint/no-explicit-any
20+
}
21+
22+
export interface CloudEventFunctionWithCallback {
23+
(cloudevent: CloudEventsContext, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
24+
}
25+
26+
export interface CloudFunctionsContext {
27+
eventId?: string;
28+
timestamp?: string;
29+
eventType?: string;
30+
resource?: string;
31+
}
32+
33+
export interface CloudEventsContext {
34+
[key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
35+
type?: string;
36+
specversion?: string;
37+
source?: string;
38+
id?: string;
39+
time?: string;
40+
schemaurl?: string;
41+
contenttype?: string;
42+
}
43+
44+
export type Context = CloudFunctionsContext | CloudEventsContext;
45+
946
export interface WrapperOptions {
1047
flushTimeout: number;
1148
}
@@ -24,11 +61,3 @@ export function configureScopeWithContext(scope: Scope, context: Context): void
2461
scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
2562
scope.setContext('gcp.function.context', { ...context } as SentryContext);
2663
}
27-
28-
/**
29-
* @returns Current active domain with a correct type.
30-
*/
31-
export function getActiveDomain(): domain.Domain {
32-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
33-
return (domain as any).active as domain.Domain;
34-
}

‎packages/serverless/src/gcpfunction/http.ts‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,15 @@
1-
// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
2-
// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
3-
import { HttpFunction } from '@google-cloud/functions-framework/build/src/functions';
41
import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
52
import { logger, stripUrlQueryAndFragment } from '@sentry/utils';
63

7-
import { getActiveDomain, WrapperOptions } from './general';
4+
import { domainify, getActiveDomain, proxyFunction } from './../utils';
5+
import { HttpFunction, WrapperOptions } from './general';
86

9-
type Request = Parameters<HttpFunction>[0];
10-
type Response = Parameters<HttpFunction>[1];
117
type ParseRequestOptions = Handlers.ParseRequestOptions;
128

139
export interface HttpFunctionWrapperOptions extends WrapperOptions {
1410
parseRequestOptions: ParseRequestOptions;
1511
}
1612

17-
export { Request, Response };
18-
1913
const { parseRequest } = Handlers;
2014

2115
/**
@@ -29,14 +23,30 @@ export function wrapHttpFunction(
2923
fn: HttpFunction,
3024
wrapOptions: Partial<HttpFunctionWrapperOptions> = {},
3125
): HttpFunction {
26+
const wrap = (f: HttpFunction): HttpFunction => domainify(_wrapHttpFunction(f, wrapOptions));
27+
28+
let overrides: Record<PropertyKey, unknown> | undefined;
29+
30+
// Functions emulator from firebase-tools has a hack-ish workaround that saves the actual function
31+
// passed to `onRequest(...)` and in fact runs it so we need to wrap it too.
32+
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
33+
const emulatorFunc = (fn as any).__emulator_func as HttpFunction | undefined;
34+
if (emulatorFunc) {
35+
overrides = { __emulator_func: proxyFunction(emulatorFunc, wrap) };
36+
}
37+
return proxyFunction(fn, wrap, overrides);
38+
}
39+
40+
/** */
41+
function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWrapperOptions> = {}): HttpFunction {
3242
const options: HttpFunctionWrapperOptions = {
3343
flushTimeout: 2000,
3444
parseRequestOptions: {},
3545
...wrapOptions,
3646
};
3747
return (req, res) => {
3848
const reqMethod = (req.method || '').toUpperCase();
39-
const reqUrl = req.url && stripUrlQueryAndFragment(req.url);
49+
const reqUrl = stripUrlQueryAndFragment(req.originalUrl || req.url || '');
4050

4151
const transaction = startTransaction({
4252
name: `${reqMethod} ${reqUrl}`,
@@ -59,7 +69,8 @@ export function wrapHttpFunction(
5969

6070
// functions-framework creates a domain for each incoming request so we take advantage of this fact and add an error handler.
6171
// BTW this is the only way to catch any exception occured during request lifecycle.
62-
getActiveDomain().on('error', err => {
72+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
73+
getActiveDomain()!.on('error', err => {
6374
captureException(err);
6475
});
6576

‎packages/serverless/src/utils.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Event, SDK_VERSION } from '@sentry/node';
22
import { addExceptionMechanism } from '@sentry/utils';
3+
import * as domain from 'domain';
34

45
/**
56
* Event processor that will override SDK details to point to the serverless SDK instead of Node,
@@ -31,3 +32,57 @@ export function serverlessEventProcessor(integration: string): (event: Event) =>
3132
return event;
3233
};
3334
}
35+
36+
/**
37+
* @returns Current active domain with a correct type.
38+
*/
39+
export function getActiveDomain(): domain.Domain | null {
40+
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
41+
return (domain as any).active as domain.Domain | null;
42+
}
43+
44+
/**
45+
* @param fn function to run
46+
* @returns function which runs in the newly created domain or in the existing one
47+
*/
48+
export function domainify<A extends unknown[], R>(fn: (...args: A) => R): (...args: A) => R {
49+
return (...args) => {
50+
if (getActiveDomain()) {
51+
return fn(...args);
52+
}
53+
const dom = domain.create();
54+
return dom.run(() => fn(...args));
55+
};
56+
}
57+
58+
/**
59+
* @param source function to be wrapped
60+
* @param wrap wrapping function that takes source and returns a wrapper
61+
* @param overrides properties to override in the source
62+
* @returns wrapped function
63+
*/
64+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
65+
export function proxyFunction<A extends any[], R, F extends (...args: A) => R>(
66+
source: F,
67+
wrap: (source: F) => F,
68+
overrides?: Record<PropertyKey, unknown>,
69+
): F {
70+
const wrapper = wrap(source);
71+
const handler: ProxyHandler<F> = {
72+
apply: <T>(_target: F, thisArg: T, args: A) => {
73+
return wrapper.apply(thisArg, args);
74+
},
75+
};
76+
77+
if (overrides) {
78+
handler.get = (target, prop) => {
79+
// eslint-disable-next-line no-prototype-builtins
80+
if (overrides.hasOwnProperty(prop)) {
81+
return overrides[prop as string];
82+
}
83+
return (target as Record<PropertyKey, unknown>)[prop as string];
84+
};
85+
}
86+
87+
return new Proxy(source, handler);
88+
}

‎packages/serverless/test/gcpfunction.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
1+
import { Event } from '@sentry/types';
2+
import * as domain from 'domain';
3+
4+
import * as Sentry from '../src';
5+
import { wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
16
import {
27
CloudEventFunction,
38
CloudEventFunctionWithCallback,
49
EventFunction,
510
EventFunctionWithCallback,
611
HttpFunction,
7-
} from '@google-cloud/functions-framework/build/src/functions';
8-
import { Event } from '@sentry/types';
9-
import * as domain from 'domain';
10-
11-
import * as Sentry from '../src';
12-
import { Request, Response, wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
12+
Request,
13+
Response,
14+
} from '../src/gcpfunction/general';
1315

1416
/**
1517
* Why @ts-ignore some Sentry.X calls

0 commit comments

Comments
 (0)