[Reporting] Change response behavior for internal Download API (#189588)

## Summary Closes #183846 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Test the UI in Stack Management > Reporting for jobs that are pending and failed. * To cause a report job to fail, restart the Kibana server during report execution ### Release Note Changed internal APIs for generating reports to return 200 response for pending report jobs and failed report jobs. If this impacts your workflow, please switch to using public APIs for generating reports: https://www.elastic.co/guide/en/kibana/current/automating-report-generation.html#use-a-script
elastic · Aug 7, 2024 · 35872af · 35872af
1 parent 5f49cc5
commit 35872af
Show file tree

Hide file tree

Showing 11 changed files with 282 additions and 155 deletions.
diff --git a/x-pack/plugins/reporting/server/routes/common/jobs/constants.ts b/x-pack/plugins/reporting/server/routes/common/jobs/constants.ts
@@ -0,0 +1,18 @@
+/*
+ * 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.
+ */
+
+export const STATUS_CODES = {
+  COMPLETED: 200,
+  PENDING: {
+    INTERNAL: 202,
+    PUBLIC: 503,
+  },
+  FAILED: {
+    INTERNAL: 202,
+    PUBLIC: 500,
+  },
+};
diff --git a/x-pack/plugins/reporting/server/routes/common/jobs/get_document_payload.test.ts b/x-pack/plugins/reporting/server/routes/common/jobs/get_document_payload.test.ts
@@ -13,22 +13,25 @@ import { CSV_JOB_TYPE } from '@kbn/reporting-export-types-csv-common';
 import { PDF_JOB_TYPE, PDF_JOB_TYPE_V2 } from '@kbn/reporting-export-types-pdf-common';
 import { createMockConfigSchema } from '@kbn/reporting-mocks-server';
 
+import { ReportingCore } from '../../..';
 import { ContentStream, getContentStream } from '../../../lib';
 import { createMockReportingCore } from '../../../test_helpers';
+import { STATUS_CODES } from './constants';
 import { getDocumentPayloadFactory } from './get_document_payload';
 import { jobsQueryFactory } from './jobs_query';
 
 jest.mock('../../../lib/content_stream');
 jest.mock('./jobs_query');
 
 describe('getDocumentPayload', () => {
+  let core: ReportingCore;
   let getDocumentPayload: ReturnType<typeof getDocumentPayloadFactory>;
 
   beforeEach(async () => {
     const schema = createMockConfigSchema();
-    const core = await createMockReportingCore(schema);
+    core = await createMockReportingCore(schema);
 
-    getDocumentPayload = getDocumentPayloadFactory(core);
+    getDocumentPayload = getDocumentPayloadFactory(core, { isInternal: false });
 
     (getContentStream as jest.MockedFunction<typeof getContentStream>).mockResolvedValue(
       new Readable({
@@ -66,7 +69,7 @@ describe('getDocumentPayload', () => {
           headers: expect.objectContaining({
             'Content-Length': '1024',
           }),
-          statusCode: 200,
+          statusCode: STATUS_CODES.COMPLETED,
         })
       );
     });
@@ -96,57 +99,115 @@ describe('getDocumentPayload', () => {
             'kbn-csv-contains-formulas': true,
             'kbn-max-size-reached': true,
           }),
-          statusCode: 200,
+          statusCode: STATUS_CODES.COMPLETED,
         })
       );
     });
   });
 
-  describe('when the report is failed', () => {
-    it('should return payload for the failed report', async () => {
-      await expect(
-        getDocumentPayload({
-          id: 'id1',
-          index: '.reporting-12345',
-          status: JOB_STATUS.FAILED,
-          jobtype: PDF_JOB_TYPE_V2,
-          output: {},
-          payload: {},
-        } as ReportApiJSON)
-      ).resolves.toEqual(
-        expect.objectContaining({
-          contentType: 'application/json',
-          content: {
-            message: expect.stringContaining('Some error'),
-          },
-          headers: {},
-          statusCode: 500,
-        })
-      );
+  describe('public API behavior', () => {
+    beforeEach(() => {
+      getDocumentPayload = getDocumentPayloadFactory(core, { isInternal: false });
+    });
+
+    describe('when the report is failed', () => {
+      it('should return payload for the failed report', async () => {
+        await expect(
+          getDocumentPayload({
+            id: 'id1',
+            index: '.reporting-12345',
+            status: JOB_STATUS.FAILED,
+            jobtype: PDF_JOB_TYPE_V2,
+            output: {},
+            payload: {},
+          } as ReportApiJSON)
+        ).resolves.toEqual(
+          expect.objectContaining({
+            contentType: 'application/json',
+            content: {
+              message: expect.stringContaining('Some error'),
+            },
+            headers: {},
+            statusCode: STATUS_CODES.FAILED.PUBLIC,
+          })
+        );
+      });
+    });
+
+    describe('when the report is incomplete', () => {
+      it('should return payload for the pending report', async () => {
+        await expect(
+          getDocumentPayload({
+            id: 'id1',
+            index: '.reporting-12345',
+            status: JOB_STATUS.PENDING,
+            jobtype: PDF_JOB_TYPE_V2,
+            output: {},
+            payload: {},
+          } as ReportApiJSON)
+        ).resolves.toEqual(
+          expect.objectContaining({
+            contentType: 'text/plain',
+            content: 'pending',
+            headers: {
+              'retry-after': '30',
+            },
+            statusCode: STATUS_CODES.PENDING.PUBLIC,
+          })
+        );
+      });
     });
   });
 
-  describe('when the report is incomplete', () => {
-    it('should return payload for the pending report', async () => {
-      await expect(
-        getDocumentPayload({
-          id: 'id1',
-          index: '.reporting-12345',
-          status: JOB_STATUS.PENDING,
-          jobtype: PDF_JOB_TYPE_V2,
-          output: {},
-          payload: {},
-        } as ReportApiJSON)
-      ).resolves.toEqual(
-        expect.objectContaining({
-          contentType: 'text/plain',
-          content: 'pending',
-          headers: {
-            'retry-after': '30',
-          },
-          statusCode: 503,
-        })
-      );
+  describe('internal API behavior', () => {
+    beforeEach(() => {
+      getDocumentPayload = getDocumentPayloadFactory(core, { isInternal: true });
+    });
+
+    describe('when the report is failed', () => {
+      it('should return payload for the failed report', async () => {
+        await expect(
+          getDocumentPayload({
+            id: 'id1',
+            index: '.reporting-12345',
+            status: JOB_STATUS.FAILED,
+            jobtype: PDF_JOB_TYPE_V2,
+            output: {},
+            payload: {},
+          } as ReportApiJSON)
+        ).resolves.toEqual(
+          expect.objectContaining({
+            contentType: 'application/json',
+            content: {
+              message: expect.stringContaining('Some error'),
+            },
+            headers: {},
+            statusCode: STATUS_CODES.FAILED.INTERNAL,
+          })
+        );
+      });
+    });
+
+    describe('when the report is incomplete', () => {
+      it('should return payload for the pending report', async () => {
+        await expect(
+          getDocumentPayload({
+            id: 'id1',
+            index: '.reporting-12345',
+            status: JOB_STATUS.PENDING,
+            jobtype: PDF_JOB_TYPE_V2,
+            output: {},
+            payload: {},
+          } as ReportApiJSON)
+        ).resolves.toEqual(
+          expect.objectContaining({
+            contentType: 'text/plain',
+            content: 'pending',
+            headers: { 'retry-after': '30' },
+            statusCode: STATUS_CODES.PENDING.INTERNAL,
+          })
+        );
+      });
     });
   });
 });
diff --git a/x-pack/plugins/reporting/server/routes/common/jobs/get_document_payload.ts b/x-pack/plugins/reporting/server/routes/common/jobs/get_document_payload.ts
@@ -8,13 +8,14 @@
 import { Stream } from 'stream';
 
 import { ResponseHeaders } from '@kbn/core-http-server';
+import { JOB_STATUS } from '@kbn/reporting-common';
 import { ReportApiJSON } from '@kbn/reporting-common/types';
 import { CSV_JOB_TYPE, CSV_JOB_TYPE_DEPRECATED } from '@kbn/reporting-export-types-csv-common';
-import { JOB_STATUS } from '@kbn/reporting-common';
 import { ExportType } from '@kbn/reporting-server';
 
 import { ReportingCore } from '../../..';
 import { getContentStream } from '../../../lib';
+import { STATUS_CODES } from './constants';
 import { jobsQueryFactory } from './jobs_query';
 
 export interface ErrorFromPayload {
@@ -53,7 +54,10 @@ const getReportingHeaders = (output: TaskRunResult, exportType: ExportType) => {
   return metaDataHeaders;
 };
 
-export function getDocumentPayloadFactory(reporting: ReportingCore) {
+export function getDocumentPayloadFactory(
+  reporting: ReportingCore,
+  { isInternal }: { isInternal: boolean }
+) {
   const { logger: _logger } = reporting.getPluginSetupDeps();
   const logger = _logger.get('download-report');
   const exportTypesRegistry = reporting.getExportTypesRegistry();
@@ -75,7 +79,7 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
     return {
       filename,
       content,
-      statusCode: 200,
+      statusCode: STATUS_CODES.COMPLETED,
       contentType,
       headers: {
         ...headers,
@@ -84,29 +88,29 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
     };
   }
 
-  // @TODO: These should be semantic HTTP codes as 500/503's indicate
-  // error then these are really operating properly.
   async function getFailure({ id }: ReportApiJSON): Promise<Payload> {
-    const jobsQuery = jobsQueryFactory(reporting);
+    const jobsQuery = jobsQueryFactory(reporting, { isInternal });
     const error = await jobsQuery.getError(id);
 
-    logger.debug(`Report job ${id} has failed. Sending statusCode: 500`);
+    // For download requested over public API, status code for failed job must be 500 to integrate with Watcher
+    const statusCode = isInternal ? STATUS_CODES.FAILED.INTERNAL : STATUS_CODES.FAILED.PUBLIC;
+    logger.debug(`Report job ${id} has failed. Sending statusCode: ${statusCode}`);
 
     return {
-      statusCode: 500,
-      content: {
-        message: `Reporting generation failed: ${error}`,
-      },
+      statusCode,
+      content: { message: `Reporting generation failed: ${error}` },
       contentType: 'application/json',
       headers: {},
     };
   }
 
   function getIncomplete({ id, status }: ReportApiJSON): Payload {
-    logger.debug(`Report job ${id} is processing. Sending statusCode: 503`);
+    // For download requested over public API, status code for processing/pending job must be 503 to integrate with Watcher
+    const statusCode = isInternal ? STATUS_CODES.PENDING.INTERNAL : STATUS_CODES.PENDING.PUBLIC;
+    logger.debug(`Report job ${id} is processing. Sending statusCode: ${statusCode}`);
 
     return {
-      statusCode: 503,
+      statusCode,
       content: status,
       contentType: 'text/plain',
       headers: { 'retry-after': '30' },
@@ -124,7 +128,6 @@ export function getDocumentPayloadFactory(reporting: ReportingCore) {
       }
     }
 
-    // send a 503 indicating that the report isn't completed yet
     return getIncomplete(report);
   };
 }