[Reporting] Handle error gracefully when browser driver unexpectedly exits or crashes. (#71989) (#74190)

* [Reporting] Handle page error event directly with Puppeteer

* clean up logger.error that was stringifying error

* use fromEvent

* better handling and error messaging if browser was closed in the middle of the screenshot pipeline

* fix pdf functional api tests

* fix i18n error

* fix jest

* fix ts

* fix i18n

* tweaks

* ok to throw error in callback

* fix ts

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Author: tsullivan

x-pack/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts

@@ -9,6 +9,7 @@ import { map, truncate } from 'lodash';
 import open from 'opn';
 import { ElementHandle, EvaluateFn, Page, Response, SerializableOrJSHandle } from 'puppeteer';
 import { parse as parseUrl } from 'url';
+import { getDisallowedOutgoingUrlError } from '../';
 import { LevelLogger } from '../../../lib';
 import { ViewZoomWidthHeight } from '../../../lib/layouts/layout';
 import { ConditionalHeaders, ElementPosition } from '../../../types';
@@ -76,6 +77,9 @@ export class HeadlessChromiumDriver {
     });
   }
 
+  /*
+   * Call Page.goto and wait to see the Kibana DOM content
+   */
   public async open(
     url: string,
     {
@@ -113,6 +117,16 @@ export class HeadlessChromiumDriver {
     logger.info(`handled ${this.interceptedCount} page requests`);
   }
 
+  /*
+   * Let modules poll if Chrome is still running so they can short circuit if needed
+   */
+  public isPageOpen() {
+    return !this.page.isClosed();
+  }
+
+  /*
+   * Call Page.screenshot and return a base64-encoded string of the image
+   */
   public async screenshot(elementPosition: ElementPosition): Promise<string> {
     const { boundingClientRect, scroll } = elementPosition;
     const screenshot = await this.page.screenshot({
@@ -220,18 +234,13 @@ export class HeadlessChromiumDriver {
 
       // We should never ever let file protocol requests go through
       if (!allowed || !this.allowRequest(interceptedUrl)) {
-        logger.error(`Got bad URL: "${interceptedUrl}", closing browser.`);
         await client.send('Fetch.failRequest', {
           errorReason: 'Aborted',
           requestId,
         });
         this.page.browser().close();
-        throw new Error(
-          i18n.translate('xpack.reporting.chromiumDriver.disallowedOutgoingUrl', {
-            defaultMessage: `Received disallowed outgoing URL: "{interceptedUrl}", exiting`,
-            values: { interceptedUrl },
-          })
-        );
+        logger.error(getDisallowedOutgoingUrlError(interceptedUrl));
+        return;
       }
 
       if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedUrl)) {
@@ -292,9 +301,9 @@ export class HeadlessChromiumDriver {
       }
 
       if (!allowed || !this.allowRequest(interceptedUrl)) {
-        logger.error(`Got disallowed URL "${interceptedUrl}", closing browser.`);
         this.page.browser().close();
-        throw new Error(`Received disallowed URL in response: ${interceptedUrl}`);
+        logger.error(getDisallowedOutgoingUrlError(interceptedUrl));
+        throw getDisallowedOutgoingUrlError(interceptedUrl);
       }
     });
 

x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts

@@ -19,6 +19,7 @@ import {
 import * as Rx from 'rxjs';
 import { InnerSubscriber } from 'rxjs/internal/InnerSubscriber';
 import { ignoreElements, map, mergeMap, tap } from 'rxjs/operators';
+import { getChromiumDisconnectedError } from '../';
 import { BROWSER_TYPE } from '../../../../common/constants';
 import { CaptureConfig } from '../../../../server/types';
 import { LevelLogger } from '../../../lib';
@@ -115,13 +116,19 @@ export class HeadlessChromiumDriverFactory {
 
         logger.debug(`Browser page driver created`);
       } catch (err) {
-        observer.error(new Error(`Error spawning Chromium browser: [${err}]`));
+        observer.error(new Error(`Error spawning Chromium browser!`));
+        observer.error(err);
         throw err;
       }
 
       const childProcess = {
         async kill() {
-          await browser.close();
+          try {
+            await browser.close();
+          } catch (err) {
+            // do not throw
+            logger.error(err);
+          }
         },
       };
       const { terminate$ } = safeChildProcess(logger, childProcess);
@@ -167,7 +174,8 @@ export class HeadlessChromiumDriverFactory {
         // the unsubscribe function isn't `async` so we're going to make our best effort at
         // deleting the userDataDir and if it fails log an error.
         del(userDataDir, { force: true }).catch((error) => {
-          logger.error(`error deleting user data directory at [${userDataDir}]: [${error}]`);
+          logger.error(`error deleting user data directory at [${userDataDir}]!`);
+          logger.error(error);
         });
       });
     });
@@ -219,7 +227,7 @@ export class HeadlessChromiumDriverFactory {
       mergeMap((err) => {
         return Rx.throwError(
           i18n.translate('xpack.reporting.browsers.chromium.errorDetected', {
-            defaultMessage: 'Reporting detected an error: {err}',
+            defaultMessage: 'Reporting encountered an error: {err}',
             values: { err: err.toString() },
           })
         );
@@ -230,23 +238,15 @@ export class HeadlessChromiumDriverFactory {
       mergeMap((err) => {
         return Rx.throwError(
           i18n.translate('xpack.reporting.browsers.chromium.pageErrorDetected', {
-            defaultMessage: `Reporting detected an error on the page: {err}`,
+            defaultMessage: `Reporting encountered an error on the page: {err}`,
             values: { err: err.toString() },
           })
         );
       })
     );
 
     const browserDisconnect$ = Rx.fromEvent(browser, 'disconnected').pipe(
-      mergeMap(() =>
-        Rx.throwError(
-          new Error(
-            i18n.translate('xpack.reporting.browsers.chromium.chromiumClosed', {
-              defaultMessage: `Reporting detected that Chromium has closed.`,
-            })
-          )
-        )
-      )
+      mergeMap(() => Rx.throwError(getChromiumDisconnectedError()))
     );
 
     return Rx.merge(pageError$, uncaughtExceptionPageError$, browserDisconnect$);

x-pack/plugins/reporting/server/browsers/chromium/index.ts

@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { i18n } from '@kbn/i18n';
 import { BrowserDownload } from '../';
 import { CaptureConfig } from '../../../server/types';
 import { LevelLogger } from '../../lib';
@@ -15,3 +16,18 @@ export const chromium: BrowserDownload = {
   createDriverFactory: (binaryPath: string, captureConfig: CaptureConfig, logger: LevelLogger) =>
     new HeadlessChromiumDriverFactory(binaryPath, captureConfig, logger),
 };
+
+export const getChromiumDisconnectedError = () =>
+  new Error(
+    i18n.translate('xpack.reporting.screencapture.browserWasClosed', {
+      defaultMessage: 'Browser was closed unexpectedly! Check the server logs for more info.',
+    })
+  );
+
+export const getDisallowedOutgoingUrlError = (interceptedUrl: string) =>
+  new Error(
+    i18n.translate('xpack.reporting.chromiumDriver.disallowedOutgoingUrl', {
+      defaultMessage: `Received disallowed outgoing URL: "{interceptedUrl}". Failing the request and closing the browser.`,
+      values: { interceptedUrl },
+    })
+  );

x-pack/plugins/reporting/server/export_types/csv_from_savedobject/create_job.ts

@@ -105,9 +105,7 @@ export const scheduleTaskFnFactory: ScheduleTaskFnFactory<ImmediateCreateJobFn>
         if (errPayload.statusCode === 404) {
           throw notFound(errPayload.message);
         }
-        if (err.stack) {
-          logger.error(err.stack);
-        }
+        logger.error(err);
         throw new Error(`Unable to create a job from saved object data! Error: ${err}`);
       });
 

x-pack/plugins/reporting/server/export_types/printable_pdf/lib/generate_pdf.ts

@@ -90,7 +90,8 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
           logger.debug(`PDF buffer byte length: ${buffer?.byteLength || 0}`);
           tracker.endGetBuffer();
         } catch (err) {
-          logger.error(`Could not generate the PDF buffer! ${err}`);
+          logger.error(`Could not generate the PDF buffer!`);
+          logger.error(err);
         }
 
         tracker.end();

x-pack/plugins/reporting/server/lib/screenshots/check_browser_open.ts

@@ -0,0 +1,21 @@
+/*
+ * 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 { HeadlessChromiumDriver } from '../../browsers';
+import { getChromiumDisconnectedError } from '../../browsers/chromium';
+
+/*
+ * Call this function within error-handling `catch` blocks while setup and wait
+ * for the Kibana URL to be ready for screenshot. This detects if a block of
+ * code threw an exception because the page is closed or crashed.
+ *
+ * Also call once after `setup$` fires in the screenshot pipeline
+ */
+export const checkPageIsOpen = (browser: HeadlessChromiumDriver) => {
+  if (!browser.isPageOpen()) {
+    throw getChromiumDisconnectedError();
+  }
+};

x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts

@@ -281,7 +281,7 @@ describe('Screenshot Observable Pipeline', () => {
             `);
     });
 
-    it('recovers if exit$ fires a timeout signal', async () => {
+    it('observes page exit', async () => {
       // mocks
       const mockGetCreatePage = (driver: HeadlessChromiumDriver) =>
         jest
@@ -311,38 +311,7 @@ describe('Screenshot Observable Pipeline', () => {
         }).toPromise();
       };
 
-      await expect(getScreenshot()).resolves.toMatchInlineSnapshot(`
-              Array [
-                Object {
-                  "elementsPositionAndAttributes": Array [
-                    Object {
-                      "attributes": Object {},
-                      "position": Object {
-                        "boundingClientRect": Object {
-                          "height": 200,
-                          "left": 0,
-                          "top": 0,
-                          "width": 200,
-                        },
-                        "scroll": Object {
-                          "x": 0,
-                          "y": 0,
-                        },
-                      },
-                    },
-                  ],
-                  "error": "Instant timeout has fired!",
-                  "screenshots": Array [
-                    Object {
-                      "base64EncodedData": "allyourBase64",
-                      "description": undefined,
-                      "title": undefined,
-                    },
-                  ],
-                  "timeRange": null,
-                },
-              ]
-            `);
+      await expect(getScreenshot()).rejects.toMatchInlineSnapshot(`"Instant timeout has fired!"`);
     });
 
     it(`uses defaults for element positions and size when Kibana page is not ready`, async () => {

x-pack/plugins/reporting/server/lib/screenshots/observable.ts

@@ -24,6 +24,7 @@ import {
   ScreenshotResults,
   ScreenshotsObservableFn,
 } from '../../types';
+import { checkPageIsOpen } from './check_browser_open';
 import { DEFAULT_PAGELOAD_SELECTOR } from './constants';
 import { getElementPositionAndAttributes } from './get_element_position_data';
 import { getNumberOfItems } from './get_number_of_items';
@@ -68,7 +69,6 @@ export function screenshotsObservableFactory(
         return Rx.from(urls).pipe(
           concatMap((url, index) => {
             const setup$: Rx.Observable<ScreenSetupData> = Rx.of(1).pipe(
-              takeUntil(exit$),
               mergeMap(() => {
                 // If we're moving to another page in the app, we'll want to wait for the app to tell us
                 // it's loaded the next page.
@@ -117,14 +117,19 @@ export function screenshotsObservableFactory(
                 }));
               }),
               catchError((err) => {
+                checkPageIsOpen(driver); // if browser has closed, throw a relevant error about it
+
                 logger.error(err);
                 return Rx.of({ elementsPositionAndAttributes: null, timeRange: null, error: err });
               })
             );
 
             return setup$.pipe(
+              takeUntil(exit$),
               mergeMap(
                 async (data: ScreenSetupData): Promise<ScreenshotResults> => {
+                  checkPageIsOpen(driver); // re-check that the browser has not closed
+
                   const elements = data.elementsPositionAndAttributes
                     ? data.elementsPositionAndAttributes
                     : getDefaultElementPosition(layout.getViewport(1));

x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts

@@ -129,6 +129,7 @@ export const createMockBrowserDriverFactory = async (
   mockBrowserDriver.evaluate = opts.evaluate ? opts.evaluate : defaultOpts.evaluate;
   mockBrowserDriver.screenshot = opts.screenshot ? opts.screenshot : defaultOpts.screenshot;
   mockBrowserDriver.open = opts.open ? opts.open : defaultOpts.open;
+  mockBrowserDriver.isPageOpen = () => true;
 
   mockBrowserDriverFactory.createPage = opts.getCreatePage
     ? opts.getCreatePage(mockBrowserDriver)

x-pack/plugins/translations/translations/ja-JP.json

@@ -14336,7 +14336,6 @@
     "xpack.remoteClusters.updateRemoteCluster.noRemoteClusterErrorMessage": "その名前のリモートクラスターはありません。",
     "xpack.remoteClusters.updateRemoteCluster.unknownRemoteClusterErrorMessage": "ES からレスポンスが返らず、クラスターを編集できません。",
     "xpack.reporting.breadcrumb": "レポート",
-    "xpack.reporting.browsers.chromium.chromiumClosed": "レポート生成時に Chromium の終了を検出しました。",
     "xpack.reporting.browsers.chromium.errorDetected": "レポート生成時にエラーを検出しました: {err}",
     "xpack.reporting.browsers.chromium.pageErrorDetected": "レポート生成時にページでエラーを検出しました: {err}",
     "xpack.reporting.chromiumDriver.disallowedOutgoingUrl": "無許可の着信 URL を受信しました: 「{interceptedUrl}」、終了します",