Allow specifying failure policies (#482)

* Define an interface for FailurePolicy

* Extract functions config to avoid dependency cycles, assign failure policy to triggers

* Add a changelog entry

* Update dependencies, minor version bump

* Reformat

* Fix a typo: allows -> allow

* Avoid abbreviations to improve readability

* Rename remaining Opts to Options

* Add tests for specifying failure policy

* Reformat

* Change format of an entry in the changelog

* Revert version bump

* Extract configuration to break dependency cycle

* Add comments to Schedule and ScheduleRetryConfig

* Stricten regions definition

* Fix tests broken due to strict typings

* More strict types for regions - reverse order

* Reformat

* Remove unused import

* Conform with npm formatting

* Reintroduce unused variable to minimize diff size

* Import lodash using _ exclusively

* Kepp @hidden tags in one line if no other JSDoc is present

* Fix a typo - sentence ending with a comma

* Wrap JSDoc as specified in Google JavaScript Style Guide

* Move memory lookup table to functions-configuration and stricten type definition

* Move default failure policy to functions-configuration

* Separate standarization of options from construction of the options object

* Rephrase failure policy description

* Rephrase description of memory and timeoutSeconds options

* Simplify tests for invalid failure policies

* Align public description of failure policies with a documentation of its interface

* Add a missing dot in the Changelog

* Minor stylistic changes to the documentation

* Make a test case more understandable

* Reformat

Author: merlinnot

changelog.txt

@@ -0,0 +1 @@
+feature - Allow specifying retry policies for event triggered functions.

spec/function-builder.spec.ts

@@ -79,14 +79,29 @@ describe('FunctionBuilder', () => {
   it('should allow valid runtime options to be set', () => {
     const fn = functions
       .runWith({
-        timeoutSeconds: 90,
+        failurePolicy: { retry: {} },
         memory: '256MB',
+        timeoutSeconds: 90,
       })
       .auth.user()
       .onCreate((user) => user);
 
     expect(fn.__trigger.availableMemoryMb).to.deep.equal(256);
     expect(fn.__trigger.timeout).to.deep.equal('90s');
+    expect(fn.__trigger.failurePolicy).to.deep.equal({ retry: {} });
+  });
+
+  it("should apply a default failure policy if it's aliased with `true`", () => {
+    const fn = functions
+      .runWith({
+        failurePolicy: true,
+        memory: '256MB',
+        timeoutSeconds: 90,
+      })
+      .auth.user()
+      .onCreate((user) => user);
+
+    expect(fn.__trigger.failurePolicy).to.deep.equal({ retry: {} });
   });
 
   it('should allow both supported region and valid runtime options to be set', () => {
@@ -132,7 +147,26 @@ describe('FunctionBuilder', () => {
       functions
         .region('asia-northeast1')
         .runWith({ timeoutSeconds: 600, memory: '256MB' });
-    }).to.throw(Error, 'TimeoutSeconds');
+    }).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
+  });
+
+  it('should throw an error if user chooses a failurePolicy which is neither an object nor a boolean', () => {
+    expect(() =>
+      functions.runWith({
+        failurePolicy: (1234 as unknown) as functions.RuntimeOptions['failurePolicy'],
+      })
+    ).to.throw(
+      Error,
+      'RuntimeOptions.failurePolicy must be a boolean or an object'
+    );
+  });
+
+  it('should throw an error if user chooses a failurePolicy.retry which is not an object', () => {
+    expect(() =>
+      functions.runWith({
+        failurePolicy: { retry: (1234 as unknown) as object },
+      })
+    ).to.throw(Error, 'RuntimeOptions.failurePolicy.retry');
   });
 
   it('should throw an error if user chooses an invalid memory allocation', () => {
@@ -154,13 +188,13 @@ describe('FunctionBuilder', () => {
       return functions.runWith({
         timeoutSeconds: 1000000,
       } as any);
-    }).to.throw(Error, 'TimeoutSeconds');
+    }).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
 
     expect(() => {
       return functions.region('asia-east2').runWith({
         timeoutSeconds: 1000000,
       } as any);
-    }).to.throw(Error, 'TimeoutSeconds');
+    }).to.throw(Error, 'RuntimeOptions.timeoutSeconds');
   });
 
   it('should throw an error if user chooses an invalid region', () => {

spec/providers/auth.spec.ts

@@ -197,9 +197,7 @@ describe('Auth Functions', () => {
     });
 
     describe('#onDelete', () => {
-      const cloudFunctionDelete: CloudFunction<
-        firebase.auth.UserRecord
-      > = functions.handler.auth.user.onDelete(
+      const cloudFunctionDelete: CloudFunction<firebase.auth.UserRecord> = functions.handler.auth.user.onDelete(
         (data: firebase.auth.UserRecord) => data
       );
 

src/cloud-functions.ts

@@ -22,7 +22,13 @@
 
 import { Request, Response } from 'express';
 import * as _ from 'lodash';
-import { DeploymentOptions, Schedule } from './function-configuration';
+import {
+  DEFAULT_FAILURE_POLICY,
+  DeploymentOptions,
+  FailurePolicy,
+  MEMORY_LOOKUP,
+  Schedule,
+} from './function-configuration';
 export { Request, Response };
 
 /** @hidden */
@@ -202,6 +208,7 @@ export namespace Change {
     if (json.fieldMask) {
       before = applyFieldMask(before, json.after, json.fieldMask);
     }
+
     return Change.fromObjects(
       customizer(before || {}),
       customizer(json.after || {})
@@ -216,14 +223,16 @@ export namespace Change {
   ) {
     const before = _.assign({}, after);
     const masks = fieldMask.split(',');
-    _.forEach(masks, (mask) => {
+
+    masks.forEach((mask) => {
       const val = _.get(sparseBefore, mask);
       if (typeof val === 'undefined') {
         _.unset(before, mask);
       } else {
         _.set(before, mask, val);
       }
     });
+
     return before;
   }
 }
@@ -253,6 +262,7 @@ export interface TriggerAnnotated {
       resource: string;
       service: string;
     };
+    failurePolicy?: FailurePolicy;
     httpsTrigger?: {};
     labels?: { [key: string]: string };
     regions?: string[];
@@ -312,6 +322,40 @@ export interface MakeCloudFunctionArgs<EventData> {
   triggerResource: () => string;
 }
 
+/** @hidden */
+export function optionsToTrigger({
+  failurePolicy: failurePolicyOrAlias,
+  memory,
+  regions,
+  schedule,
+  timeoutSeconds,
+}: DeploymentOptions): TriggerAnnotated['__trigger'] {
+  /*
+   * FailurePolicy can be aliased with a boolean value in the public API.
+   * Convert aliases `true` and `false` to a standardized interface.
+   */
+  const failurePolicy: FailurePolicy | undefined =
+    failurePolicyOrAlias === false
+      ? undefined
+      : failurePolicyOrAlias === true
+      ? DEFAULT_FAILURE_POLICY
+      : failurePolicyOrAlias;
+
+  const availableMemoryMb: number | undefined =
+    memory === undefined ? undefined : MEMORY_LOOKUP[memory];
+
+  const timeout: string | undefined =
+    timeoutSeconds === undefined ? undefined : `${timeoutSeconds}s`;
+
+  return {
+    ...(failurePolicy === undefined ? {} : { failurePolicy }),
+    ...(availableMemoryMb === undefined ? {} : { availableMemoryMb }),
+    ...(regions === undefined ? {} : { regions }),
+    ...(schedule === undefined ? {} : { schedule }),
+    ...(timeout === undefined ? {} : { timeout }),
+  };
+}
+
 /** @hidden */
 export function makeCloudFunction<EventData>({
   after = () => {},
@@ -463,28 +507,3 @@ function _detectAuthType(event: Event) {
   }
   return 'UNAUTHENTICATED';
 }
-
-/** @hidden */
-export function optionsToTrigger(options: DeploymentOptions) {
-  const trigger: any = {};
-  if (options.regions) {
-    trigger.regions = options.regions;
-  }
-  if (options.timeoutSeconds) {
-    trigger.timeout = options.timeoutSeconds.toString() + 's';
-  }
-  if (options.memory) {
-    const memoryLookup = {
-      '128MB': 128,
-      '256MB': 256,
-      '512MB': 512,
-      '1GB': 1024,
-      '2GB': 2048,
-    };
-    trigger.availableMemoryMb = _.get(memoryLookup, options.memory);
-  }
-  if (options.schedule) {
-    trigger.schedule = options.schedule;
-  }
-  return trigger;
-}