From d9fc2262c2be27bb93b68677eb08c2c86ef3c9a8 Mon Sep 17 00:00:00 2001 From: Ash <1849116+ashokaditya@users.noreply.github.com> Date: Fri, 13 Dec 2024 21:54:49 +0100 Subject: [PATCH] [Serverless][DataUsage]Handle route response schema validation error (#204223) ## Summary - Handles issues with the API response when there are no relevant data streams to respond with and the API returns an empty array instead. This PR handles that and throws an error in that case that is shown as an error toast on the UX. - On the UX side we are setting the URL with data streams and that was failing when no data streams are present. We've added a check that verifies that data streams list is not an empty array before attempting to update the URL state. ![Screenshot 2024-12-13 at 15 33 39](https://github.com/user-attachments/assets/88db79e0-322e-4a93-ba77-0039f03959a5) ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... --------- Co-authored-by: neptunian --- .../app/components/data_usage_metrics.tsx | 2 +- .../routes/internal/data_streams.test.ts | 12 +++++++---- .../routes/internal/data_streams_handler.ts | 21 ++++++++++++++++++- .../common/data_usage/tests/data_streams.ts | 5 +---- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/x-pack/platform/plugins/private/data_usage/public/app/components/data_usage_metrics.tsx b/x-pack/platform/plugins/private/data_usage/public/app/components/data_usage_metrics.tsx index 829b198e59ab3..35ff42decef0e 100644 --- a/x-pack/platform/plugins/private/data_usage/public/app/components/data_usage_metrics.tsx +++ b/x-pack/platform/plugins/private/data_usage/public/app/components/data_usage_metrics.tsx @@ -76,7 +76,7 @@ export const DataUsageMetrics = memo( if (!metricTypesFromUrl && isFirstPageLoad) { setUrlMetricTypesFilter(metricsFilters.metricTypes.join(',')); } - if (!dataStreamsFromUrl && dataStreams && isFirstPageLoad) { + if (!dataStreamsFromUrl && !!dataStreams && dataStreams.length > 0 && isFirstPageLoad) { const hasMoreThan50 = dataStreams.length > 50; const _dataStreams = hasMoreThan50 ? dataStreams.slice(0, 50) : dataStreams; setUrlDataStreamsFilter(_dataStreams.map((ds) => ds.name).join(',')); diff --git a/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams.test.ts b/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams.test.ts index 9316a64328c9b..e7166baea59d3 100644 --- a/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams.test.ts +++ b/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams.test.ts @@ -233,14 +233,18 @@ describe('registerDataStreamsRoute', () => { // @ts-expect-error if (stats && stats.datastreams && stats.datastreams.length) { - expect(mockResponse.ok).toHaveBeenCalledTimes(1); - expect(mockResponse.ok.mock.calls[0][0]).toEqual({ - body: res, + expect(mockResponse.customError).toHaveBeenCalledTimes(1); + expect(mockResponse.customError).toHaveBeenCalledWith({ + body: new CustomHttpRequestError( + 'No relevant user defined data streams found with storage size greater than zero', + 404 + ), + statusCode: 404, }); } else { expect(mockResponse.customError).toHaveBeenCalledTimes(1); expect(mockResponse.customError).toHaveBeenCalledWith({ - body: new CustomHttpRequestError('No user defined data streams found', 404), + body: new CustomHttpRequestError('No data streams found', 404), statusCode: 404, }); } diff --git a/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams_handler.ts b/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams_handler.ts index 0f472ca98065e..ccda00a9478dd 100644 --- a/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams_handler.ts +++ b/x-pack/platform/plugins/private/data_usage/server/routes/internal/data_streams_handler.ts @@ -31,7 +31,15 @@ export const getDataStreamsHandler = ( core.elasticsearch.client.asSecondaryAuthUser ); - const nonSystemDataStreams = meteringStatsDataStreams?.filter((dataStream) => { + if (!meteringStatsDataStreams || !meteringStatsDataStreams.length) { + return errorHandler( + logger, + response, + new CustomHttpRequestError('No data streams found', 404) + ); + } + + const nonSystemDataStreams = meteringStatsDataStreams.filter((dataStream) => { return !dataStream.name?.startsWith('.'); }); @@ -55,6 +63,17 @@ export const getDataStreamsHandler = ( }, []) .sort((a, b) => b.storageSizeBytes - a.storageSizeBytes); + if (!body || !body.length) { + return errorHandler( + logger, + response, + new CustomHttpRequestError( + 'No relevant user defined data streams found with storage size greater than zero', + 404 + ) + ); + } + return response.ok({ body, }); diff --git a/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams.ts b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams.ts index b4dd8d51c331a..e591d23b125e4 100644 --- a/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams.ts +++ b/x-pack/test_serverless/api_integration/test_suites/common/data_usage/tests/data_streams.ts @@ -54,10 +54,7 @@ export default function ({ getService }: FtrProviderContext) { const res = await supertestAdminWithCookieCredentials .get(DATA_USAGE_DATA_STREAMS_API_ROUTE) .set('elastic-api-version', '1'); - const dataStreams: DataStreamsResponseBodySchemaBody = res.body; - const foundStream = dataStreams.find((stream) => stream.name === testDataStreamName); - expect(res.statusCode).to.be(200); - expect(foundStream).to.be(undefined); + expect(res.statusCode).to.be(404); }); }); }