Skip to content

Commit

Permalink
[Reporting] Give users better information when pdf fails (#126171)
Browse files Browse the repository at this point in the history
* raise pdf worker out memory error to common errors

* not having a PDF buffer for a PDF report should fail the report

* remove unused import

* make it possible to format Reporting errors nicely

* make pdf v1 behave the same way as v2 regarding not compiling a PDF

* remove unused import

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jloleysens and kibanamachine authored Feb 24, 2022
1 parent afa9557 commit a8fb874
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 36 deletions.
14 changes: 14 additions & 0 deletions x-pack/plugins/reporting/common/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

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

import { i18n } from '@kbn/i18n';
export abstract class ReportingError extends Error {
public abstract code: string;

Expand Down Expand Up @@ -45,6 +46,19 @@ export class UnknownError extends ReportingError {
code = 'unknown_error';
}

export class PdfWorkerOutOfMemoryError extends ReportingError {
code = 'pdf_worker_out_of_memory_error';

details = i18n.translate('xpack.reporting.common.pdfWorkerOutOfMemoryErrorMessage', {
defaultMessage:
'Cannot generate PDF due to low memory. Consider making a smaller PDF before retrying this report.',
});

public override get message(): string {
return this.details;
}
}

// TODO: Add ReportingError for Kibana stopping unexpectedly
// TODO: Add ReportingError for missing Chromium dependencies
// TODO: Add ReportingError for missing Chromium dependencies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@
*/

export { PdfMaker } from './pdfmaker';
export { PdfWorkerOutOfMemoryError } from './pdfmaker_errors';
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
import path from 'path';
import { isUint8Array } from 'util/types';
import { createMockLayout } from '../../../../../../screenshotting/server/layouts/mock';
import { PdfWorkerOutOfMemoryError } from '../../../../../common/errors';
import { PdfMaker } from '../';
import { PdfWorkerOutOfMemoryError } from '../pdfmaker_errors';

const imageBase64 = Buffer.from(
`iVBORw0KGgoAAAANSUhEUgAAAOEAAADhCAMAAAAJbSJIAAAAGFBMVEXy8vJpaWn7+/vY2Nj39/cAAACcnJzx8fFvt0oZAAAAi0lEQVR4nO3SSQoDIBBFwR7U3P/GQXKEIIJULXr9H3TMrHhX5Yysvj3jjM8+XRnVa9wec8QuHKv3h74Z+PNyGwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/xu3Bxy026rXu4ljdUVW395xUFfGzLo946DK+QW+bgCTFcecSAAAAABJRU5ErkJggg==`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import path from 'path';
import { Content, ContentImage, ContentText } from 'pdfmake/interfaces';
import { MessageChannel, MessagePort, Worker } from 'worker_threads';
import type { Layout } from '../../../../../screenshotting/server';
import { PdfWorkerOutOfMemoryError } from '../../../../common/errors';
import {
headingHeight,
pageMarginBottom,
Expand All @@ -20,7 +21,6 @@ import {
} from './constants';
import { REPORTING_TABLE_LAYOUT } from './get_doc_options';
import { getFont } from './get_font';
import { PdfWorkerOutOfMemoryError } from './pdfmaker_errors';
import type { GeneratePdfRequest, GeneratePdfResponse, WorkerData } from './worker';

// Ensure that all dependencies are included in the release bundle.
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type { PdfMetrics } from '../../../../common/types';
import { ReportingCore } from '../../../';
import { LevelLogger } from '../../../lib';
import { ScreenshotOptions } from '../../../types';
import { PdfMaker, PdfWorkerOutOfMemoryError } from '../../common/pdf';
import { PdfMaker } from '../../common/pdf';
import { getTracker } from './tracker';

const getTimeRange = (urlScreenshots: ScreenshotResult['results']) => {
Expand Down Expand Up @@ -96,14 +96,7 @@ export function generatePdfObservable(
tracker.setByteLength(byteLength);
} catch (err) {
logger.error(`Could not generate the PDF buffer!`);
logger.error(err);
if (err instanceof PdfWorkerOutOfMemoryError) {
warnings.push(
'Failed to generate PDF due to low memory. Please consider generating a smaller PDF.'
);
} else {
warnings.push(`Failed to generate PDF due to the following error: ${err.message}`);
}
throw err;
}

tracker.end();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { LocatorParams, PdfMetrics, UrlOrUrlLocatorTuple } from '../../../../com
import { LevelLogger } from '../../../lib';
import { ScreenshotOptions } from '../../../types';
import { PdfMaker } from '../../common/pdf';
import { PdfWorkerOutOfMemoryError } from '../../common/pdf';
import { getFullRedirectAppUrl } from '../../common/v2/get_full_redirect_app_url';
import type { TaskPayloadPDFV2 } from '../types';
import { getTracker } from './tracker';
Expand Down Expand Up @@ -109,14 +108,7 @@ export function generatePdfObservable(
tracker.end();
} catch (err) {
logger.error(`Could not generate the PDF buffer!`);
logger.error(err);
if (err instanceof PdfWorkerOutOfMemoryError) {
warnings.push(
'Failed to generate PDF due to low memory. Please consider generating a smaller PDF.'
);
} else {
warnings.push(`Failed to generate PDF due to the following error: ${err.message}`);
}
throw err;
}

return {
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/reporting/server/lib/tasks/execute_report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,13 @@ export class ExecuteReportTask implements ReportingTask {
if (report == null) {
throw new Error(`Report ${jobId} is null!`);
}
const maxAttemptsMsg = `Max attempts (${attempts}) reached for job ${jobId}. Failed with: ${failedToExecuteErr.message}`;
const error =
failedToExecuteErr instanceof ReportingError
? failedToExecuteErr
: new UnknownError();
error.details = maxAttemptsMsg;
error.details =
error.details ||
`Max attempts (${attempts}) reached for job ${jobId}. Failed with: ${failedToExecuteErr.message}`;
const resp = await this._failJob(report, error);
report._seq_no = resp._seq_no;
report._primary_term = resp._primary_term;
Expand Down

0 comments on commit a8fb874

Please sign in to comment.