Expressions service fixes: better error and loading states handling (#51183) (#51800)

1. This pr fixes regression in v7.6 - #51153.

Visualisation which are using ExpressionLoader directly are swallowing render errors in 7.6, because generic error handling is happening only on expression_renderer.tsx level (react component wrapper around renderer).

Now react agnostic render code render.ts, loader.ts:

has it's own default error handler which is toast from notification service.
It also allows to pass custom error handler instead of default one
React expression renderer expression_renderer.tsx:

if consumer wants to render error as react component inline, then the component uses custom error handler from render.ts to wire up react component.

2. This pr fixes issue of loader.ts where initial loading$ was not emitted

Had to change loadingSubject from Subject to BehaviorSubject to remember the last value

3. This pr fixes dependencies in effects of expression_renderer.tsx
# Conflicts:
#	src/plugins/expressions/public/loader.test.ts

src/plugins/expressions/public/execute.test.ts

@@ -29,6 +29,13 @@ jest.mock('./services', () => ({
       },
     };
   },
+  getNotifications: jest.fn(() => {
+    return {
+      toasts: {
+        addError: jest.fn(() => {}),
+      },
+    };
+  }),
 }));
 
 describe('execute helper function', () => {

src/plugins/expressions/public/expression_renderer.test.tsx

@@ -18,12 +18,14 @@
  */
 
 import React from 'react';
+import { act } from 'react-dom/test-utils';
 import { Subject } from 'rxjs';
 import { share } from 'rxjs/operators';
 import { ExpressionRendererImplementation } from './expression_renderer';
 import { ExpressionLoader } from './loader';
 import { mount } from 'enzyme';
 import { EuiProgress } from '@elastic/eui';
+import { RenderErrorHandlerFnType } from './types';
 
 jest.mock('./loader', () => {
   return {
@@ -54,30 +56,38 @@ describe('ExpressionRenderer', () => {
 
     const instance = mount(<ExpressionRendererImplementation expression="" />);
 
-    loadingSubject.next();
+    act(() => {
+      loadingSubject.next();
+    });
+
     instance.update();
 
     expect(instance.find(EuiProgress)).toHaveLength(1);
 
-    renderSubject.next(1);
+    act(() => {
+      renderSubject.next(1);
+    });
 
     instance.update();
 
     expect(instance.find(EuiProgress)).toHaveLength(0);
 
     instance.setProps({ expression: 'something new' });
-    loadingSubject.next();
+    act(() => {
+      loadingSubject.next();
+    });
     instance.update();
 
     expect(instance.find(EuiProgress)).toHaveLength(1);
-
-    renderSubject.next(1);
+    act(() => {
+      renderSubject.next(1);
+    });
     instance.update();
 
     expect(instance.find(EuiProgress)).toHaveLength(0);
   });
 
-  it('should display an error message when the expression fails', () => {
+  it('should display a custom error message if the user provides one and then remove it after successful render', () => {
     const dataSubject = new Subject();
     const data$ = dataSubject.asObservable().pipe(share());
     const renderSubject = new Subject();
@@ -85,7 +95,10 @@ describe('ExpressionRenderer', () => {
     const loadingSubject = new Subject();
     const loading$ = loadingSubject.asObservable().pipe(share());
 
-    (ExpressionLoader as jest.Mock).mockImplementation(() => {
+    let onRenderError: RenderErrorHandlerFnType;
+    (ExpressionLoader as jest.Mock).mockImplementation((...args) => {
+      const params = args[2];
+      onRenderError = params.onRenderError;
       return {
         render$,
         data$,
@@ -94,48 +107,32 @@ describe('ExpressionRenderer', () => {
       };
     });
 
-    const instance = mount(<ExpressionRendererImplementation expression="" />);
-
-    dataSubject.next('good data');
-    renderSubject.next({
-      type: 'error',
-      error: { message: 'render error' },
-    });
-    instance.update();
-
-    expect(instance.find(EuiProgress)).toHaveLength(0);
-    expect(instance.find('[data-test-subj="expression-renderer-error"]')).toHaveLength(1);
-  });
-
-  it('should display a custom error message if the user provides one', () => {
-    const dataSubject = new Subject();
-    const data$ = dataSubject.asObservable().pipe(share());
-    const renderSubject = new Subject();
-    const render$ = renderSubject.asObservable().pipe(share());
-    const loadingSubject = new Subject();
-    const loading$ = loadingSubject.asObservable().pipe(share());
-
-    (ExpressionLoader as jest.Mock).mockImplementation(() => {
-      return {
-        render$,
-        data$,
-        loading$,
-        update: jest.fn(),
-      };
-    });
-
-    const renderErrorFn = jest.fn().mockReturnValue(null);
-
     const instance = mount(
-      <ExpressionRendererImplementation expression="" renderError={renderErrorFn} />
+      <ExpressionRendererImplementation
+        expression=""
+        renderError={message => <div data-test-subj={'custom-error'}>{message}</div>}
+      />
     );
 
-    renderSubject.next({
-      type: 'error',
-      error: { message: 'render error' },
+    act(() => {
+      onRenderError!(instance.getDOMNode(), new Error('render error'), {
+        done: () => {
+          renderSubject.next(1);
+        },
+      } as any);
+    });
+
+    instance.update();
+    expect(instance.find(EuiProgress)).toHaveLength(0);
+    expect(instance.find('[data-test-subj="custom-error"]')).toHaveLength(1);
+    expect(instance.find('[data-test-subj="custom-error"]').contains('render error')).toBeTruthy();
+
+    act(() => {
+      loadingSubject.next();
+      renderSubject.next(2);
     });
     instance.update();
-
-    expect(renderErrorFn).toHaveBeenCalledWith('render error');
+    expect(instance.find(EuiProgress)).toHaveLength(0);
+    expect(instance.find('[data-test-subj="custom-error"]')).toHaveLength(0);
   });
 });

src/plugins/expressions/public/expression_renderer.tsx

@@ -17,12 +17,15 @@
  * under the License.
  */
 
-import { useRef, useEffect, useState } from 'react';
+import { useRef, useEffect, useState, useLayoutEffect } from 'react';
 import React from 'react';
 import classNames from 'classnames';
+import { Subscription } from 'rxjs';
+import { filter } from 'rxjs/operators';
 import { EuiLoadingChart, EuiProgress } from '@elastic/eui';
 import theme from '@elastic/eui/dist/eui_theme_light.json';
-import { IExpressionLoaderParams } from './types';
+import { useShallowCompareEffect } from '../../kibana_react/public';
+import { IExpressionLoaderParams, IInterpreterRenderHandlers, RenderError } from './types';
 import { ExpressionAST } from '../common/types';
 import { ExpressionLoader } from './loader';
 
@@ -39,7 +42,7 @@ export interface ExpressionRendererProps extends IExpressionLoaderParams {
 interface State {
   isEmpty: boolean;
   isLoading: boolean;
-  error: null | { message: string };
+  error: null | RenderError;
 }
 
 export type ExpressionRenderer = React.FC<ExpressionRendererProps>;
@@ -53,73 +56,94 @@ const defaultState: State = {
 export const ExpressionRendererImplementation = ({
   className,
   dataAttrs,
-  expression,
-  renderError,
   padding,
-  ...options
+  renderError,
+  expression,
+  ...expressionLoaderOptions
 }: ExpressionRendererProps) => {
   const mountpoint: React.MutableRefObject<null | HTMLDivElement> = useRef(null);
-  const handlerRef: React.MutableRefObject<null | ExpressionLoader> = useRef(null);
   const [state, setState] = useState<State>({ ...defaultState });
+  const hasCustomRenderErrorHandler = !!renderError;
+  const expressionLoaderRef: React.MutableRefObject<null | ExpressionLoader> = useRef(null);
+  // flag to skip next render$ notification,
+  // because of just handled error
+  const hasHandledErrorRef = useRef(false);
+
+  // will call done() in LayoutEffect when done with rendering custom error state
+  const errorRenderHandlerRef: React.MutableRefObject<null | IInterpreterRenderHandlers> = useRef(
+    null
+  );
+
+  /* eslint-disable react-hooks/exhaustive-deps */
+  // OK to ignore react-hooks/exhaustive-deps because options update is handled by calling .update()
+  useEffect(() => {
+    const subs: Subscription[] = [];
+    expressionLoaderRef.current = new ExpressionLoader(mountpoint.current!, expression, {
+      ...expressionLoaderOptions,
+      // react component wrapper provides different
+      // error handling api which is easier to work with from react
+      // if custom renderError is not provided then we fallback to default error handling from ExpressionLoader
+      onRenderError: hasCustomRenderErrorHandler
+        ? (domNode, error, handlers) => {
+            errorRenderHandlerRef.current = handlers;
+            setState(() => ({
+              ...defaultState,
+              isEmpty: false,
+              error,
+            }));
+
+            if (expressionLoaderOptions.onRenderError) {
+              expressionLoaderOptions.onRenderError(domNode, error, handlers);
+            }
+          }
+        : expressionLoaderOptions.onRenderError,
+    });
+    subs.push(
+      expressionLoaderRef.current.loading$.subscribe(() => {
+        hasHandledErrorRef.current = false;
+        setState(prevState => ({ ...prevState, isLoading: true }));
+      }),
+      expressionLoaderRef.current.render$
+        .pipe(filter(() => !hasHandledErrorRef.current))
+        .subscribe(item => {
+          setState(() => ({
+            ...defaultState,
+            isEmpty: false,
+          }));
+        })
+    );
+
+    return () => {
+      subs.forEach(s => s.unsubscribe());
+      if (expressionLoaderRef.current) {
+        expressionLoaderRef.current.destroy();
+        expressionLoaderRef.current = null;
+      }
+
+      errorRenderHandlerRef.current = null;
+    };
+  }, [hasCustomRenderErrorHandler]);
 
   // Re-fetch data automatically when the inputs change
-  /* eslint-disable react-hooks/exhaustive-deps */
-  useEffect(() => {
-    if (handlerRef.current) {
-      handlerRef.current.update(expression, options);
-    }
-  }, [
-    expression,
-    options.searchContext,
-    options.context,
-    options.variables,
-    options.disableCaching,
-  ]);
-  /* eslint-enable react-hooks/exhaustive-deps */
-
-  // Initialize the loader only once
-  useEffect(() => {
-    if (mountpoint.current && !handlerRef.current) {
-      handlerRef.current = new ExpressionLoader(mountpoint.current, expression, options);
-
-      handlerRef.current.loading$.subscribe(() => {
-        if (!handlerRef.current) {
-          return;
-        }
-        setState(prevState => ({ ...prevState, isLoading: true }));
-      });
-      handlerRef.current.render$.subscribe(item => {
-        if (!handlerRef.current) {
-          return;
-        }
-        if (typeof item !== 'number') {
-          setState(() => ({
-            ...defaultState,
-            isEmpty: false,
-            error: item.error,
-          }));
-        } else {
-          setState(() => ({
-            ...defaultState,
-            isEmpty: false,
-          }));
-        }
-      });
-    }
-  /* eslint-disable */
-  // TODO: Replace mountpoint.current by something else.
-  }, [mountpoint.current]);
-  /* eslint-enable */
-
-  useEffect(() => {
-    // We only want a clean up to run when the entire component is unloaded, not on every render
-    return function cleanup() {
-      if (handlerRef.current) {
-        handlerRef.current.destroy();
-        handlerRef.current = null;
+  useShallowCompareEffect(
+    () => {
+      if (expressionLoaderRef.current) {
+        expressionLoaderRef.current.update(expression, expressionLoaderOptions);
       }
-    };
-  }, []);
+    },
+    // when expression is changed by reference and when any other loaderOption is changed by reference
+    [{ expression, ...expressionLoaderOptions }]
+  );
+
+  /* eslint-enable react-hooks/exhaustive-deps */
+  // call expression loader's done() handler when finished rendering custom error state
+  useLayoutEffect(() => {
+    if (state.error && errorRenderHandlerRef.current) {
+      hasHandledErrorRef.current = true;
+      errorRenderHandlerRef.current.done();
+      errorRenderHandlerRef.current = null;
+    }
+  }, [state.error]);
 
   const classes = classNames('expExpressionRenderer', {
     'expExpressionRenderer-isEmpty': state.isEmpty,
@@ -135,15 +159,9 @@ export const ExpressionRendererImplementation = ({
 
   return (
     <div {...dataAttrs} className={classes}>
-      {state.isEmpty ? <EuiLoadingChart mono size="l" /> : null}
-      {state.isLoading ? <EuiProgress size="xs" color="accent" position="absolute" /> : null}
-      {!state.isLoading && state.error ? (
-        renderError ? (
-          renderError(state.error.message)
-        ) : (
-          <div data-test-subj="expression-renderer-error">{state.error.message}</div>
-        )
-      ) : null}
+      {state.isEmpty && <EuiLoadingChart mono size="l" />}
+      {state.isLoading && <EuiProgress size="xs" color="accent" position="absolute" />}
+      {!state.isLoading && state.error && renderError && renderError(state.error.message)}
       <div
         className="expExpressionRenderer__expression"
         style={expressionStyles}

src/plugins/expressions/public/index.ts

@@ -29,7 +29,7 @@ export { interpreterProvider, ExpressionInterpret } from './interpreter_provider
 export { ExpressionRenderer, ExpressionRendererProps } from './expression_renderer';
 export { ExpressionDataHandler } from './execute';
 
-export { RenderResult, ExpressionRenderHandler } from './render';
+export { ExpressionRenderHandler } from './render';
 
 export function plugin(initializerContext: PluginInitializerContext) {
   return new ExpressionsPublicPlugin(initializerContext);

src/plugins/expressions/public/loader.test.ts

@@ -17,9 +17,9 @@
  * under the License.
  */
 
-import { fromExpression } from '@kbn/interpreter/common';
-import { first, skip } from 'rxjs/operators';
 import { Observable } from 'rxjs';
+import { first, skip, toArray } from 'rxjs/operators';
+import { fromExpression } from '@kbn/interpreter/common';
 import { loader, ExpressionLoader } from './loader';
 import { ExpressionDataHandler } from './execute';
 import { IInterpreterRenderHandlers } from './types';
@@ -46,6 +46,13 @@ jest.mock('./services', () => {
     getRenderersRegistry: () => ({
       get: (id: string) => renderers[id],
     }),
+    getNotifications: jest.fn(() => {
+      return {
+        toasts: {
+          addError: jest.fn(() => {}),
+        },
+      };
+    }),
   };
 });
 
@@ -97,20 +104,14 @@ describe('ExpressionLoader', () => {
     expect(response).toEqual({ type: 'render', as: 'test' });
   });
 
-  it('emits on loading$ when starting to load', async () => {
+  it('emits on loading$ on initial load and on updates', async () => {
     const expressionLoader = new ExpressionLoader(element, expressionString, {});
-    let loadingPromise = expressionLoader.loading$.pipe(first()).toPromise();
+    const loadingPromise = expressionLoader.loading$.pipe(toArray()).toPromise();
     expressionLoader.update('test');
-    let response = await loadingPromise;
-    expect(response).toBeUndefined();
-    loadingPromise = expressionLoader.loading$.pipe(first()).toPromise();
     expressionLoader.update('');
-    response = await loadingPromise;
-    expect(response).toBeUndefined();
-    loadingPromise = expressionLoader.loading$.pipe(first()).toPromise();
     expressionLoader.update();
-    response = await loadingPromise;
-    expect(response).toBeUndefined();
+    expressionLoader.destroy();
+    expect(await loadingPromise).toHaveLength(4);
   });
 
   it('emits on render$ when rendering is done', async () => {

src/plugins/expressions/public/loader.ts

@@ -17,8 +17,8 @@
  * under the License.
  */
 
-import { Observable, Subject } from 'rxjs';
-import { share } from 'rxjs/operators';
+import { BehaviorSubject, Observable, Subject } from 'rxjs';
+import { filter, map } from 'rxjs/operators';
 import { Adapters, InspectorSession } from '../../inspector/public';
 import { ExpressionDataHandler } from './execute';
 import { ExpressionRenderHandler } from './render';
@@ -36,7 +36,7 @@ export class ExpressionLoader {
   private dataHandler: ExpressionDataHandler | undefined;
   private renderHandler: ExpressionRenderHandler;
   private dataSubject: Subject<Data>;
-  private loadingSubject: Subject<void>;
+  private loadingSubject: Subject<boolean>;
   private data: Data;
   private params: IExpressionLoaderParams = {};
 
@@ -46,12 +46,20 @@ export class ExpressionLoader {
     params?: IExpressionLoaderParams
   ) {
     this.dataSubject = new Subject();
-    this.data$ = this.dataSubject.asObservable().pipe(share());
+    this.data$ = this.dataSubject.asObservable();
 
-    this.loadingSubject = new Subject();
-    this.loading$ = this.loadingSubject.asObservable().pipe(share());
+    this.loadingSubject = new BehaviorSubject<boolean>(false);
+    // loading is a "hot" observable,
+    // as loading$ could emit straight away in the constructor
+    // and we want to notify subscribers about it, but all subscriptions will happen later
+    this.loading$ = this.loadingSubject.asObservable().pipe(
+      filter(_ => _ === true),
+      map(() => void 0)
+    );
 
-    this.renderHandler = new ExpressionRenderHandler(element);
+    this.renderHandler = new ExpressionRenderHandler(element, {
+      onRenderError: params && params.onRenderError,
+    });
     this.render$ = this.renderHandler.render$;
     this.update$ = this.renderHandler.update$;
     this.events$ = this.renderHandler.events$;
@@ -64,9 +72,14 @@ export class ExpressionLoader {
       this.render(data);
     });
 
+    this.render$.subscribe(() => {
+      this.loadingSubject.next(false);
+    });
+
     this.setParams(params);
 
     if (expression) {
+      this.loadingSubject.next(true);
       this.loadData(expression, this.params);
     }
   }
@@ -120,7 +133,7 @@ export class ExpressionLoader {
   update(expression?: string | ExpressionAST, params?: IExpressionLoaderParams): void {
     this.setParams(params);
 
-    this.loadingSubject.next();
+    this.loadingSubject.next(true);
     if (expression) {
       this.loadData(expression, this.params);
     } else if (this.data) {

src/plugins/expressions/public/plugin.ts

@@ -21,7 +21,13 @@ import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from '../../..
 import { ExpressionInterpretWithHandlers, ExpressionExecutor } from './types';
 import { FunctionsRegistry, RenderFunctionsRegistry, TypesRegistry } from './registries';
 import { Setup as InspectorSetup, Start as InspectorStart } from '../../inspector/public';
-import { setCoreStart, setInspector, setInterpreter, setRenderersRegistry } from './services';
+import {
+  setCoreStart,
+  setInspector,
+  setInterpreter,
+  setRenderersRegistry,
+  setNotifications,
+} from './services';
 import { clog as clogFunction } from './functions/clog';
 import { font as fontFunction } from './functions/font';
 import { kibana as kibanaFunction } from './functions/kibana';
@@ -158,6 +164,7 @@ export class ExpressionsPublicPlugin
   public start(core: CoreStart, { inspector }: ExpressionsStartDeps): ExpressionsStart {
     setCoreStart(core);
     setInspector(inspector);
+    setNotifications(core.notifications);
 
     return {
       execute,

src/plugins/expressions/public/render.test.ts

@@ -17,14 +17,18 @@
  * under the License.
  */
 
-import { render, ExpressionRenderHandler } from './render';
+import { ExpressionRenderHandler, render } from './render';
 import { Observable } from 'rxjs';
-import { IInterpreterRenderHandlers } from './types';
+import { IInterpreterRenderHandlers, RenderError } from './types';
 import { getRenderersRegistry } from './services';
-import { first } from 'rxjs/operators';
+import { first, take, toArray } from 'rxjs/operators';
 
 const element: HTMLElement = {} as HTMLElement;
-
+const mockNotificationService = {
+  toasts: {
+    addError: jest.fn(() => {}),
+  },
+};
 jest.mock('./services', () => {
   const renderers: Record<string, unknown> = {
     test: {
@@ -38,9 +42,24 @@ jest.mock('./services', () => {
     getRenderersRegistry: jest.fn(() => ({
       get: jest.fn((id: string) => renderers[id]),
     })),
+    getNotifications: jest.fn(() => {
+      return mockNotificationService;
+    }),
   };
 });
 
+const mockMockErrorRenderFunction = jest.fn(
+  (el: HTMLElement, error: RenderError, handlers: IInterpreterRenderHandlers) => handlers.done()
+);
+// extracts data from mockMockErrorRenderFunction call to assert in tests
+const getHandledError = () => {
+  try {
+    return mockMockErrorRenderFunction.mock.calls[0][1];
+  } catch (e) {
+    return null;
+  }
+};
+
 describe('render helper function', () => {
   it('returns ExpressionRenderHandler instance', () => {
     const response = render(element, {});
@@ -62,40 +81,33 @@ describe('ExpressionRenderHandler', () => {
   });
 
   describe('render()', () => {
-    it('sends an observable error and keeps it open if invalid data is provided', async () => {
+    beforeEach(() => {
+      mockMockErrorRenderFunction.mockClear();
+      mockNotificationService.toasts.addError.mockClear();
+    });
+
+    it('in case of error render$ should emit when error renderer is finished', async () => {
       const expressionRenderHandler = new ExpressionRenderHandler(element);
+      expressionRenderHandler.render(false);
       const promise1 = expressionRenderHandler.render$.pipe(first()).toPromise();
-      expressionRenderHandler.render(false);
-      await expect(promise1).resolves.toEqual({
-        type: 'error',
-        error: {
-          message: 'invalid data provided to the expression renderer',
-        },
-      });
+      await expect(promise1).resolves.toEqual(1);
 
+      expressionRenderHandler.render(false);
       const promise2 = expressionRenderHandler.render$.pipe(first()).toPromise();
-      expressionRenderHandler.render(false);
-      await expect(promise2).resolves.toEqual({
-        type: 'error',
-        error: {
-          message: 'invalid data provided to the expression renderer',
-        },
-      });
+      await expect(promise2).resolves.toEqual(2);
     });
 
-    it('sends an observable error if renderer does not exist', async () => {
-      const expressionRenderHandler = new ExpressionRenderHandler(element);
-      const promise = expressionRenderHandler.render$.pipe(first()).toPromise();
-      expressionRenderHandler.render({ type: 'render', as: 'something' });
-      await expect(promise).resolves.toEqual({
-        type: 'error',
-        error: {
-          message: `invalid renderer id 'something'`,
-        },
+    it('should use custom error handler if provided', async () => {
+      const expressionRenderHandler = new ExpressionRenderHandler(element, {
+        onRenderError: mockMockErrorRenderFunction,
       });
+      await expressionRenderHandler.render(false);
+      expect(getHandledError()!.message).toEqual(
+        `invalid data provided to the expression renderer`
+      );
     });
 
-    it('sends an observable error if the rendering function throws', async () => {
+    it('should throw error if the rendering function throws', async () => {
       (getRenderersRegistry as jest.Mock).mockReturnValueOnce({ get: () => true });
       (getRenderersRegistry as jest.Mock).mockReturnValueOnce({
         get: () => ({
@@ -105,15 +117,11 @@ describe('ExpressionRenderHandler', () => {
         }),
       });
 
-      const expressionRenderHandler = new ExpressionRenderHandler(element);
-      const promise = expressionRenderHandler.render$.pipe(first()).toPromise();
-      expressionRenderHandler.render({ type: 'render', as: 'something' });
-      await expect(promise).resolves.toEqual({
-        type: 'error',
-        error: {
-          message: 'renderer error',
-        },
+      const expressionRenderHandler = new ExpressionRenderHandler(element, {
+        onRenderError: mockMockErrorRenderFunction,
       });
+      await expressionRenderHandler.render({ type: 'render', as: 'something' });
+      expect(getHandledError()!.message).toEqual('renderer error');
     });
 
     it('sends a next observable once rendering is complete', () => {
@@ -129,18 +137,56 @@ describe('ExpressionRenderHandler', () => {
       });
     });
 
+    it('default renderer should use notification service', async () => {
+      const expressionRenderHandler = new ExpressionRenderHandler(element);
+      const promise1 = expressionRenderHandler.render$.pipe(first()).toPromise();
+      expressionRenderHandler.render(false);
+      await expect(promise1).resolves.toEqual(1);
+      expect(mockNotificationService.toasts.addError).toBeCalledWith(
+        expect.objectContaining({
+          message: 'invalid data provided to the expression renderer',
+        }),
+        {
+          title: 'Error in visualisation',
+          toastMessage: 'invalid data provided to the expression renderer',
+        }
+      );
+    });
+
     // in case render$ subscription happen after render() got called
     // we still want to be notified about sync render$ updates
     it("doesn't swallow sync render errors", async () => {
+      const expressionRenderHandler1 = new ExpressionRenderHandler(element, {
+        onRenderError: mockMockErrorRenderFunction,
+      });
+      expressionRenderHandler1.render(false);
+      const renderPromiseAfterRender = expressionRenderHandler1.render$.pipe(first()).toPromise();
+      await expect(renderPromiseAfterRender).resolves.toEqual(1);
+      expect(getHandledError()!.message).toEqual(
+        'invalid data provided to the expression renderer'
+      );
+
+      mockMockErrorRenderFunction.mockClear();
+
+      const expressionRenderHandler2 = new ExpressionRenderHandler(element, {
+        onRenderError: mockMockErrorRenderFunction,
+      });
+      const renderPromiseBeforeRender = expressionRenderHandler2.render$.pipe(first()).toPromise();
+      expressionRenderHandler2.render(false);
+      await expect(renderPromiseBeforeRender).resolves.toEqual(1);
+      expect(getHandledError()!.message).toEqual(
+        'invalid data provided to the expression renderer'
+      );
+    });
+
+    // it is expected side effect of using BehaviorSubject for render$,
+    // that observables will emit previous result if subscription happens after render
+    it('should emit previous render and error results', async () => {
       const expressionRenderHandler = new ExpressionRenderHandler(element);
       expressionRenderHandler.render(false);
-      const promise = expressionRenderHandler.render$.pipe(first()).toPromise();
-      await expect(promise).resolves.toEqual({
-        type: 'error',
-        error: {
-          message: 'invalid data provided to the expression renderer',
-        },
-      });
+      const renderPromise = expressionRenderHandler.render$.pipe(take(2), toArray()).toPromise();
+      expressionRenderHandler.render(false);
+      await expect(renderPromise).resolves.toEqual([1, 2]);
     });
   });
 });

src/plugins/expressions/public/render.ts

@@ -17,48 +17,58 @@
  * under the License.
  */
 
-import { Observable } from 'rxjs';
 import * as Rx from 'rxjs';
-import { filter, share } from 'rxjs/operators';
-import { event, RenderId, Data, IInterpreterRenderHandlers } from './types';
+import { Observable } from 'rxjs';
+import { filter } from 'rxjs/operators';
+import {
+  Data,
+  event,
+  IInterpreterRenderHandlers,
+  RenderError,
+  RenderErrorHandlerFnType,
+  RenderId,
+} from './types';
 import { getRenderersRegistry } from './services';
-
-interface RenderError {
-  type: 'error';
-  error: { type?: string; message: string };
-}
+import { renderErrorHandler as defaultRenderErrorHandler } from './render_error_handler';
 
 export type IExpressionRendererExtraHandlers = Record<string, any>;
 
-export type RenderResult = RenderId | RenderError;
+export interface ExpressionRenderHandlerParams {
+  onRenderError: RenderErrorHandlerFnType;
+}
 
 export class ExpressionRenderHandler {
-  render$: Observable<RenderResult>;
+  render$: Observable<RenderId>;
   update$: Observable<any>;
   events$: Observable<event>;
 
   private element: HTMLElement;
   private destroyFn?: any;
   private renderCount: number = 0;
-  private renderSubject: Rx.BehaviorSubject<RenderResult | null>;
+  private renderSubject: Rx.BehaviorSubject<RenderId | null>;
   private eventsSubject: Rx.Subject<unknown>;
   private updateSubject: Rx.Subject<unknown>;
   private handlers: IInterpreterRenderHandlers;
+  private onRenderError: RenderErrorHandlerFnType;
 
-  constructor(element: HTMLElement) {
+  constructor(
+    element: HTMLElement,
+    { onRenderError }: Partial<ExpressionRenderHandlerParams> = {}
+  ) {
     this.element = element;
 
     this.eventsSubject = new Rx.Subject();
-    this.events$ = this.eventsSubject.asObservable().pipe(share());
+    this.events$ = this.eventsSubject.asObservable();
 
-    this.renderSubject = new Rx.BehaviorSubject(null as RenderResult | null);
-    this.render$ = this.renderSubject.asObservable().pipe(
-      share(),
-      filter(_ => _ !== null)
-    ) as Observable<RenderResult>;
+    this.onRenderError = onRenderError || defaultRenderErrorHandler;
+
+    this.renderSubject = new Rx.BehaviorSubject(null as RenderId | null);
+    this.render$ = this.renderSubject.asObservable().pipe(filter(_ => _ !== null)) as Observable<
+      RenderId
+    >;
 
     this.updateSubject = new Rx.Subject();
-    this.update$ = this.updateSubject.asObservable().pipe(share());
+    this.update$ = this.updateSubject.asObservable();
 
     this.handlers = {
       onDestroy: (fn: any) => {
@@ -82,33 +92,21 @@ export class ExpressionRenderHandler {
 
   render = async (data: Data, extraHandlers: IExpressionRendererExtraHandlers = {}) => {
     if (!data || typeof data !== 'object') {
-      this.renderSubject.next({
-        type: 'error',
-        error: {
-          message: 'invalid data provided to the expression renderer',
-        },
-      });
-      return;
+      return this.handleRenderError(new Error('invalid data provided to the expression renderer'));
     }
 
     if (data.type !== 'render' || !data.as) {
       if (data.type === 'error') {
-        this.renderSubject.next(data);
+        return this.handleRenderError(data.error);
       } else {
-        this.renderSubject.next({
-          type: 'error',
-          error: { message: 'invalid data provided to the expression renderer' },
-        });
+        return this.handleRenderError(
+          new Error('invalid data provided to the expression renderer')
+        );
       }
-      return;
     }
 
     if (!getRenderersRegistry().get(data.as)) {
-      this.renderSubject.next({
-        type: 'error',
-        error: { message: `invalid renderer id '${data.as}'` },
-      });
-      return;
+      return this.handleRenderError(new Error(`invalid renderer id '${data.as}'`));
     }
 
     try {
@@ -117,10 +115,7 @@ export class ExpressionRenderHandler {
         .get(data.as)!
         .render(this.element, data.value, { ...this.handlers, ...extraHandlers });
     } catch (e) {
-      this.renderSubject.next({
-        type: 'error',
-        error: { type: e.type, message: e.message },
-      });
+      return this.handleRenderError(e);
     }
   };
 
@@ -136,10 +131,18 @@ export class ExpressionRenderHandler {
   getElement = () => {
     return this.element;
   };
+
+  handleRenderError = (error: RenderError) => {
+    this.onRenderError(this.element, error, this.handlers);
+  };
 }
 
-export function render(element: HTMLElement, data: Data): ExpressionRenderHandler {
-  const handler = new ExpressionRenderHandler(element);
+export function render(
+  element: HTMLElement,
+  data: Data,
+  options?: Partial<ExpressionRenderHandlerParams>
+): ExpressionRenderHandler {
+  const handler = new ExpressionRenderHandler(element, options);
   handler.render(data);
   return handler;
 }

src/plugins/expressions/public/render_error_handler.ts

@@ -0,0 +1,36 @@
+/*
+ * 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 { i18n } from '@kbn/i18n';
+import { RenderErrorHandlerFnType, IInterpreterRenderHandlers, RenderError } from './types';
+import { getNotifications } from './services';
+
+export const renderErrorHandler: RenderErrorHandlerFnType = (
+  element: HTMLElement,
+  error: RenderError,
+  handlers: IInterpreterRenderHandlers
+) => {
+  getNotifications().toasts.addError(error, {
+    title: i18n.translate('expressions.defaultErrorRenderer.errorTitle', {
+      defaultMessage: 'Error in visualisation',
+    }),
+    toastMessage: error.message,
+  });
+  handlers.done();
+};

src/plugins/expressions/public/services.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { NotificationsStart } from 'kibana/public';
 import { createKibanaUtilsCore, createGetterSetter } from '../../kibana_utils/public';
 import { ExpressionInterpreter } from './types';
 import { Start as IInspector } from '../../inspector/public';
@@ -29,6 +30,9 @@ export const [getInspector, setInspector] = createGetterSetter<IInspector>('Insp
 export const [getInterpreter, setInterpreter] = createGetterSetter<ExpressionInterpreter>(
   'Interpreter'
 );
+export const [getNotifications, setNotifications] = createGetterSetter<NotificationsStart>(
+  'Notifications'
+);
 
 export const [getRenderersRegistry, setRenderersRegistry] = createGetterSetter<
   ExpressionsSetup['__LEGACY']['renderers']

src/plugins/expressions/public/types/index.ts

@@ -20,6 +20,7 @@
 import { ExpressionInterpret } from '../interpreter_provider';
 import { TimeRange, Query, esFilters } from '../../../data/public';
 import { Adapters } from '../../../inspector/public';
+import { ExpressionRenderDefinition } from '../registries';
 
 export type ExpressionInterpretWithHandlers = (
   ast: Parameters<ExpressionInterpret>[0],
@@ -58,6 +59,7 @@ export interface IExpressionLoaderParams {
   customRenderers?: [];
   extraHandlers?: Record<string, any>;
   inspectorAdapters?: Adapters;
+  onRenderError?: RenderErrorHandlerFnType;
 }
 
 export interface IInterpreterHandlers {
@@ -99,3 +101,15 @@ export interface IInterpreterSuccessResult {
 }
 
 export type IInterpreterResult = IInterpreterSuccessResult & IInterpreterErrorResult;
+
+export { ExpressionRenderDefinition };
+
+export interface RenderError extends Error {
+  type?: string;
+}
+
+export type RenderErrorHandlerFnType = (
+  domNode: HTMLElement,
+  error: RenderError,
+  handlers: IInterpreterRenderHandlers
+) => void;

src/plugins/kibana_react/public/index.ts

@@ -24,4 +24,4 @@ export * from './overlays';
 export * from './ui_settings';
 export * from './field_icon';
 export * from './table_list_view';
-export { toMountPoint } from './util';
+export { toMountPoint, useShallowCompareEffect } from './util';

src/plugins/kibana_react/public/util/index.ts

@@ -20,3 +20,4 @@
 export * from './use_observable';
 export * from './use_unmount';
 export * from './react_mount';
+export * from './use_shallow_compare_effect';

src/plugins/kibana_react/public/util/use_shallow_compare_effect.test.ts

@@ -0,0 +1,86 @@
+/*
+ * 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 { renderHook } from 'react-hooks-testing-library';
+import { useShallowCompareEffect } from './use_shallow_compare_effect';
+
+describe('useShallowCompareEffect', () => {
+  test("doesn't run effect on shallow change", () => {
+    const callback = jest.fn();
+    let deps = [1, { a: 'b' }, true];
+    const { rerender } = renderHook(() => useShallowCompareEffect(callback, deps));
+
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+
+    // no change
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(0);
+    callback.mockClear();
+
+    // no-change (new object with same properties)
+    deps = [1, { a: 'b' }, true];
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(0);
+    callback.mockClear();
+
+    // change (new primitive value)
+    deps = [2, { a: 'b' }, true];
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+
+    // no-change
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(0);
+    callback.mockClear();
+
+    // change (new primitive value)
+    deps = [1, { a: 'b' }, false];
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+
+    // change (new properties on object)
+    deps = [1, { a: 'c' }, false];
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+  });
+
+  test('runs effect on deep change', () => {
+    const callback = jest.fn();
+    let deps = [1, { a: { b: 'c' } }, true];
+    const { rerender } = renderHook(() => useShallowCompareEffect(callback, deps));
+
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+
+    // no change
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(0);
+    callback.mockClear();
+
+    // change (new nested object )
+    deps = [1, { a: { b: 'c' } }, true];
+    rerender();
+    expect(callback).toHaveBeenCalledTimes(1);
+    callback.mockClear();
+  });
+});

src/plugins/kibana_react/public/util/use_shallow_compare_effect.ts

@@ -0,0 +1,80 @@
+/*
+ * 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, { useEffect, useRef } from 'react';
+
+/**
+ * Similar to https://github.com/kentcdodds/use-deep-compare-effect
+ * but uses shallow compare instead of deep
+ */
+export function useShallowCompareEffect(
+  callback: React.EffectCallback,
+  deps: React.DependencyList
+) {
+  useEffect(callback, useShallowCompareMemoize(deps));
+}
+function useShallowCompareMemoize(deps: React.DependencyList) {
+  const ref = useRef<React.DependencyList | undefined>(undefined);
+
+  if (!ref.current || deps.some((dep, index) => !shallowEqual(dep, ref.current![index]))) {
+    ref.current = deps;
+  }
+
+  return ref.current;
+}
+// https://github.com/facebook/fbjs/blob/master/packages/fbjs/src/core/shallowEqual.js
+function shallowEqual(objA: any, objB: any): boolean {
+  if (is(objA, objB)) {
+    return true;
+  }
+
+  if (typeof objA !== 'object' || objA === null || typeof objB !== 'object' || objB === null) {
+    return false;
+  }
+
+  const keysA = Object.keys(objA);
+  const keysB = Object.keys(objB);
+
+  if (keysA.length !== keysB.length) {
+    return false;
+  }
+
+  // Test for A's keys different from B.
+  for (let i = 0; i < keysA.length; i++) {
+    if (
+      !Object.prototype.hasOwnProperty.call(objB, keysA[i]) ||
+      !is(objA[keysA[i]], objB[keysA[i]])
+    ) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+/**
+ * IE11 does not support Object.is
+ */
+function is(x: any, y: any): boolean {
+  if (x === y) {
+    return x !== 0 || y !== 0 || 1 / x === 1 / y;
+  } else {
+    return x !== x && y !== y;
+  }
+}

test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/np_ready/app/components/main.tsx

@@ -29,7 +29,7 @@ import {
   Context,
   ExpressionRenderHandler,
   ExpressionDataHandler,
-  RenderResult,
+  RenderId,
 } from '../../types';
 import { getExpressions } from '../../services';
 
@@ -40,7 +40,7 @@ declare global {
       context?: Context,
       initialContext?: Context
     ) => ReturnType<ExpressionDataHandler['getData']>;
-    renderPipelineResponse: (context?: Context) => Promise<RenderResult>;
+    renderPipelineResponse: (context?: Context) => Promise<RenderId>;
   }
 }
 
@@ -85,16 +85,16 @@ class Main extends React.Component<{}, State> {
         lastRenderHandler.destroy();
       }
 
-      lastRenderHandler = getExpressions().render(this.chartRef.current!, context);
-      const renderResult = await lastRenderHandler.render$.pipe(first()).toPromise();
+      lastRenderHandler = getExpressions().render(this.chartRef.current!, context, {
+        onRenderError: (el, error, handler) => {
+          this.setState({
+            expression: 'Render error!\n\n' + JSON.stringify(error),
+          });
+          handler.done();
+        },
+      });
 
-      if (typeof renderResult === 'object' && renderResult.type === 'error') {
-        this.setState({
-          expression: 'Render error!\n\n' + JSON.stringify(renderResult.error),
-        });
-      }
-
-      return renderResult;
+      return lastRenderHandler.render$.pipe(first()).toPromise();
     };
   }
 

test/interpreter_functional/plugins/kbn_tp_run_pipeline/public/np_ready/types.ts

@@ -22,7 +22,7 @@ import {
   Context,
   ExpressionRenderHandler,
   ExpressionDataHandler,
-  RenderResult,
+  RenderId,
 } from 'src/plugins/expressions/public';
 
 import { Adapters } from 'src/plugins/inspector/public';
@@ -32,6 +32,6 @@ export {
   Context,
   ExpressionRenderHandler,
   ExpressionDataHandler,
-  RenderResult,
+  RenderId,
   Adapters,
 };

test/interpreter_functional/test_suites/run_pipeline/helpers.ts

@@ -21,8 +21,8 @@ import expect from '@kbn/expect';
 import { FtrProviderContext } from '../../../functional/ftr_provider_context';
 import {
   ExpressionDataHandler,
-  RenderResult,
   Context,
+  RenderId,
 } from '../../plugins/kbn_tp_run_pipeline/public/np_ready/types';
 
 type UnWrapPromise<T> = T extends Promise<infer U> ? U : T;
@@ -168,8 +168,8 @@ export function expectExpressionProvider({
       toMatchScreenshot: async () => {
         const pipelineResponse = await handler.getResponse();
         log.debug('starting to render');
-        const result = await browser.executeAsync<RenderResult>(
-          (_context: ExpressionResult, done: (renderResult: RenderResult) => void) =>
+        const result = await browser.executeAsync<RenderId>(
+          (_context: ExpressionResult, done: (renderResult: RenderId) => void) =>
             window.renderPipelineResponse(_context).then(renderResult => {
               done(renderResult);
               return renderResult;

x-pack/legacy/plugins/lens/public/editor_frame_plugin/embeddable/expression_wrapper.tsx

@@ -50,6 +50,7 @@ export function ExpressionWrapper({
             padding="m"
             expression={expression}
             searchContext={{ ...context, type: 'kibana_context' }}
+            renderError={error => <div data-test-subj="expression-renderer-error">{error}</div>}
           />
         </div>
       )}