Wrap google cloud functions with a Proxy().

marshall-lee · marshall-lee · commit cbb6dca7986d · 2020-11-05T17:16:57.000+03:00
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.
diff --git a/packages/serverless/package.json b/packages/serverless/package.json
@@ -20,6 +20,7 @@
     "@sentry/node": "5.27.3",
     "@sentry/types": "5.27.3",
     "@sentry/utils": "5.27.3",
+    "@types/express": "^4.17.2",
     "tslib": "^1.9.3"
   },
   "devDependencies": {
diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts
@@ -1,13 +1,13 @@
-// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
-// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
-import {
-  CloudEventFunction,
-  CloudEventFunctionWithCallback,
-} from '@google-cloud/functions-framework/build/src/functions';
 import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
-import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
+import { domainify, getActiveDomain, proxyFunction } from '../utils';
+import {
+  CloudEventFunction,
+  CloudEventFunctionWithCallback,
+  configureScopeWithContext,
+  WrapperOptions,
+} from './general';
 
 export type CloudEventFunctionWrapperOptions = WrapperOptions;
 
@@ -21,6 +21,14 @@ export type CloudEventFunctionWrapperOptions = WrapperOptions;
 export function wrapCloudEventFunction(
   fn: CloudEventFunction | CloudEventFunctionWithCallback,
   wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
+): CloudEventFunctionWithCallback {
+  return proxyFunction(fn, f => domainify(_wrapCloudEventFunction(f, wrapOptions)));
+}
+
+/** */
+function _wrapCloudEventFunction(
+  fn: CloudEventFunction | CloudEventFunctionWithCallback,
+  wrapOptions: Partial<CloudEventFunctionWrapperOptions> = {},
 ): CloudEventFunctionWithCallback {
   const options: CloudEventFunctionWrapperOptions = {
     flushTimeout: 2000,
@@ -41,7 +49,7 @@ export function wrapCloudEventFunction(
       scope.setSpan(transaction);
     });
 
-    const activeDomain = getActiveDomain();
+    const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
 
     activeDomain.on('error', captureException);
 
diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts
@@ -1,10 +1,8 @@
-// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
-// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
-import { EventFunction, EventFunctionWithCallback } from '@google-cloud/functions-framework/build/src/functions';
 import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
-import { configureScopeWithContext, getActiveDomain, WrapperOptions } from './general';
+import { domainify, getActiveDomain, proxyFunction } from '../utils';
+import { configureScopeWithContext, EventFunction, EventFunctionWithCallback, WrapperOptions } from './general';
 
 export type EventFunctionWrapperOptions = WrapperOptions;
 
@@ -18,6 +16,14 @@ export type EventFunctionWrapperOptions = WrapperOptions;
 export function wrapEventFunction(
   fn: EventFunction | EventFunctionWithCallback,
   wrapOptions: Partial<EventFunctionWrapperOptions> = {},
+): EventFunctionWithCallback {
+  return proxyFunction(fn, f => domainify(_wrapEventFunction(f, wrapOptions)));
+}
+
+/** */
+function _wrapEventFunction(
+  fn: EventFunction | EventFunctionWithCallback,
+  wrapOptions: Partial<EventFunctionWrapperOptions> = {},
 ): EventFunctionWithCallback {
   const options: EventFunctionWrapperOptions = {
     flushTimeout: 2000,
@@ -38,7 +44,7 @@ export function wrapEventFunction(
       scope.setSpan(transaction);
     });
 
-    const activeDomain = getActiveDomain();
+    const activeDomain = getActiveDomain()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
 
     activeDomain.on('error', captureException);
 
diff --git a/packages/serverless/src/gcpfunction/general.ts b/packages/serverless/src/gcpfunction/general.ts
@@ -1,11 +1,48 @@
-// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
-// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
-import { Context } from '@google-cloud/functions-framework/build/src/functions';
 import { Scope } from '@sentry/node';
 import { Context as SentryContext } from '@sentry/types';
-import * as domain from 'domain';
+import { Request, Response } from 'express'; // eslint-disable-line import/no-extraneous-dependencies
 import { hostname } from 'os';
 
+export interface HttpFunction {
+  (req: Request, res: Response): any; // eslint-disable-line @typescript-eslint/no-explicit-any
+}
+
+export interface EventFunction {
+  (data: Record<string, any>, context: Context): any; // eslint-disable-line @typescript-eslint/no-explicit-any
+}
+
+export interface EventFunctionWithCallback {
+  (data: Record<string, any>, context: Context, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
+}
+
+export interface CloudEventFunction {
+  (cloudevent: CloudEventsContext): any; // eslint-disable-line @typescript-eslint/no-explicit-any
+}
+
+export interface CloudEventFunctionWithCallback {
+  (cloudevent: CloudEventsContext, callback: Function): any; // eslint-disable-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types
+}
+
+export interface CloudFunctionsContext {
+  eventId?: string;
+  timestamp?: string;
+  eventType?: string;
+  resource?: string;
+}
+
+export interface CloudEventsContext {
+  [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any
+  type?: string;
+  specversion?: string;
+  source?: string;
+  id?: string;
+  time?: string;
+  schemaurl?: string;
+  contenttype?: string;
+}
+
+export type Context = CloudFunctionsContext | CloudEventsContext;
+
 export interface WrapperOptions {
   flushTimeout: number;
 }
@@ -24,11 +61,3 @@ export function configureScopeWithContext(scope: Scope, context: Context): void
   scope.setTag('server_name', process.env.SENTRY_NAME || hostname());
   scope.setContext('gcp.function.context', { ...context } as SentryContext);
 }
-
-/**
- * @returns Current active domain with a correct type.
- */
-export function getActiveDomain(): domain.Domain {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
-  return (domain as any).active as domain.Domain;
-}
diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts
@@ -1,21 +1,15 @@
-// '@google-cloud/functions-framework/build/src/functions' import is expected to be type-only so it's erased in the final .js file.
-// When TypeScript compiler is upgraded, use `import type` syntax to explicitly assert that we don't want to load a module here.
-import { HttpFunction } from '@google-cloud/functions-framework/build/src/functions';
 import { captureException, flush, getCurrentHub, Handlers, startTransaction } from '@sentry/node';
 import { logger, stripUrlQueryAndFragment } from '@sentry/utils';
 
-import { getActiveDomain, WrapperOptions } from './general';
+import { domainify, getActiveDomain, proxyFunction } from './../utils';
+import { HttpFunction, WrapperOptions } from './general';
 
-type Request = Parameters<HttpFunction>[0];
-type Response = Parameters<HttpFunction>[1];
 type ParseRequestOptions = Handlers.ParseRequestOptions;
 
 export interface HttpFunctionWrapperOptions extends WrapperOptions {
   parseRequestOptions: ParseRequestOptions;
 }
 
-export { Request, Response };
-
 const { parseRequest } = Handlers;
 
 /**
@@ -29,14 +23,30 @@ export function wrapHttpFunction(
   fn: HttpFunction,
   wrapOptions: Partial<HttpFunctionWrapperOptions> = {},
 ): HttpFunction {
+  const wrap = (f: HttpFunction): HttpFunction => domainify(_wrapHttpFunction(f, wrapOptions));
+
+  let overrides: Record<PropertyKey, unknown> | undefined;
+
+  // Functions emulator from firebase-tools has a hack-ish workaround that saves the actual function
+  // passed to `onRequest(...)` and in fact runs it so we need to wrap it too.
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
+  const emulatorFunc = (fn as any).__emulator_func as HttpFunction | undefined;
+  if (emulatorFunc) {
+    overrides = { __emulator_func: proxyFunction(emulatorFunc, wrap) };
+  }
+  return proxyFunction(fn, wrap, overrides);
+}
+
+/** */
+function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWrapperOptions> = {}): HttpFunction {
   const options: HttpFunctionWrapperOptions = {
     flushTimeout: 2000,
     parseRequestOptions: {},
     ...wrapOptions,
   };
   return (req, res) => {
     const reqMethod = (req.method || '').toUpperCase();
-    const reqUrl = req.url && stripUrlQueryAndFragment(req.url);
+    const reqUrl = stripUrlQueryAndFragment(req.originalUrl || req.url || '');
 
     const transaction = startTransaction({
       name: `${reqMethod} ${reqUrl}`,
@@ -59,7 +69,8 @@ export function wrapHttpFunction(
 
     // functions-framework creates a domain for each incoming request so we take advantage of this fact and add an error handler.
     // BTW this is the only way to catch any exception occured during request lifecycle.
-    getActiveDomain().on('error', err => {
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    getActiveDomain()!.on('error', err => {
       captureException(err);
     });
 
diff --git a/packages/serverless/src/utils.ts b/packages/serverless/src/utils.ts
@@ -1,5 +1,6 @@
 import { Event, SDK_VERSION } from '@sentry/node';
 import { addExceptionMechanism } from '@sentry/utils';
+import * as domain from 'domain';
 
 /**
  * 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) =>
     return event;
   };
 }
+
+/**
+ * @returns Current active domain with a correct type.
+ */
+export function getActiveDomain(): domain.Domain | null {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
+  return (domain as any).active as domain.Domain | null;
+}
+
+/**
+ * @param fn function to run
+ * @returns function which runs in the newly created domain or in the existing one
+ */
+export function domainify<A extends unknown[], R>(fn: (...args: A) => R): (...args: A) => R {
+  return (...args) => {
+    if (getActiveDomain()) {
+      return fn(...args);
+    }
+    const dom = domain.create();
+    return dom.run(() => fn(...args));
+  };
+}
+
+/**
+ * @param source function to be wrapped
+ * @param wrap wrapping function that takes source and returns a wrapper
+ * @param overrides properties to override in the source
+ * @returns wrapped function
+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export function proxyFunction<A extends any[], R, F extends (...args: A) => R>(
+  source: F,
+  wrap: (source: F) => F,
+  overrides?: Record<PropertyKey, unknown>,
+): F {
+  const wrapper = wrap(source);
+  const handler: ProxyHandler<F> = {
+    apply: <T>(_target: F, thisArg: T, args: A) => {
+      return wrapper.apply(thisArg, args);
+    },
+  };
+
+  if (overrides) {
+    handler.get = (target, prop) => {
+      // eslint-disable-next-line no-prototype-builtins
+      if (overrides.hasOwnProperty(prop)) {
+        return overrides[prop as string];
+      }
+      return (target as Record<PropertyKey, unknown>)[prop as string];
+    };
+  }
+
+  return new Proxy(source, handler);
+}
diff --git a/packages/serverless/test/gcpfunction.test.ts b/packages/serverless/test/gcpfunction.test.ts
@@ -1,15 +1,17 @@
+import { Event } from '@sentry/types';
+import * as domain from 'domain';
+
+import * as Sentry from '../src';
+import { wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
 import {
   CloudEventFunction,
   CloudEventFunctionWithCallback,
   EventFunction,
   EventFunctionWithCallback,
   HttpFunction,
-} from '@google-cloud/functions-framework/build/src/functions';
-import { Event } from '@sentry/types';
-import * as domain from 'domain';
-
-import * as Sentry from '../src';
-import { Request, Response, wrapCloudEventFunction, wrapEventFunction, wrapHttpFunction } from '../src/gcpfunction';
+  Request,
+  Response,
+} from '../src/gcpfunction/general';
 
 /**
  * Why @ts-ignore some Sentry.X calls
