Skip to content

Commit

Permalink
[Reporting] Move error_code to output (elastic#126044)
Browse files Browse the repository at this point in the history
* move error_code to output

* first version of api integration test for error_code

* clean up old error_code field and make exisint one optional

* use correct mapping format

* fix api integration test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and lucasfcosta committed Mar 2, 2022
1 parent 23f00c6 commit 9037839
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 7 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/reporting/common/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface ReportDocumentHead {

export interface ReportOutput extends TaskRunResult {
content: string | null;
error_code?: string;
size: number;
}

Expand Down Expand Up @@ -71,7 +72,6 @@ export interface ReportSource {
*/
jobtype: string; // refers to `ExportTypeDefinition.jobType`
created_by: string | false; // username or `false` if security is disabled. Used for ensuring users can only access the reports they've created.
error_code?: string;
payload: BasePayload;
meta: {
// for telemetry
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/reporting/server/lib/store/mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export const mapping = {
created_at: { type: 'date' },
started_at: { type: 'date' },
completed_at: { type: 'date' },
error_code: { type: 'keyword' },
attempts: { type: 'short' },
max_attempts: { type: 'short' },
kibana_name: { type: 'keyword' },
Expand All @@ -54,6 +53,7 @@ export const mapping = {
output: {
type: 'object',
properties: {
error_code: { type: 'keyword' },
chunk: { type: 'long' },
content_type: { type: 'keyword' },
size: { type: 'long' },
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/reporting/server/lib/store/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export type ReportProcessingFields = Required<{
export type ReportFailedFields = Required<{
completed_at: Report['completed_at'];
output: ReportOutput | null;
error_code: undefined | string;
}>;

export type ReportCompletedFields = Required<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ export class ExecuteReportTask implements ReportingTask {
const doc: ReportFailedFields = {
completed_at: completedTime,
output: docOutput ?? null,
error_code: error?.code,
};

return await store.setReportFailed(report, doc);
Expand All @@ -233,6 +232,7 @@ export class ExecuteReportTask implements ReportingTask {
docOutput.content = output.toString() || defaultOutput;
docOutput.content_type = unknownMime;
docOutput.warnings = [output.details ?? output.toString()];
docOutput.error_code = output.code;
}

return docOutput;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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 expect from '@kbn/expect';
import { ReportApiJSON } from '../../../plugins/reporting/common/types';
import { FtrProviderContext } from '../ftr_provider_context';

// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const reportingAPI = getService('reportingAPI');

describe('Reporting error codes', () => {
it('places error_code in report output', async () => {
await reportingAPI.initEcommerce();

const { body: reportApiJson, status } = await reportingAPI.generateCsv({
title: 'CSV Report',
browserTimezone: 'UTC',
objectType: 'search',
version: '7.15.0',
searchSource: null, // Invalid searchSource that should cause job to throw at execute phase...
} as any);
expect(status).to.be(200);

const { job: report, path: downloadPath } = reportApiJson as {
job: ReportApiJSON;
path: string;
};

// wait for the the pending job to complete
await reportingAPI.waitForJobToFinish(downloadPath, true);

expect(await reportingAPI.getJobErrorCode(report.id)).to.be('unknown_error');

await reportingAPI.teardownEcommerce();
await reportingAPI.deleteAllReports();
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
loadTestFile(require.resolve('./spaces'));
loadTestFile(require.resolve('./usage'));
loadTestFile(require.resolve('./ilm_migration_apis'));
loadTestFile(require.resolve('./error_codes'));
});
}
18 changes: 18 additions & 0 deletions x-pack/test/reporting_api_integration/services/scenarios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ export function createScenarios({ getService }: Pick<FtrProviderContext, 'getSer
password = process.env.TEST_KIBANA_PASS || 'changeme'
) => {
const jobParams = rison.encode(job as object as RisonValue);

return await supertestWithoutAuth
.post(`/api/reporting/generate/csv_searchsource`)
.auth(username, password)
Expand Down Expand Up @@ -191,6 +192,22 @@ export function createScenarios({ getService }: Pick<FtrProviderContext, 'getSer
return response.text as unknown;
};

const getJobErrorCode = async (
id: string,
username = 'elastic',
password = process.env.TEST_KIBANA_PASS || 'changeme'
): Promise<undefined | string> => {
const {
body: [job],
} = await supertestWithoutAuth
.get(`/api/reporting/jobs/list?page=0&ids=${id}`)
.auth(username, password)
.set('kbn-xsrf', 'xxx')
.send()
.expect(200);
return job?.output?.error_code;
};

const deleteAllReports = async () => {
log.debug('ReportingAPI.deleteAllReports');

Expand Down Expand Up @@ -271,5 +288,6 @@ export function createScenarios({ getService }: Pick<FtrProviderContext, 'getSer
checkIlmMigrationStatus,
migrateReportingIndices,
makeAllReportingIndicesUnmanaged,
getJobErrorCode,
};
}
7 changes: 4 additions & 3 deletions x-pack/test/reporting_api_integration/services/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function createUsageServices({ getService }: FtrProviderContext) {
const supertest = getService('supertest');

return {
async waitForJobToFinish(downloadReportPath: string) {
async waitForJobToFinish(downloadReportPath: string, ignoreFailure?: boolean) {
log.debug(`Waiting for job to finish: ${downloadReportPath}`);
const JOB_IS_PENDING_CODE = 503;

Expand All @@ -65,8 +65,9 @@ export function createUsageServices({ getService }: FtrProviderContext) {
}
}, 1500);
});

expect(statusCode).to.be(200);
if (!ignoreFailure) {
expect(statusCode).to.be(200);
}
},

/**
Expand Down

0 comments on commit 9037839

Please sign in to comment.