fix(watch): Add WatchErrors to capture invalid watches

Instead of immediately throwing an error if a watch Action is invalid (email for example), we can now pass an option object to the fromUpstreamJson() method and receive back any error in the Watch.
The JSON object has now a "watchErrors" object that will be used in the UI to display an error message to the user.

fix(watch): Add models in public code to handle watch errors

fix(watcher): Add temp styling for error messages

fix(watcher): Set actionErrors to null instead of empty object when no error

test(watcher): Fix broken tests after adding watchErrors prop on the Watch

test(watcher): Add test for WatchError class

fix(watcher): Add Watch & Action state for CONFIG_ERROR

fix(watcher): move inline style to its scss file

x-pack/plugins/watcher/common/constants/action_states.js

@@ -19,6 +19,9 @@ export const ACTION_STATES = {
   FIRING: 'Firing',
 
   // Action has failed
-  ERROR: 'Error'
+  ERROR: 'Error',
+
+  // Action has a configuration error
+  CONFIG_ERROR: 'Config Error'
 
 };

x-pack/plugins/watcher/common/constants/watch_states.js

@@ -12,6 +12,7 @@ export const WATCH_STATES = {
 
   FIRING: 'Firing',
 
-  ERROR: 'Error!'
+  ERROR: 'Error!',
 
+  CONFIG_ERROR: 'Config Error!',
 };

x-pack/plugins/watcher/public/components/action_state_icon/action_state_icon.html

@@ -17,6 +17,6 @@
   ></div>
   <div
     class="kuiIcon kuiIcon--error fa-exclamation-triangle"
-    ng-if="actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.ERROR"
+    ng-if="actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.ERROR || actionStateIcon.actionStatus.state === actionStateIcon.ACTION_STATES.CONFIG_ERROR"
   ></div>
 </div>

x-pack/plugins/watcher/public/components/watch_state_icon/watch_state_icon.html

@@ -13,6 +13,6 @@
   ></div>
   <div
     class="kuiIcon kuiIcon--error fa-exclamation-triangle"
-    ng-if="watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.ERROR"
+    ng-if="watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.ERROR || watchStateIcon.watchStatus.state === watchStateIcon.WATCH_STATES.CONFIG_ERROR"
   ></div>
 </div>

x-pack/plugins/watcher/public/index.scss

@@ -26,3 +26,4 @@
 @import 'sections/watch_edit/components/watch_edit_detail/index';
 @import 'sections/watch_edit/components/watch_edit_execute_detail/index';
 @import 'sections/watch_edit/components/watch_edit_title_panel/index';
+@import 'sections/watch_detail/components/action_status_table/index';

x-pack/plugins/watcher/public/models/watch/base_watch.js

@@ -8,6 +8,7 @@ import { getSearchValue } from 'plugins/watcher/lib/get_search_value';
 import { get, isEqual, remove, map, merge } from 'lodash';
 import { Action } from '../action';
 import { WatchStatus } from '../watch_status';
+import { WatchErrors } from '../watch_errors';
 import { createActionId } from './lib/create_action_id';
 import { checkActionIdCollision } from './lib/check_action_id_collision';
 
@@ -30,6 +31,7 @@ export class BaseWatch {
     this.name = get(props, 'name', '');
     this.isSystemWatch = Boolean(get(props, 'isSystemWatch'));
     this.watchStatus = WatchStatus.fromUpstreamJson(get(props, 'watchStatus'));
+    this.watchErrors = WatchErrors.fromUpstreamJson(get(props, 'watchErrors'));
 
     const actions = get(props, 'actions', []);
     this.actions = actions.map(Action.fromUpstreamJson);

x-pack/plugins/watcher/public/models/watch_errors/index.js

@@ -0,0 +1,7 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+export { WatchErrors } from './watch_errors';

x-pack/plugins/watcher/public/models/watch_errors/watch_errors.js

@@ -0,0 +1,17 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { get } from 'lodash';
+
+export class WatchErrors {
+  constructor(props = {}) {
+    this.actionErrors = get(props, 'actions');
+  }
+
+  static fromUpstreamJson(upstreamWatchStatus) {
+    return new WatchErrors(upstreamWatchStatus);
+  }
+}

x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/_action_status_table.scss

@@ -0,0 +1,23 @@
+/**
+ * Action status table
+ *
+ * 1. So we can display the whole error message on multiple lines
+ */
+
+ .watcher__action-status-table {
+    &__errors {
+        white-space: initial; /* [1] */
+    }
+
+    &__list-errors {
+        li {
+            padding-bottom: $euiSizeXS * 0.5;
+            margin-bottom: $euiSizeXS * 0.5;
+            border-bottom: $euiBorderThin;
+
+            &:last-child {
+                border-bottom: none;
+            }
+        }
+    }
+}

x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/_index.scss

@@ -0,0 +1 @@
+@import 'action_status_table';

x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.html

@@ -21,6 +21,9 @@
           State
         </sortable-column>
       </th>
+      <th scope="col" class="kuiTableHeaderCell" ng-if="actionStatusTable.actionErrors">
+          <span class="kuiTableHeaderCell__liner">Errors</span>
+      </th>
       <th scope="col" class="kuiTableHeaderCell">
       </th>
     </tr>
@@ -39,6 +42,15 @@
           {{ actionStatus.state }}
         </div>
       </td>
+      <td class="kuiTableRowCell">
+        <div class="kuiTableRowCell__liner watcher__action-status-table__errors">
+          <ul
+            ng-if="actionStatusTable.actionErrors && actionStatusTable.actionErrors[actionStatus.id]"
+            class="watcher__action-status-table__list-errors">
+            <li ng-repeat="error in actionStatusTable.actionErrors[actionStatus.id]">{{ error.message }}</li>
+          </ul>
+        </div>
+      </td>
       <td class="kuiTableRowCell">
         <div class="kuiTableRowCell__liner">
           <div class="kuiMenuButtonGroup kuiMenuButtonGroup--alignRight">

x-pack/plugins/watcher/public/sections/watch_detail/components/action_status_table/action_status_table.js

@@ -16,6 +16,7 @@ app.directive('actionStatusTable', function () {
     template: template,
     scope: {
       actionStatuses: '=',
+      actionErrors: '=',
       sortField: '=',
       sortReverse: '=',
       onSortChange: '=',

x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.html

@@ -51,6 +51,7 @@ <h1 class="kuiTitle">
       <!-- Action status table -->
       <action-status-table
         action-statuses="watchDetail.sortedActionStatuses"
+        action-errors="watchDetail.actionErrors"
         on-action-acknowledge="watchDetail.onActionAcknowledge"
         sort-field="watchDetail.actionStatusTableSortField"
         sort-reverse="watchDetail.actionStatusTableSortReverse"

x-pack/plugins/watcher/public/sections/watch_detail/components/watch_detail/watch_detail.js

@@ -54,38 +54,39 @@ app.directive('watchDetail', function ($injector) {
 
         this.actionStatusTableSortField = 'id';
         this.actionStatusTableSortReverse = false;
+        this.actionErrors = (this.watch.watchErrors && this.watch.watchErrors.actionErrors) || null;
 
-        this.omitBreadcrumbPages = [
-          'watch',
-          this.watch.id
-        ];
+        this.omitBreadcrumbPages = ['watch', this.watch.id];
         this.breadcrumb = this.watch.displayName;
 
         // Reload watch history periodically
-        const refreshInterval = $interval(() => this.loadWatchHistory(), REFRESH_INTERVALS.WATCH_HISTORY);
+        const refreshInterval = $interval(
+          () => this.loadWatchHistory(),
+          REFRESH_INTERVALS.WATCH_HISTORY
+        );
         $scope.$on('$destroy', () => $interval.cancel(refreshInterval));
 
         // react to data and UI changes
-        $scope.$watchMulti([
-          'watchDetail.actionStatusTableSortField',
-          'watchDetail.actionStatusTableSortReverse',
-        ], this.applySortToActionStatusTable);
+        $scope.$watchMulti(
+          ['watchDetail.actionStatusTableSortField', 'watchDetail.actionStatusTableSortReverse'],
+          this.applySortToActionStatusTable
+        );
       }
 
       loadWatchHistory = () => {
-        return watchService.loadWatchHistory(this.watch.id, this.historyRange)
+        return watchService
+          .loadWatchHistory(this.watch.id, this.historyRange)
           .then(watchHistoryItems => {
             this.isHistoryLoading = false;
             this.watchHistoryItems = watchHistoryItems;
           })
           .catch(err => {
-            return licenseService.checkValidity()
-              .then(() => toastNotifications.addDanger(err));
+            return licenseService.checkValidity().then(() => toastNotifications.addDanger(err));
           });
-      }
+      };
 
       // update the watch history items when the time range changes
-      onHistoryRangeChange = (range) => {
+      onHistoryRangeChange = range => {
         this.historyRange = range;
         this.isHistoryLoading = true;
         return this.loadWatchHistory();

x-pack/plugins/watcher/public/sections/watch_list/components/watch_table/watch_table.html

@@ -29,7 +29,7 @@
           Name
         </sortable-column>
       </th>
-      <th scope="col" class="kuiTableHeaderCell">
+      <th scope="col" class="kuiTableHeaderCell" width="124">
         <sortable-column
           field="watchStatus.state"
           on-sort-change="watchTable.onSortChange"

x-pack/plugins/watcher/server/models/action/action.js

@@ -25,14 +25,14 @@ export class Action {
   }
 
   // From Elasticsearch
-  static fromUpstreamJson(json) {
+  static fromUpstreamJson(json, options) {
     if (!json.actionJson) {
       throw badRequest('json argument must contain an actionJson property');
     }
 
     const type = getActionType(json.actionJson);
     const ActionType = ActionTypes[type] || UnknownAction;
-    return ActionType.fromUpstreamJson(json);
+    return ActionType.fromUpstreamJson(json, options);
   }
 
   // From Kibana

x-pack/plugins/watcher/server/models/action/base_action.js

@@ -7,9 +7,10 @@
 import { badRequest } from 'boom';
 
 export class BaseAction {
-  constructor(props) {
+  constructor(props, errors) {
     this.id = props.id;
     this.type = props.type;
+    this.errors = errors;
   }
 
   get downstreamJson() {

x-pack/plugins/watcher/server/models/action/email_action.js

@@ -9,9 +9,9 @@ import { BaseAction } from './base_action';
 import { ACTION_TYPES } from '../../../common/constants';
 
 export class EmailAction extends BaseAction {
-  constructor(props) {
+  constructor(props, errors) {
     props.type = ACTION_TYPES.EMAIL;
-    super(props);
+    super(props, errors);
 
     this.to = props.to;
     this.subject = props.subject;
@@ -37,7 +37,7 @@ export class EmailAction extends BaseAction {
     Object.assign(props, {
       to: json.to,
       subject: json.subject,
-      body: json.body
+      body: json.body,
     });
 
     return new EmailAction(props);
@@ -67,15 +67,10 @@ export class EmailAction extends BaseAction {
   }
 
   // From Elasticsearch
-  static fromUpstreamJson(json) {
+  static fromUpstreamJson(json, options = { throwExceptions: {} }) {
     const props = super.getPropsFromUpstreamJson(json);
-
-    if (!json.actionJson.email) {
-      throw badRequest('json argument must contain an actionJson.email property');
-    }
-    if (!json.actionJson.email.to) {
-      throw badRequest('json argument must contain an actionJson.email.to property');
-    }
+    const doThrowException = options.throwExceptions.Action !== false;
+    const { errors } = this.validateJson(json, doThrowException);
 
     const optionalFields = {};
     if (json.actionJson.email.subject) {
@@ -91,7 +86,37 @@ export class EmailAction extends BaseAction {
       ...optionalFields,
     });
 
-    return new EmailAction(props);
+    return new EmailAction(props, errors);
+  }
+
+  static validateJson(json, doThrowException) {
+    const errors = [];
+
+    if (!json.actionJson.email) {
+      const message = 'json argument must contain an actionJson.email property';
+
+      if (doThrowException) {
+        throw badRequest(message);
+      }
+
+      errors.push({
+        code: 'ERR_PROP_MISSING',
+        message
+      });
+    }
+
+    if (!json.actionJson.email.to) {
+      const message = 'json argument must contain an actionJson.email.to property';
+      if (doThrowException) {
+        throw badRequest(message);
+      }
+      errors.push({
+        code: 'ERR_PROP_MISSING',
+        message
+      });
+    }
+
+    return { errors: errors.length ? errors : null };
   }
 
   /*

x-pack/plugins/watcher/server/models/action_status/__tests__/action_status.js

@@ -36,7 +36,7 @@ describe('action_status', () => {
               'timestamp': '2017-03-01T20:55:49.679Z',
               'successful': true
             }
-          }
+          },
         };
       });
 
@@ -53,7 +53,7 @@ describe('action_status', () => {
       });
 
       it('returns correct ActionStatus instance', () => {
-        const actionStatus = ActionStatus.fromUpstreamJson(upstreamJson);
+        const actionStatus = ActionStatus.fromUpstreamJson({ ...upstreamJson, errors: { foo: 'bar' } });
 
         expect(actionStatus.id).to.be(upstreamJson.id);
         expect(actionStatus.lastAcknowledged).to
@@ -68,6 +68,8 @@ describe('action_status', () => {
           .eql(moment(upstreamJson.actionStatusJson.last_throttle.timestamp));
         expect(actionStatus.lastSuccessfulExecution).to
           .eql(moment(upstreamJson.actionStatusJson.last_successful_execution.timestamp));
+        expect(actionStatus.errors).to
+          .eql({ foo: 'bar' });
       });
 
     });
@@ -106,6 +108,12 @@ describe('action_status', () => {
         expect(actionStatus.state).to.be(ACTION_STATES.ERROR);
       });
 
+      it('correctly calculates ACTION_STATES.CONFIG_ERROR', () => {
+        const actionStatus = ActionStatus.fromUpstreamJson({ ...upstreamJson, errors: { foo: 'bar' } });
+        expect(actionStatus.state).to.be(ACTION_STATES.CONFIG_ERROR);
+      });
+
+
       it(`correctly calculates ACTION_STATES.OK`, () => {
         upstreamJson.actionStatusJson.ack.state = 'awaits_successful_execution';
         const actionStatus = ActionStatus.fromUpstreamJson(upstreamJson);

x-pack/plugins/watcher/server/models/action_status/action_status.js

@@ -13,6 +13,7 @@ export class ActionStatus {
   constructor(props) {
     this.id = props.id;
     this.actionStatusJson = props.actionStatusJson;
+    this.errors = props.errors;
 
     this.lastAcknowledged = getMoment(get(this.actionStatusJson, 'ack.timestamp'));
     this.lastExecution = getMoment(get(this.actionStatusJson, 'last_execution.timestamp'));
@@ -30,6 +31,10 @@ export class ActionStatus {
       return ACTION_STATES.ERROR;
     }
 
+    if (this.errors) {
+      return ACTION_STATES.CONFIG_ERROR;
+    }
+
     if (ackState === 'awaits_successful_execution') {
       return ACTION_STATES.OK;
     }