Skip to content

Commit

Permalink
[Reporting] Capture Kibana stopped error (#127017)
Browse files Browse the repository at this point in the history
* added new error and error code

* updated error code of auth error and added a test

* added a way to observe Kibana shutdown via ReportingCore

* added a test for execute report during Kibana shutdown

* observe kibana shutdown while performing job and updated report mocks

* hook up reporting plugin to "stop" phase of reporting plugin

* rather use code from KibanaShuttingDownError

* remove done TODO

* fix jest test snapshots that are now failing
  • Loading branch information
jloleysens authored Mar 9, 2022
1 parent 06e453e commit 030816d
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 11 deletions.
16 changes: 11 additions & 5 deletions x-pack/plugins/reporting/common/errors/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,23 @@
* 2.0.
*/

import { AuthenticationExpiredError } from '.';
import * as errors from '.';

describe('ReportingError', () => {
it('provides error code when stringified', () => {
expect(new AuthenticationExpiredError() + '').toBe(
`ReportingError(code: authentication_expired)`
expect(new errors.AuthenticationExpiredError() + '').toBe(
`ReportingError(code: authentication_expired_error)`
);
});
it('provides details if there are any and error code when stringified', () => {
expect(new AuthenticationExpiredError('some details') + '').toBe(
`ReportingError(code: authentication_expired) "some details"`
expect(new errors.AuthenticationExpiredError('some details') + '').toBe(
`ReportingError(code: authentication_expired_error) "some details"`
);
});
it('has the expected code structure', () => {
const { ReportingError: _, ...nonAbstractErrors } = errors;
Object.values(nonAbstractErrors).forEach((Ctor) => {
expect(new Ctor().code).toMatch(/^[a-z_]+_error$/);
});
});
});
14 changes: 11 additions & 3 deletions x-pack/plugins/reporting/common/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@

import { i18n } from '@kbn/i18n';
export abstract class ReportingError extends Error {
/**
* A string that uniquely brands an error type. This is used to power telemetry
* about reporting failures.
*
* @note Convention for codes: lower-case, snake-case and end in `_error`.
*/
public abstract code: string;

constructor(public details?: string) {
Expand All @@ -32,7 +38,7 @@ export abstract class ReportingError extends Error {
* access token expired.
*/
export class AuthenticationExpiredError extends ReportingError {
code = 'authentication_expired';
code = 'authentication_expired_error';
}

export class QueueTimeoutError extends ReportingError {
Expand All @@ -59,7 +65,9 @@ export class PdfWorkerOutOfMemoryError extends ReportingError {
}
}

// TODO: Add ReportingError for Kibana stopping unexpectedly
// TODO: Add ReportingError for missing Chromium dependencies
export class KibanaShuttingDownError extends ReportingError {
code = 'kibana_shutting_down_error';
}

// TODO: Add ReportingError for missing Chromium dependencies
// TODO: Add ReportingError for Chromium not starting for an unknown reason
10 changes: 10 additions & 0 deletions x-pack/plugins/reporting/server/config/create_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ describe('Reporting server createConfig$', () => {

expect(result).toMatchInlineSnapshot(`
Object {
"capture": Object {
"loadDelay": 1,
"maxAttempts": 1,
"timeouts": Object {
"openUrl": 100,
"renderComplete": 100,
"waitForElements": 100,
},
"zoom": 1,
},
"csv": Object {},
"encryptionKey": "iiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiii",
"index": ".reporting",
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/reporting/server/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ export class ReportingCore {

public getContract: () => ReportingSetup;

private kibanaShuttingDown$ = new Rx.ReplaySubject<void>(1);

constructor(private logger: Logger, context: PluginInitializerContext<ReportingConfigType>) {
this.packageInfo = context.env.packageInfo;
const syncConfig = context.config.get<ReportingConfigType>();
Expand Down Expand Up @@ -129,6 +131,14 @@ export class ReportingCore {
await Promise.all([executeTask.init(taskManager), monitorTask.init(taskManager)]);
}

public pluginStop() {
this.kibanaShuttingDown$.next();
}

public getKibanaShutdown$(): Rx.Observable<void> {
return this.kibanaShuttingDown$.pipe(take(1));
}

private async assertKibanaIsAvailable(): Promise<void> {
const { status } = this.getPluginSetupDeps();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ describe('error codes', () => {
);

const { error_code: errorCode, warnings } = await generateCsv.generateData();
expect(errorCode).toBe('authentication_expired');
expect(errorCode).toBe('authentication_expired_error');
expect(warnings).toMatchInlineSnapshot(`
Array [
"This report contains partial CSV results because the authentication token expired. Export a smaller amount of data or increase the timeout of the authentication token.",
Expand Down
43 changes: 43 additions & 0 deletions x-pack/plugins/reporting/server/lib/tasks/execute_report.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { loggingSystemMock } from 'src/core/server/mocks';
import { ReportingCore } from '../..';
import { RunContext } from '../../../../task_manager/server';
import { taskManagerMock } from '../../../../task_manager/server/mocks';
import { KibanaShuttingDownError } from '../../../common/errors';
import type { SavedReport } from '../store';
import { ReportingConfigType } from '../../config';
import { createMockConfigSchema, createMockReportingCore } from '../../test_helpers';
import { ExecuteReportTask } from './';
Expand Down Expand Up @@ -79,4 +81,45 @@ describe('Execute Report Task', () => {
}
`);
});

it('throws during reporting if Kibana starts shutting down', async () => {
mockReporting.getExportTypesRegistry().register({
id: 'noop',
name: 'Noop',
createJobFnFactory: () => async () => new Promise(() => {}),
runTaskFnFactory: () => async () => new Promise(() => {}),
jobContentExtension: 'none',
jobType: 'noop',
validLicenses: [],
});
const store = await mockReporting.getStore();
store.setReportFailed = jest.fn(() => Promise.resolve({} as any));
const task = new ExecuteReportTask(mockReporting, configType, logger);
task._claimJob = jest.fn(() =>
Promise.resolve({ _id: 'test', jobtype: 'noop', status: 'pending' } as SavedReport)
);
const mockTaskManager = taskManagerMock.createStart();
await task.init(mockTaskManager);

const taskDef = task.getTaskDefinition();
const taskRunner = taskDef.createTaskRunner({
taskInstance: {
id: 'random-task-id',
params: { index: 'cool-reporting-index', id: 'noop', jobtype: 'noop', payload: {} },
},
} as unknown as RunContext);

const taskPromise = taskRunner.run();
setImmediate(() => {
mockReporting.pluginStop();
});
await taskPromise;

expect(store.setReportFailed).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
output: expect.objectContaining({ error_code: new KibanaShuttingDownError().code }),
})
);
});
});
18 changes: 16 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 @@ -20,7 +20,12 @@ import type {
TaskRunCreatorFunction,
} from '../../../../task_manager/server';
import { CancellationToken } from '../../../common/cancellation_token';
import { QueueTimeoutError, ReportingError, UnknownError } from '../../../common/errors';
import {
ReportingError,
UnknownError,
QueueTimeoutError,
KibanaShuttingDownError,
} from '../../../common/errors';
import { durationToNumber, numberToDuration } from '../../../common/schema_utils';
import type { ReportOutput } from '../../../common/types';
import type { ReportingConfigType } from '../../config';
Expand Down Expand Up @@ -288,6 +293,12 @@ export class ExecuteReportTask implements ReportingTask {
return report;
}

// Generic is used to let TS infer the return type at call site.
private async throwIfKibanaShutsDown<T>(): Promise<T> {
await this.reporting.getKibanaShutdown$().toPromise();
throw new KibanaShuttingDownError();
}

/*
* Provides a TaskRunner for Task Manager
*/
Expand Down Expand Up @@ -361,7 +372,10 @@ export class ExecuteReportTask implements ReportingTask {

eventLog.logExecutionStart();

const output = await this._performJob(task, cancellationToken, stream);
const output = await Promise.race<TaskRunResult>([
this._performJob(task, cancellationToken, stream),
this.throwIfKibanaShutsDown(),
]);

stream.end();

Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/reporting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,8 @@ export class ReportingPlugin

return reportingCore.getContract();
}

stop() {
this.reportingCore?.pluginStop();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ export const createMockConfigSchema = (
enabled: false,
...overrides.roles,
},
capture: {
maxAttempts: 1,
loadDelay: 1,
timeouts: {
openUrl: 100,
renderComplete: 100,
waitForElements: 100,
},
zoom: 1,
},
} as ReportingConfigType;
};

Expand Down

0 comments on commit 030816d

Please sign in to comment.