wrap alert validation in task runner to prevent failing the task

gmmorris · Dec 23, 2019 · d5991af · d5991af
1 parent 628b4c9
commit d5991af
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 19 deletions.
diff --git a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts
@@ -257,8 +257,17 @@ describe('Task Runner Factory', () => {
       },
       references: [],
     });
-    await expect(taskRunner.run()).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"params invalid: [param1]: expected value of type [string] but got [undefined]"`
+    expect(await taskRunner.run()).toMatchInlineSnapshot(`
+      Object {
+        "runAt": 1970-01-01T00:00:10.000Z,
+        "state": Object {
+          "previousStartedAt": 1970-01-01T00:00:00.000Z,
+          "startedAt": 1969-12-31T23:55:00.000Z,
+        },
+      }
+    `);
+    expect(taskRunnerFactoryInitializerParams.logger.error).toHaveBeenCalledWith(
+      `Executing Alert \"1\" has resulted in Error: params invalid: [param1]: expected value of type [string] but got [undefined]`
     );
   });
 
@@ -278,8 +287,17 @@ describe('Task Runner Factory', () => {
       },
       references: [],
     });
-    await expect(taskRunner.run()).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"Action reference \\"action_0\\" not found in alert id: 1"`
+    expect(await taskRunner.run()).toMatchInlineSnapshot(`
+      Object {
+        "runAt": 1970-01-01T00:00:10.000Z,
+        "state": Object {
+          "previousStartedAt": 1970-01-01T00:00:00.000Z,
+          "startedAt": 1969-12-31T23:55:00.000Z,
+        },
+      }
+    `);
+    expect(taskRunnerFactoryInitializerParams.logger.error).toHaveBeenCalledWith(
+      `Executing Alert \"1\" has resulted in Error: Action reference \"action_0\" not found in alert id: 1`
     );
   });
 

diff --git a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts
@@ -153,7 +153,7 @@ export class TaskRunnerFactory {
       ): Promise<State> {
         const {
           params: { alertId },
-          state: { alertInstances: alertRawInstances = {}, alertTypeState = {} },
+          state: { alertInstances: alertRawInstances = {}, alertTypeState = {}, previousStartedAt },
         } = taskInstance;
 
         const alertInstances = mapValues<AlertInstances>(
@@ -170,7 +170,7 @@ export class TaskRunnerFactory {
           params,
           state: alertTypeState,
           startedAt: taskInstance.startedAt!,
-          previousStartedAt: taskInstance.state.previousStartedAt,
+          previousStartedAt,
         });
 
         // Cleanup alert instances that are no longer scheduling actions to avoid over populating the alertInstances object
@@ -202,10 +202,33 @@ export class TaskRunnerFactory {
         };
       },
 
+      async validateAndRunAlert(
+        services: Services,
+        apiKey: string | null,
+        attributes: SavedObject['attributes'],
+        references: SavedObject['references']
+      ) {
+        const {
+          params: { alertId, spaceId },
+        } = taskInstance;
+
+        // Validate
+        const params = validateAlertTypeParams(alertType, attributes.params);
+        const executionHandler = this.getExecutionHandler(
+          alertId,
+          spaceId,
+          apiKey,
+          attributes.actions,
+          references
+        );
+        return this.executeAlertInstances(services, { ...attributes, params }, executionHandler);
+      },
+
       async run() {
         const {
           params: { alertId, spaceId },
           startedAt: previousStartedAt,
+          state: originalState,
         } = taskInstance;
 
         const apiKey = await this.getApiKeyForAlertPermissions(alertId, spaceId);
@@ -217,20 +240,10 @@ export class TaskRunnerFactory {
           alertId
         );
 
-        // Validate
-        const params = validateAlertTypeParams(alertType, attributes.params);
-        const executionHandler = this.getExecutionHandler(
-          alertId,
-          spaceId,
-          apiKey,
-          attributes.actions,
-          references
-        );
-
         return {
           state: map<State, Error, State>(
             await promiseResult<State, Error>(
-              this.executeAlertInstances(services, { ...attributes, params }, executionHandler)
+              this.validateAndRunAlert(services, apiKey, attributes, references)
             ),
             (stateUpdates: State) => {
               return {
@@ -239,9 +252,9 @@ export class TaskRunnerFactory {
               };
             },
             (err: Error) => {
-              logger.error(`Executing Alert "${alertId}" has resulted in Error: ${err.message}.`);
+              logger.error(`Executing Alert "${alertId}" has resulted in Error: ${err.message}`);
               return {
-                ...taskInstance.state,
+                ...originalState,
                 previousStartedAt,
               };
             }