Skip to content

Commit

Permalink
[Screenshotting] Ensure worker.js content contents are compiled in bu…
Browse files Browse the repository at this point in the history
…ild (#129796)

* [Screenshotting] Expose packageInfo to pdf maker for dist data

* somehow use `dist` in the pdfmaker?

* use worker_src_harness exclusively when running from source

* Update pdfmaker.ts

* Update x-pack/plugins/screenshotting/server/formats/pdf/pdf_maker/pdfmaker.ts

Co-authored-by: Michael Dokolin <[email protected]>

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

Co-authored-by: Michael Dokolin <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
3 people authored Apr 11, 2022
1 parent 837813e commit 82e7356
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 14 deletions.
4 changes: 3 additions & 1 deletion x-pack/plugins/screenshotting/server/formats/pdf/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { groupBy } from 'lodash';
import type { Values } from '@kbn/utility-types';
import type { Logger } from 'src/core/server';
import type { Logger, PackageInfo } from 'src/core/server';
import type { LayoutParams } from '../../../common';
import { LayoutTypes } from '../../../common';
import type { Layout } from '../../layouts';
Expand Down Expand Up @@ -93,6 +93,7 @@ function getTimeRange(results: CaptureResult['results']) {

export async function toPdf(
logger: Logger,
packageInfo: PackageInfo,
layout: Layout,
{ logo, title }: PdfScreenshotOptions,
{ metrics, results }: CaptureResult
Expand All @@ -104,6 +105,7 @@ export async function toPdf(
results,
layout,
logo,
packageInfo,
logger,
});

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

import type { Logger } from 'src/core/server';
import type { Logger, PackageInfo } from 'src/core/server';
import { PdfMaker } from './pdfmaker';
import type { Layout } from '../../../layouts';
import { getTracker } from './tracker';
Expand All @@ -14,6 +14,7 @@ import type { CaptureResult } from '../../../screenshots';
interface PngsToPdfArgs {
results: CaptureResult['results'];
layout: Layout;
packageInfo: PackageInfo;
logger: Logger;
logo?: string;
title?: string;
Expand All @@ -24,9 +25,10 @@ export async function pngsToPdf({
layout,
logo,
title,
packageInfo,
logger,
}: PngsToPdfArgs): Promise<{ buffer: Buffer; pages: number }> {
const pdfMaker = new PdfMaker(layout, logo, logger);
const pdfMaker = new PdfMaker(layout, logo, packageInfo, logger);
const tracker = getTracker();
if (title) {
pdfMaker.setTitle(title);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@

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

import { PackageInfo } from 'kibana/server';
import path from 'path';
import { loggingSystemMock } from 'src/core/server/mocks';
import { isUint8Array } from 'util/types';
import { createMockLayout } from '../../../../layouts/mock';
import { errors } from '../../../../../common';
import { createMockLayout } from '../../../../layouts/mock';
import { PdfMaker } from '../pdfmaker';

const imageBase64 = Buffer.from(
Expand All @@ -23,11 +24,19 @@ describe('PdfMaker', () => {
let layout: ReturnType<typeof createMockLayout>;
let pdf: PdfMaker;
let logger: ReturnType<typeof loggingSystemMock.createLogger>;
let packageInfo: Readonly<PackageInfo>;

beforeEach(() => {
layout = createMockLayout();
logger = loggingSystemMock.createLogger();
pdf = new PdfMaker(layout, undefined, logger);
packageInfo = {
branch: 'screenshot-test',
buildNum: 567891011,
buildSha: 'screenshot-dfdfed0a',
dist: false,
version: '1000.0.0',
};
pdf = new PdfMaker(layout, undefined, packageInfo, logger);
});

describe('generate', () => {
Expand Down Expand Up @@ -56,14 +65,14 @@ describe('PdfMaker', () => {
protected workerMaxOldHeapSizeMb = 2;
protected workerMaxYoungHeapSizeMb = 2;
protected workerModulePath = path.resolve(__dirname, './memory_leak_worker.js');
})(layout, undefined, logger);
})(layout, undefined, packageInfo, logger);
await expect(leakyMaker.generate()).rejects.toBeInstanceOf(errors.PdfWorkerOutOfMemoryError);
});

it('restarts the PDF worker if it crashes', async () => {
const buggyMaker = new (class BuggyPdfMaker extends PdfMaker {
protected workerModulePath = path.resolve(__dirname, './buggy_worker.js');
})(layout, undefined, logger);
})(layout, undefined, packageInfo, logger);

await expect(buggyMaker.generate()).rejects.toThrowError(new Error('This is a bug'));
await expect(buggyMaker.generate()).rejects.toThrowError(new Error('This is a bug'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { Logger } from 'src/core/server';
import type { Logger, PackageInfo } from 'src/core/server';
import { SerializableRecord } from '@kbn/utility-types';
import path from 'path';
import { Content, ContentImage, ContentText } from 'pdfmake/interfaces';
Expand Down Expand Up @@ -34,7 +34,7 @@ export class PdfMaker {
private worker?: Worker;
private pageCount: number = 0;

protected workerModulePath = path.resolve(__dirname, './worker.js');
protected workerModulePath: string;

/**
* The maximum heap size for old memory region of the worker thread.
Expand Down Expand Up @@ -65,10 +65,18 @@ export class PdfMaker {
constructor(
private readonly layout: Layout,
private readonly logo: string | undefined,
{ dist }: PackageInfo,
private readonly logger: Logger
) {
this.title = '';
this.content = [];

// running in dist: `worker.ts` becomes `worker.js`
// running in source: `worker_src_harness.ts` needs to be wrapped in JS and have a ts-node environment initialized.
this.workerModulePath = path.resolve(
__dirname,
dist ? './worker.js' : './worker_src_harness.js'
);
}

_addContents(contents: Content[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@
* 2.0.
*/

/**
* This file is the harness for importing worker.ts with Kibana running in dev mode.
* The TS file needs to be compiled on the fly, unlike when Kibana is running as a dist.
*/

require('../../../../../../../src/setup_node_env');
require('./worker.ts');
5 changes: 4 additions & 1 deletion x-pack/plugins/screenshotting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
CoreSetup,
CoreStart,
Logger,
PackageInfo,
Plugin,
PluginInitializerContext,
} from 'src/core/server';
Expand Down Expand Up @@ -45,13 +46,15 @@ export interface ScreenshottingStart {
export class ScreenshottingPlugin implements Plugin<void, ScreenshottingStart, SetupDeps> {
private config: ConfigType;
private logger: Logger;
private packageInfo: PackageInfo;
private screenshotMode!: ScreenshotModePluginSetup;
private browserDriverFactory!: Promise<HeadlessChromiumDriverFactory>;
private screenshots!: Promise<Screenshots>;

constructor(context: PluginInitializerContext<ConfigType>) {
this.logger = context.logger.get();
this.config = context.config.get();
this.packageInfo = context.env.packageInfo;
}

setup({ http }: CoreSetup, { screenshotMode }: SetupDeps) {
Expand Down Expand Up @@ -82,7 +85,7 @@ export class ScreenshottingPlugin implements Plugin<void, ScreenshottingStart, S
const browserDriverFactory = await this.browserDriverFactory;
const logger = this.logger.get('screenshot');

return new Screenshots(browserDriverFactory, logger, http, this.config);
return new Screenshots(browserDriverFactory, logger, this.packageInfo, http, this.config);
})();
// Already handled in `browserDriverFactory`
this.screenshots.catch(() => {});
Expand Down
14 changes: 12 additions & 2 deletions x-pack/plugins/screenshotting/server/screenshots/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { of, throwError } from 'rxjs';
import type { Logger } from 'src/core/server';
import type { Logger, PackageInfo } from 'src/core/server';
import { httpServiceMock } from 'src/core/server/mocks';
import {
SCREENSHOTTING_APP_ID,
Expand All @@ -31,6 +31,7 @@ describe('Screenshot Observable Pipeline', () => {
let http: ReturnType<typeof httpServiceMock.createSetupContract>;
let layout: ReturnType<typeof createMockLayout>;
let logger: jest.Mocked<Logger>;
let packageInfo: Readonly<PackageInfo>;
let options: ScreenshotOptions;
let screenshots: Screenshots;

Expand All @@ -44,6 +45,13 @@ describe('Screenshot Observable Pipeline', () => {
error: jest.fn(),
info: jest.fn(),
} as unknown as jest.Mocked<Logger>;
packageInfo = {
branch: 'screenshot-test',
buildNum: 567891011,
buildSha: 'screenshot-dfdfed0a',
dist: false,
version: '5000.0.0',
};
options = {
browserTimezone: 'UTC',
headers: {},
Expand All @@ -56,7 +64,9 @@ describe('Screenshot Observable Pipeline', () => {
},
urls: ['/welcome/home/start/index.htm'],
} as unknown as typeof options;
screenshots = new Screenshots(driverFactory, logger, http, { poolSize: 1 } as ConfigType);
screenshots = new Screenshots(driverFactory, logger, packageInfo, http, {
poolSize: 1,
} as ConfigType);

jest.spyOn(Layouts, 'createLayout').mockReturnValue(layout);

Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/screenshotting/server/screenshots/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
tap,
toArray,
} from 'rxjs/operators';
import type { HttpServiceSetup, KibanaRequest, Logger } from 'src/core/server';
import type { HttpServiceSetup, KibanaRequest, Logger, PackageInfo } from 'src/core/server';
import type { ExpressionAstExpression } from 'src/plugins/expressions/common';
import {
LayoutParams,
Expand Down Expand Up @@ -99,6 +99,7 @@ export class Screenshots {
constructor(
private readonly browserDriverFactory: HeadlessChromiumDriverFactory,
private readonly logger: Logger,
private readonly packageInfo: PackageInfo,
private readonly http: HttpServiceSetup,
{ poolSize }: ConfigType
) {
Expand Down Expand Up @@ -214,7 +215,7 @@ export class Screenshots {
mergeMap((result) => {
switch (options.format) {
case 'pdf':
return toPdf(this.logger, layout, options, result);
return toPdf(this.logger, this.packageInfo, layout, options, result);
default:
return toPng(result);
}
Expand Down

0 comments on commit 82e7356

Please sign in to comment.