Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting] Capture browser errors #127135

Merged
merged 8 commits into from
Mar 10, 2022
31 changes: 28 additions & 3 deletions x-pack/plugins/reporting/common/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,37 @@ export class PdfWorkerOutOfMemoryError extends ReportingError {
'Cannot generate PDF due to low memory. Consider making a smaller PDF before retrying this report.',
});

/**
* No need to provide extra details, we know exactly what happened and can provide
* a nicely formatted message
*/
public override get message(): string {
return this.details;
}
}

export class BrowserCouldNotLaunchError extends ReportingError {
code = 'browser_could_not_launch_error';

details = i18n.translate('xpack.reporting.common.browserCouldNotLaunchErrorMessage', {
defaultMessage: 'Cannot generate screenshots because the browser did not launch.',
});

/**
* For this error message we expect that users will use the diagnostics
* functionality in reporting to debug further.
*/
public override get message() {
return this.details;
}
}

export class BrowserUnexpectedlyClosedError extends ReportingError {
code = 'browser_unexpectedly_closed_error';
}

export class BrowserScreenshotError extends ReportingError {
code = 'browser_screenshot_error';
}

// TODO: Add ReportingError for Kibana stopping unexpectedly
// TODO: Add ReportingError for missing Chromium dependencies
// TODO: Add ReportingError for missing Chromium dependencies
// TODO: Add ReportingError for Chromium not starting for an unknown reason
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { mapToReportingError } from './map_to_reporting_error';
import { errors } from '../../../screenshotting/common';
import {
UnknownError,
BrowserCouldNotLaunchError,
BrowserUnexpectedlyClosedError,
BrowserScreenshotError,
} from '.';

describe('mapToReportingError', () => {
test('Non-Error values', () => {
[null, undefined, '', 0, false, true, () => {}, {}, []].forEach((v) => {
expect(mapToReportingError(v)).toBeInstanceOf(UnknownError);
});
});

test('Screenshotting error', () => {
expect(mapToReportingError(new errors.BrowserClosedUnexpectedly())).toBeInstanceOf(
BrowserUnexpectedlyClosedError
);
expect(mapToReportingError(new errors.FailedToCaptureScreenshot())).toBeInstanceOf(
BrowserScreenshotError
);
expect(mapToReportingError(new errors.FailedToSpawnBrowserError())).toBeInstanceOf(
BrowserCouldNotLaunchError
);
});
});
30 changes: 30 additions & 0 deletions x-pack/plugins/reporting/common/errors/map_to_reporting_error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { errors } from '../../../screenshotting/common';
import {
UnknownError,
ReportingError,
BrowserCouldNotLaunchError,
BrowserUnexpectedlyClosedError,
BrowserScreenshotError,
} from '.';

export function mapToReportingError(error: unknown): ReportingError {
if (error instanceof ReportingError) {
return error;
}
switch (true) {
case error instanceof errors.BrowserClosedUnexpectedly:
return new BrowserUnexpectedlyClosedError((error as Error).message);
case error instanceof errors.FailedToCaptureScreenshot:
return new BrowserScreenshotError((error as Error).message);
case error instanceof errors.FailedToSpawnBrowserError:
return new BrowserCouldNotLaunchError();
}
return new UnknownError();
}
10 changes: 4 additions & 6 deletions x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import type {
TaskRunCreatorFunction,
} from '../../../../task_manager/server';
import { CancellationToken } from '../../../common/cancellation_token';
import { QueueTimeoutError, ReportingError, UnknownError } from '../../../common/errors';
import { ReportingError, QueueTimeoutError } from '../../../common/errors';
import { mapToReportingError } from '../../../common/errors/map_to_reporting_error';
import { durationToNumber, numberToDuration } from '../../../common/schema_utils';
import type { ReportOutput } from '../../../common/types';
import type { ReportingConfigType } from '../../config';
Expand Down Expand Up @@ -233,7 +234,7 @@ export class ExecuteReportTask implements ReportingTask {
const defaultOutput = null;
docOutput.content = output.toString() || defaultOutput;
docOutput.content_type = unknownMime;
docOutput.warnings = [output.details ?? output.toString()];
docOutput.warnings = [output.toString()];
docOutput.error_code = output.code;
}

Expand Down Expand Up @@ -418,10 +419,7 @@ export class ExecuteReportTask implements ReportingTask {
if (report == null) {
throw new Error(`Report ${jobId} is null!`);
}
const error =
failedToExecuteErr instanceof ReportingError
? failedToExecuteErr
: new UnknownError();
const error = mapToReportingError(failedToExecuteErr);
error.details =
error.details ||
`Max attempts (${attempts}) reached for job ${jobId}. Failed with: ${failedToExecuteErr.message}`;
Expand Down
14 changes: 14 additions & 0 deletions x-pack/plugins/screenshotting/common/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

/* eslint-disable max-classes-per-file */

export class FailedToSpawnBrowserError extends Error {}

export class BrowserClosedUnexpectedly extends Error {}

export class FailedToCaptureScreenshot extends Error {}
3 changes: 3 additions & 0 deletions x-pack/plugins/screenshotting/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@

export type { LayoutParams } from './layout';
export { LayoutTypes } from './layout';

import * as errors from './errors';
export { errors };
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
import type { Logger } from 'src/core/server';
import type { ScreenshotModePluginSetup } from 'src/plugins/screenshot_mode/server';
import { ConfigType } from '../../../config';
import { errors } from '../../../../common';
import { getChromiumDisconnectedError } from '../';
import { safeChildProcess } from '../../safe_child_process';
import { HeadlessChromiumDriver } from '../driver';
Expand Down Expand Up @@ -141,7 +142,6 @@ export class HeadlessChromiumDriverFactory {
logger.debug(`Chromium launch args set to: ${chromiumArgs}`);

let browser: Browser | undefined;

try {
browser = await puppeteer.launch({
pipe: !this.config.browser.chromium.inspect,
Expand All @@ -162,7 +162,9 @@ export class HeadlessChromiumDriverFactory {
},
});
} catch (err) {
observer.error(new Error(`Error spawning Chromium browser! ${err}`));
observer.error(
new errors.FailedToSpawnBrowserError(`Error spawning Chromium browser! ${err}`)
);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@
* 2.0.
*/

import { errors } from '../../../common';

export const getChromiumDisconnectedError = () =>
new Error('Browser was closed unexpectedly! Check the server logs for more info.');
new errors.BrowserClosedUnexpectedly(
'Browser was closed unexpectedly! Check the server logs for more info.'
);

export { HeadlessChromiumDriver } from './driver';
export type { Context } from './driver';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type { Transaction } from 'elastic-apm-node';
import { defer, forkJoin, throwError, Observable } from 'rxjs';
import { catchError, mergeMap, switchMapTo, timeoutWith } from 'rxjs/operators';
import type { Headers, Logger } from 'src/core/server';
import { errors } from '../../common';
import type { Context, HeadlessChromiumDriver } from '../browsers';
import { getChromiumDisconnectedError, DEFAULT_VIEWPORT } from '../browsers';
import type { Layout } from '../layouts';
Expand Down Expand Up @@ -244,7 +245,12 @@ export class ScreenshotObservableHandler {
const elements =
data.elementsPositionAndAttributes ??
getDefaultElementPosition(this.layout.getViewport(1));
const screenshots = await getScreenshots(this.driver, this.logger, elements);
let screenshots: Screenshot[] = [];
try {
screenshots = await getScreenshots(this.driver, this.logger, elements);
} catch (e) {
throw new errors.FailedToCaptureScreenshot(e.message);
}
const { timeRange, error: setupError } = data;

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function ({ getService }: FtrProviderContext) {
await retry.tryForTime(120000, async () => {
const { body } = await supertest.get(downloadPath).expect(500);
expect(body.message).to.match(
/Reporting generation failed: ReportingError\(code: unknown_error\) "/
/Reporting generation failed: ReportingError\(code: browser_unexpectedly_closed_error\) "/
);
});
});
Expand Down