From 189d35c4c0a5cd67923f7938f1bb3e19136d6b39 Mon Sep 17 00:00:00 2001
From: CJ Cenizal <cj@cenizal.com>
Date: Mon, 23 Jul 2018 16:01:34 -0700
Subject: [PATCH] Add rollupSearchStrategy and SearchError (#20505)

* Add /api/rollup/search endpoint and rollupSearchStrategy.
* Add SearchError for surfacing courier search errors.
- Use toastNotifications to surface errors in visualization editor.
- Add call-out react directive and use it to surface rollup errors in the visualization editor sidebar.
- Temporarily assign timezone and interval values from rollup job to the search to avoid errors.
---
 src/ui/public/courier/fetch/fetch_now.js      |  11 +-
 src/ui/public/courier/index.js                |   2 +
 .../default_search_strategy.js                |  23 +++-
 .../public/courier/search_strategy/index.js   |   2 +
 .../courier/search_strategy/search_error.js   |  34 ++++++
 .../search_strategy_registry.js               |  13 +-
 .../public/vis/editors/default/sidebar.html   |   9 ++
 src/ui/public/vis/editors/default/sidebar.js  |  20 ++-
 .../public/vis/editors/default/sidebar.less   |   3 +
 .../visualize/loader/visualize_data_loader.js |  40 ++++--
 x-pack/plugins/rollup/index.js                |   8 +-
 x-pack/plugins/rollup/public/search/index.js  |   7 ++
 .../plugins/rollup/public/search/register.js  |  10 ++
 .../public/search/rollup_search_strategy.js   | 114 ++++++++++++++++++
 .../server/client/elasticsearch_rollup.js     |  15 +++
 .../plugins/rollup/server/routes/api/index.js |   1 +
 .../rollup/server/routes/api/search.js        |  36 ++++++
 17 files changed, 319 insertions(+), 29 deletions(-)
 create mode 100644 src/ui/public/courier/search_strategy/search_error.js
 create mode 100644 src/ui/public/vis/editors/default/sidebar.less
 create mode 100644 x-pack/plugins/rollup/public/search/index.js
 create mode 100644 x-pack/plugins/rollup/public/search/register.js
 create mode 100644 x-pack/plugins/rollup/public/search/rollup_search_strategy.js
 create mode 100644 x-pack/plugins/rollup/server/routes/api/search.js

diff --git a/src/ui/public/courier/fetch/fetch_now.js b/src/ui/public/courier/fetch/fetch_now.js
index 70aa20a26619c..7bb3890fbf95e 100644
--- a/src/ui/public/courier/fetch/fetch_now.js
+++ b/src/ui/public/courier/fetch/fetch_now.js
@@ -52,7 +52,10 @@ export function FetchNowProvider(Private, Promise) {
 
       return searchRequest.retry();
     }))
-      .catch(error => fatalError(error, 'Courier fetch'));
+      .catch(error => {
+        // If any errors occur after the search requests have resolved, then we kill Kibana.
+        fatalError(error, 'Courier fetch');
+      });
   }
 
   function fetchSearchResults(searchRequests) {
@@ -70,7 +73,11 @@ export function FetchNowProvider(Private, Promise) {
     return startRequests(searchRequests)
       .then(function () {
         replaceAbortedRequests();
-        return callClient(searchRequests);
+        return callClient(searchRequests)
+          .catch(() => {
+            // Silently swallow errors that result from search requests so the consumer can surface
+            // them as notifications instead of courier forcing fatal errors.
+          });
       })
       .then(function (responses) {
         replaceAbortedRequests();
diff --git a/src/ui/public/courier/index.js b/src/ui/public/courier/index.js
index d466b2e080e3f..583172f1731cd 100644
--- a/src/ui/public/courier/index.js
+++ b/src/ui/public/courier/index.js
@@ -30,6 +30,8 @@ export {
 } from './search_source';
 
 export {
+  addSearchStrategy,
   hasSearchStategyForIndexPattern,
   isDefaultTypeIndexPattern,
+  SearchError,
 } from './search_strategy';
diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js
index 05fc45c5c67d1..f5bd529b35c0b 100644
--- a/src/ui/public/courier/search_strategy/default_search_strategy.js
+++ b/src/ui/public/courier/search_strategy/default_search_strategy.js
@@ -19,6 +19,7 @@
 
 import { addSearchStrategy } from './search_strategy_registry';
 import { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';
+import { SearchError } from './search_error';
 
 function getAllFetchParams(searchRequests, Promise) {
   return Promise.map(searchRequests, (searchRequest) => {
@@ -71,14 +72,30 @@ export const defaultSearchStrategy = {
     const searching = es.msearch({ body: serializedFetchParams });
 
     return {
-      // Unwrap the responses object returned by the es client.
-      searching: searching.then(({ responses }) => responses),
+      // Munge data into shape expected by consumer.
+      searching: new Promise((resolve, reject) => {
+        // Unwrap the responses object returned by the ES client.
+        searching.then(({ responses }) => {
+          resolve(responses);
+        }).catch(error => {
+          // Format ES client error as a SearchError.
+          const { statusCode, displayName, message, path } = error;
+
+          const searchError = new SearchError({
+            status: statusCode,
+            title: displayName,
+            message,
+            path,
+          });
+
+          reject(searchError);
+        });
+      }),
       abort: searching.abort,
       failedSearchRequests,
     };
   },
 
-  // Accept multiple criteria for determining viability to be as flexible as possible.
   isViable: (indexPattern) => {
     if (!indexPattern) {
       return false;
diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js
index 41991b63f950d..102610538df98 100644
--- a/src/ui/public/courier/search_strategy/index.js
+++ b/src/ui/public/courier/search_strategy/index.js
@@ -24,3 +24,5 @@ export {
 } from './search_strategy_registry';
 
 export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';
+
+export { SearchError } from './search_error';
diff --git a/src/ui/public/courier/search_strategy/search_error.js b/src/ui/public/courier/search_strategy/search_error.js
new file mode 100644
index 0000000000000..fecafb9b6fc13
--- /dev/null
+++ b/src/ui/public/courier/search_strategy/search_error.js
@@ -0,0 +1,34 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export class SearchError extends Error {
+  constructor({ status, title, message, path }) {
+    super(message);
+    this.name = 'SearchError';
+    this.status = status;
+    this.title = title;
+    this.message = message;
+    this.path = path;
+    Error.captureStackTrace(this, SearchError);
+
+    // Babel doesn't support traditional `extends` syntax for built-in classes.
+    // https://babeljs.io/docs/en/caveats/#classes
+    Object.setPrototypeOf(this, SearchError.prototype);
+  }
+}
diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js
index a0341932565da..79e959b2a80ad 100644
--- a/src/ui/public/courier/search_strategy/search_strategy_registry.js
+++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js
@@ -19,7 +19,7 @@
 
 const searchStrategies = [];
 
-const addSearchStrategy = searchStrategy => {
+export const addSearchStrategy = searchStrategy => {
   if (searchStrategies.includes(searchStrategy)) {
     return;
   }
@@ -47,12 +47,11 @@ const getSearchStrategy = indexPattern => {
  * We use an array of objects to preserve the order of the search requests, which we use to
  * deterministically associate each response with the originating request.
  */
-const assignSearchRequestsToSearchStrategies = searchRequests => {
+export const assignSearchRequestsToSearchStrategies = searchRequests => {
   const searchStrategiesWithRequests = [];
   const searchStrategyById = {};
 
   searchRequests.forEach(searchRequest => {
-
     const indexPattern = searchRequest.source.getField('index');
     const matchingSearchStrategy = getSearchStrategy(indexPattern);
 
@@ -76,12 +75,6 @@ const assignSearchRequestsToSearchStrategies = searchRequests => {
   return searchStrategiesWithRequests;
 };
 
-const hasSearchStategyForIndexPattern = indexPattern => {
+export const hasSearchStategyForIndexPattern = indexPattern => {
   return Boolean(getSearchStrategy(indexPattern));
 };
-
-export {
-  assignSearchRequestsToSearchStrategies,
-  addSearchStrategy,
-  hasSearchStategyForIndexPattern,
-};
diff --git a/src/ui/public/vis/editors/default/sidebar.html b/src/ui/public/vis/editors/default/sidebar.html
index f0cff4acd142f..43ddf5e1aff21 100644
--- a/src/ui/public/vis/editors/default/sidebar.html
+++ b/src/ui/public/vis/editors/default/sidebar.html
@@ -19,6 +19,15 @@
     </div>
 
     <nav class="navbar navbar-default subnav">
+      <div class="visEditorSidebarError" ng-if="sidebar.persistentError">
+        <call-out
+          color="'danger'"
+          iconType="'alert'"
+          title="sidebar.persistentError"
+          size="'s'"
+        />
+      </div>
+
       <div class="container-fluid">
 
         <!-- tabs -->
diff --git a/src/ui/public/vis/editors/default/sidebar.js b/src/ui/public/vis/editors/default/sidebar.js
index 6503a11b998f1..96bd4f755ea2d 100644
--- a/src/ui/public/vis/editors/default/sidebar.js
+++ b/src/ui/public/vis/editors/default/sidebar.js
@@ -23,17 +23,18 @@ import './vis_options';
 import { uiModules } from '../../../modules';
 import sidebarTemplate from './sidebar.html';
 
+import './sidebar.less';
+
 uiModules
   .get('app/visualize')
   .directive('visEditorSidebar', function () {
-
-
     return {
       restrict: 'E',
       template: sidebarTemplate,
       scope: true,
       controllerAs: 'sidebar',
       controller: function ($scope) {
+        this.persistentError = undefined;
 
         $scope.$watch('vis.type', (visType) => {
           if (visType) {
@@ -49,6 +50,21 @@ uiModules
             this.section = this.section || (this.showData ? 'data' : _.get(visType, 'editorConfig.optionTabs[0].name'));
           }
         });
+
+        // TODO: This doesn't update when requestError is set.
+        $scope.$watch('vis.requestError', (requestError) => {
+          if (requestError && requestError.messsage) {
+            const { message } = requestError;
+            const isRollupError = message.includes('rollup');
+
+            if (isRollupError) {
+              this.persistentError = message;
+              return;
+            }
+          }
+
+          this.persistentError = undefined;
+        });
       }
     };
   });
diff --git a/src/ui/public/vis/editors/default/sidebar.less b/src/ui/public/vis/editors/default/sidebar.less
new file mode 100644
index 0000000000000..9e346211e073f
--- /dev/null
+++ b/src/ui/public/vis/editors/default/sidebar.less
@@ -0,0 +1,3 @@
+.visEditorSidebarError {
+  padding: 8px;
+}
diff --git a/src/ui/public/visualize/loader/visualize_data_loader.js b/src/ui/public/visualize/loader/visualize_data_loader.js
index 3cd5d5472aa70..4ddf1ba235093 100644
--- a/src/ui/public/visualize/loader/visualize_data_loader.js
+++ b/src/ui/public/visualize/loader/visualize_data_loader.js
@@ -16,6 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
+import React, { Fragment } from 'react';
 import { isEqual } from 'lodash';
 import { VisRequestHandlersRegistryProvider } from '../../registry/vis_request_handlers';
 import { VisResponseHandlersRegistryProvider } from '../../registry/vis_response_handlers';
@@ -25,6 +27,7 @@ import {
 } from '../../elasticsearch_errors';
 
 import { toastNotifications } from 'ui/notify';
+import { SearchError } from 'ui/courier';
 
 function getHandler(from, name) {
   if (typeof name === 'function') return name;
@@ -62,24 +65,41 @@ export class VisualizeDataLoader {
 
       this._previousVisState = this._vis.getState();
       this._previousRequestHandlerResponse = requestHandlerResponse;
+      this._vis.requestError = undefined;
 
       if (!canSkipResponseHandler) {
         this._visData = await Promise.resolve(this._responseHandler(this._vis, requestHandlerResponse));
       }
       return this._visData;
     }
-    catch (e) {
+    catch (error) {
       props.searchSource.cancelQueued();
-      this._vis.requestError = e;
-      if (isTermSizeZeroError(e)) {
-        return toastNotifications.addDanger(
-          `Your visualization ('${props.vis.title}') has an error: it has a term ` +
-          `aggregation with a size of 0. Please set it to a number greater than 0 to resolve ` +
-          `the error.`
-        );
+      this._vis.requestError = error;
+
+      if (isTermSizeZeroError(error)) {
+        return toastNotifications.addDanger({
+          title: `Visualization ('${this._vis.title}') has term aggregation of size 0`,
+          text: `Set it to a size greater than 0`,
+        });
+      }
+
+      if (error instanceof SearchError) {
+        const { message, path, status } = error;
+        return toastNotifications.addDanger({
+          title: `Visualization '${this._vis.title}' search failed`,
+          text: (
+            <Fragment>
+              <p>{message}</p>
+              <p><code>{status} {path}</code></p>
+            </Fragment>
+          ),
+        });
       }
-      toastNotifications.addDanger(e);
+
+      toastNotifications.addDanger({
+        title: `Visualization '${this._vis.title}' has an error`,
+        text: error,
+      });
     }
   }
-
 }
diff --git a/x-pack/plugins/rollup/index.js b/x-pack/plugins/rollup/index.js
index 42879f503b445..2f91455c91288 100644
--- a/x-pack/plugins/rollup/index.js
+++ b/x-pack/plugins/rollup/index.js
@@ -7,9 +7,9 @@
 import { resolve } from 'path';
 import { PLUGIN } from './common/constants';
 import { registerLicenseChecker } from './server/lib/register_license_checker';
-import { registerIndicesRoute, registerFieldsForWildcardRoute } from './server/routes/api';
+import { registerIndicesRoute, registerFieldsForWildcardRoute, registerSearchRoute } from './server/routes/api';
 
-export function rollup(kibana)  {
+export function rollup(kibana) {
   return new kibana.Plugin({
     id: PLUGIN.ID,
     publicDir: resolve(__dirname, 'public'),
@@ -22,11 +22,15 @@ export function rollup(kibana)  {
       visualize: [
         'plugins/rollup/visualize',
       ],
+      search: [
+        'plugins/rollup/search',
+      ],
     },
     init: function (server) {
       registerLicenseChecker(server);
       registerIndicesRoute(server);
       registerFieldsForWildcardRoute(server);
+      registerSearchRoute(server);
     }
   });
 }
diff --git a/x-pack/plugins/rollup/public/search/index.js b/x-pack/plugins/rollup/public/search/index.js
new file mode 100644
index 0000000000000..4d2947eb36f46
--- /dev/null
+++ b/x-pack/plugins/rollup/public/search/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.
+*/
+
+import './register';
diff --git a/x-pack/plugins/rollup/public/search/register.js b/x-pack/plugins/rollup/public/search/register.js
new file mode 100644
index 0000000000000..9f0da80f14245
--- /dev/null
+++ b/x-pack/plugins/rollup/public/search/register.js
@@ -0,0 +1,10 @@
+/*
+* 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 { addSearchStrategy } from 'ui/courier';
+import { rollupSearchStrategy } from './rollup_search_strategy';
+
+addSearchStrategy(rollupSearchStrategy);
diff --git a/x-pack/plugins/rollup/public/search/rollup_search_strategy.js b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js
new file mode 100644
index 0000000000000..37e61ad00fa15
--- /dev/null
+++ b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js
@@ -0,0 +1,114 @@
+/*
+* 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 { kfetchAbortable } from 'ui/kfetch';
+import { SearchError } from 'ui/courier';
+
+export const rollupSearchStrategy = {
+  id: 'rollup',
+
+  search: async ({ searchRequests, Promise }) => {
+    // TODO: Batch together requests to hit a bulk rollup search endpoint.
+    const searchRequest = searchRequests[0];
+    const {
+      index: { title: indexPattern },
+      body: {
+        size,
+        aggs,
+      },
+    } = await searchRequest.getFetchParams();
+
+    function findDateHistogram(aggs) {
+      if (Array.isArray(aggs)) {
+        for (let i = 0; i < aggs.length; i++) {
+          const dateHistogram = findDateHistogram(aggs[i]);
+
+          if (dateHistogram) {
+            return dateHistogram;
+          }
+        }
+      } else if (typeof aggs === 'object') {
+        const aggNames = Object.keys(aggs);
+        const aggsList = aggNames.map(aggName => aggs[aggName]);
+
+        if (aggsList.includes('date_histogram')) {
+          return aggs;
+        }
+
+        return findDateHistogram(aggsList);
+      }
+    }
+
+    // TODO: Temporarily automatically assign same timezone and interval as what's defined by
+    // the rollup job. This should be done by the visualization itself.
+    const searchableAggs = JSON.parse(searchRequest.source.getField('index').originalBody.typeMeta);
+    const { time_zone: timeZone, interval } = findDateHistogram(searchableAggs);
+
+    Object.keys(aggs).forEach(aggName => {
+      const subAggs = aggs[aggName];
+
+      Object.keys(subAggs).forEach(subAggName => {
+        if (subAggName === 'date_histogram') {
+          const subAgg = subAggs[subAggName];
+          subAgg.time_zone = timeZone;
+          subAgg.interval = interval;
+        }
+      });
+    });
+
+    const index = indexPattern;
+    const query = {
+      'size': size,
+      'aggregations': aggs,
+    };
+
+    const {
+      fetching,
+      abort,
+    } = kfetchAbortable({
+      pathname: '../api/rollup/search',
+      method: 'POST',
+      body: JSON.stringify({ index, query }),
+    });
+
+    // TODO: Implement this. Search requests which can't be sent.
+    const failedSearchRequests = [];
+
+    return {
+      // Munge data into shape expected by consumer.
+      searching: new Promise((resolve, reject) => {
+        fetching.then(result => {
+          resolve([ result ]);
+        }).catch(error => {
+          const {
+            body: { statusText, error: title, message },
+            res: { url },
+          } = error;
+
+          // Format kfetch error as a SearchError.
+          const searchError = new SearchError({
+            status: statusText,
+            title,
+            message,
+            path: url,
+          });
+
+          reject(searchError);
+        });
+      }),
+      abort,
+      failedSearchRequests,
+    };
+  },
+
+  isViable: (indexPattern) => {
+    if (!indexPattern) {
+      return false;
+    }
+
+    return indexPattern.type === 'rollup';
+  },
+};
diff --git a/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js b/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js
index 22148a0b112b1..10399861cad64 100644
--- a/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js
+++ b/x-pack/plugins/rollup/server/client/elasticsearch_rollup.js
@@ -26,5 +26,20 @@ export const elasticsearchJsPlugin = (Client, config, components) => {
     ],
     method: 'GET'
   });
+
+  rollup.search = ca({
+    urls: [
+      {
+        fmt: '/<%=index%>/_rollup_search',
+        req: {
+          index: {
+            type: 'string'
+          }
+        }
+      }
+    ],
+    needBody: true,
+    method: 'POST'
+  });
 };
 
diff --git a/x-pack/plugins/rollup/server/routes/api/index.js b/x-pack/plugins/rollup/server/routes/api/index.js
index 5e9e9644d422e..0bf361d6c6399 100644
--- a/x-pack/plugins/rollup/server/routes/api/index.js
+++ b/x-pack/plugins/rollup/server/routes/api/index.js
@@ -6,3 +6,4 @@
 
 export { registerIndicesRoute } from './indices';
 export { registerFieldsForWildcardRoute } from './index_patterns';
+export { registerSearchRoute } from './search';
diff --git a/x-pack/plugins/rollup/server/routes/api/search.js b/x-pack/plugins/rollup/server/routes/api/search.js
new file mode 100644
index 0000000000000..a77040daad78a
--- /dev/null
+++ b/x-pack/plugins/rollup/server/routes/api/search.js
@@ -0,0 +1,36 @@
+/*
+* 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 { callWithRequestFactory } from '../../lib/call_with_request_factory';
+import { isEsErrorFactory } from '../../lib/is_es_error_factory';
+import { wrapEsError, wrapUnknownError } from '../../lib/error_wrappers';
+
+export function registerSearchRoute(server) {
+  const isEsError = isEsErrorFactory(server);
+
+  server.route({
+    path: '/api/rollup/search',
+    method: 'POST',
+    handler: async (request, reply) => {
+      const { index, query } = request.payload;
+      const callWithRequest = callWithRequestFactory(server, request);
+
+      try {
+        const results = await callWithRequest('rollup.search', {
+          index,
+          body: query,
+        });
+
+        reply(results);
+      } catch(err) {
+        if (isEsError(err)) {
+          return reply(wrapEsError(err));
+        }
+
+        reply(wrapUnknownError(err));
+      }
+    },
+  });
+}