Let search errors bubble up instead of killing all search requests du…

…e to `Promise.all` rejecting on first failure (#23863)

* Show unsupported query search errors within visualization itself (aka ones not supported by rollup search)

src/ui/public/courier/fetch/call_client.js

@@ -161,7 +161,7 @@ export function CallClientProvider(Private, Promise, es, config) {
           return;
         }
 
-        const segregatedResponses = await Promise.all(abortableSearches.map(({ searching }) => searching));
+        const segregatedResponses = await Promise.all(abortableSearches.map(({ searching }) => searching.catch((e) => [{ error: e }])));
 
         // Assigning searchRequests to strategies means that the responses come back in a different
         // order than the original searchRequests. So we'll put them back in order so that we can

src/ui/public/courier/fetch/call_response_handlers.js

@@ -20,6 +20,7 @@
 import { toastNotifications } from '../../notify';
 import { RequestFailure } from '../../errors';
 import { RequestStatus } from './req_status';
+import { SearchError } from '../search_strategy/search_error';
 
 export function CallResponseHandlersProvider(Private, Promise) {
   const ABORTED = RequestStatus.ABORTED;
@@ -58,7 +59,7 @@ export function CallResponseHandlersProvider(Private, Promise) {
         if (searchRequest.filterError(response)) {
           return progress();
         } else {
-          return searchRequest.handleFailure(new RequestFailure(null, response));
+          return searchRequest.handleFailure(response.error instanceof SearchError ? response.error : new RequestFailure(null, response));
         }
       }
 

src/ui/public/courier/fetch/request/search_request/search_request.js

@@ -123,7 +123,8 @@ export function SearchRequestProvider(Promise) {
 
     handleFailure(error) {
       this.success = false;
-      this.resp = error && error.resp;
+      this.resp = error;
+      this.resp = (error && error.resp) || error;
       return this.errorHandler(this, error);
     }
 

src/ui/public/courier/index.d.ts

@@ -18,3 +18,4 @@
  */
 
 export * from './search_source';
+export * from './search_strategy';

src/ui/public/courier/index.js

@@ -34,4 +34,5 @@ export {
   hasSearchStategyForIndexPattern,
   isDefaultTypeIndexPattern,
   SearchError,
+  getSearchErrorType,
 } from './search_strategy';

src/ui/public/courier/search_strategy/index.d.ts

@@ -0,0 +1,20 @@
+/*
+ * 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 { SearchError, getSearchErrorType } from './search_error';

src/ui/public/courier/search_strategy/index.js

@@ -25,4 +25,4 @@ export {
 
 export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern';
 
-export { SearchError } from './search_error';
+export { SearchError, getSearchErrorType } from './search_error';

src/ui/public/courier/search_strategy/search_error.d.ts

@@ -0,0 +1,21 @@
+/*
+ * 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 type SearchError = any;
+export type getSearchErrorType = any;

src/ui/public/courier/search_strategy/search_error.js

@@ -18,13 +18,14 @@
  */
 
 export class SearchError extends Error {
-  constructor({ status, title, message, path }) {
+  constructor({ status, title, message, path, type }) {
     super(message);
     this.name = 'SearchError';
     this.status = status;
     this.title = title;
     this.message = message;
     this.path = path;
+    this.type = type;
 
     // captureStackTrace is only available in the V8 engine, so any browser using
     // a different JS engine won't have access to this method.
@@ -37,3 +38,10 @@ export class SearchError extends Error {
     Object.setPrototypeOf(this, SearchError.prototype);
   }
 }
+
+export function getSearchErrorType({ message }) {
+  const msg = message.toLowerCase();
+  if(msg.indexOf('unsupported query') > -1) {
+    return 'UNSUPPORTED_QUERY';
+  }
+}

src/ui/public/visualize/components/__snapshots__/visualization_requesterror.test.js.snap

@@ -0,0 +1,17 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`VisualizationRequestError should render according to snapshot 1`] = `
+<div
+  class="visualize-error visualize-chart"
+>
+  <div
+    class="euiText euiText--extraSmall visualize-request-error"
+  >
+    <div
+      class="euiTextColor euiTextColor--danger"
+    >
+      Request error
+    </div>
+  </div>
+</div>
+`;

src/ui/public/visualize/components/visualization.less

@@ -65,7 +65,15 @@
   display: flex;
   align-items: center;
   justify-content: center;
-  .top { align-self: flext-start; }
+  .top { align-self: flex-start; }
   .item { }
-  .bottom { align-self: flext-end; }
+  .bottom { align-self: flex-end; }
+}
+
+/**
+ * 1. Prevent large request errors from overflowing the container
+ */
+.visualize-request-error {
+  max-width: 100%;
+  max-height: 100%;
 }

src/ui/public/visualize/components/visualization.test.js

@@ -79,6 +79,13 @@ describe('<Visualization/>', () => {
     expect(wrapper.text()).toBe('No results found');
   });
 
+  it('should display error message when there is a request error that should be shown and no data', () => {
+    const errorVis = { ...vis, requestError: { message: 'Request error' }, showRequestError: true };
+    const data = null;
+    const wrapper = render(<Visualization vis={errorVis} visData={data} listenOnChange={true} uiState={uiState} />);
+    expect(wrapper.text()).toBe('Request error');
+  });
+
   it('should render chart when data is present', () => {
     const wrapper = render(<Visualization vis={vis} visData={visData} uiState={uiState} listenOnChange={true} />);
     expect(wrapper.text()).not.toBe('No results found');

src/ui/public/visualize/components/visualization.tsx

@@ -25,6 +25,7 @@ import { memoizeLast } from '../../utils/memoize';
 import { Vis } from '../../vis';
 import { VisualizationChart } from './visualization_chart';
 import { VisualizationNoResults } from './visualization_noresults';
+import { VisualizationRequestError } from './visualization_requesterror';
 
 import './visualization.less';
 
@@ -37,6 +38,12 @@ function shouldShowNoResultsMessage(vis: Vis, visData: any): boolean {
   return Boolean(requiresSearch && isZeroHits && shouldShowMessage);
 }
 
+function shouldShowRequestErrorMessage(vis: Vis, visData: any): boolean {
+  const requestError = get(vis, 'requestError');
+  const showRequestError = get(vis, 'showRequestError');
+  return Boolean(!visData && requestError && showRequestError);
+}
+
 interface VisualizationProps {
   listenOnChange: boolean;
   onInit?: () => void;
@@ -63,10 +70,13 @@ export class Visualization extends React.Component<VisualizationProps> {
     const { vis, visData, onInit, uiState } = this.props;
 
     const noResults = this.showNoResultsMessage(vis, visData);
+    const requestError = shouldShowRequestErrorMessage(vis, visData);
 
     return (
       <div className="visualization">
-        {noResults ? (
+        {requestError ? (
+          <VisualizationRequestError onInit={onInit} error={vis.requestError} />
+        ) : noResults ? (
           <VisualizationNoResults onInit={onInit} />
         ) : (
           <VisualizationChart vis={vis} visData={visData} onInit={onInit} uiState={uiState} />

src/ui/public/visualize/components/visualization_requesterror.test.js

@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+import React from 'react';
+import { render } from 'enzyme';
+import { VisualizationRequestError } from './visualization_requesterror';
+
+describe('VisualizationRequestError', () => {
+  it('should render according to snapshot', () => {
+    const wrapper = render(<VisualizationRequestError error="Request error" />);
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  it('should set html when error is an object', () => {
+    const wrapper = render(<VisualizationRequestError error={{ message: 'Request error' }} />);
+    expect(wrapper.text()).toBe('Request error');
+  });
+
+  it('should set html when error is a string', () => {
+    const wrapper = render(<VisualizationRequestError error="Request error" />);
+    expect(wrapper.text()).toBe('Request error');
+  });
+});

src/ui/public/visualize/components/visualization_requesterror.tsx

@@ -0,0 +1,62 @@
+/*
+ * 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.
+ */
+
+import { EuiText } from '@elastic/eui';
+import React from 'react';
+import { SearchError } from 'ui/courier';
+import { dispatchRenderComplete } from '../../render_complete';
+
+interface VisualizationRequestErrorProps {
+  onInit?: () => void;
+  error: SearchError | string;
+}
+
+export class VisualizationRequestError extends React.Component<VisualizationRequestErrorProps> {
+  private containerDiv = React.createRef<HTMLDivElement>();
+
+  public render() {
+    const { error } = this.props;
+    const errorMessage = (error && error.message) || error;
+
+    return (
+      <div className="visualize-error visualize-chart" ref={this.containerDiv}>
+        <EuiText className="visualize-request-error" color="danger" size="xs">
+          {errorMessage}
+        </EuiText>
+      </div>
+    );
+  }
+
+  public componentDidMount() {
+    this.afterRender();
+  }
+
+  public componentDidUpdate() {
+    this.afterRender();
+  }
+
+  private afterRender() {
+    if (this.props.onInit) {
+      this.props.onInit();
+    }
+    if (this.containerDiv.current) {
+      dispatchRenderComplete(this.containerDiv.current);
+    }
+  }
+}

src/ui/public/visualize/loader/visualize_data_loader.ts

@@ -67,8 +67,10 @@ export class VisualizeDataLoader {
     this.responseHandler = getHandler(responseHandlers, responseHandler);
   }
 
-  public async fetch(params: RequestHandlerParams): Promise<any> {
+  public fetch = async (params: RequestHandlerParams): Promise<any> => {
     this.vis.filters = { timeRange: params.timeRange };
+    this.vis.requestError = undefined;
+    this.vis.showRequestError = false;
 
     try {
       // searchSource is only there for courier request handler
@@ -95,6 +97,7 @@ export class VisualizeDataLoader {
     } catch (e) {
       params.searchSource.cancelQueued();
       this.vis.requestError = e;
+      this.vis.showRequestError = e.type && e.type === 'UNSUPPORTED_QUERY';
       if (isTermSizeZeroError(e)) {
         return toastNotifications.addDanger(
           `Your visualization ('${this.vis.title}') has an error: it has a term ` +
@@ -107,5 +110,5 @@ export class VisualizeDataLoader {
         text: e.message,
       });
     }
-  }
+  };
 }

x-pack/plugins/rollup/public/search/rollup_search_strategy.js

@@ -5,7 +5,7 @@
 */
 
 import { kfetchAbortable } from 'ui/kfetch';
-import { SearchError } from 'ui/courier';
+import { SearchError, getSearchErrorType } from 'ui/courier';
 
 function getAllFetchParams(searchRequests, Promise) {
   return Promise.map(searchRequests, (searchRequest) => {
@@ -118,8 +118,9 @@ export const rollupSearchStrategy = {
           const searchError = new SearchError({
             status: statusText,
             title,
-            message,
+            message: `Rollup search error: ${message}`,
             path: url,
+            type: getSearchErrorType({ message }),
           });
 
           reject(searchError);