From fbb89736e390e825f669ac49def4a529695c1cf8 Mon Sep 17 00:00:00 2001 From: Yu Jin <112784385+yujin-emma@users.noreply.github.com> Date: Thu, 6 Jun 2024 11:12:53 -0700 Subject: [PATCH] [MDS] Use DataSourceError to replace Error in getDataSourceById call (#6903) * Use DataSourceError to replace Error in getDataSourceById call Signed-off-by: yujin-emma * Relocate the DataSourceError to make it can be used cross plugin folder Signed-off-by: yujin-emma * add the missing bundles Signed-off-by: yujin-emma * Fix failed integration test Signed-off-by: yujin-emma --------- Signed-off-by: yujin-emma Co-authored-by: Lu Yu Co-authored-by: ZilongX <99905560+ZilongX@users.noreply.github.com> --- .../opensearch_search_strategy.test.ts | 2 +- .../data_source/common/data_sources/error.ts | 42 +++++++++++++++++++ .../data_source/common/data_sources/types.ts | 2 + .../data_source/opensearch_dashboards.json | 2 +- src/plugins/data_source/server/index.ts | 1 + .../data_source/server/lib/error.test.ts | 3 +- src/plugins/data_source/server/lib/error.ts | 38 +---------------- src/plugins/data_source/server/types.ts | 2 +- .../public/components/utils.test.ts | 25 +++++++++-- .../public/components/utils.ts | 7 +++- .../data_source_management/public/mocks.ts | 12 +++++- .../data_source_management/public/types.ts | 1 + 12 files changed, 90 insertions(+), 47 deletions(-) create mode 100644 src/plugins/data_source/common/data_sources/error.ts diff --git a/src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts b/src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts index ae6e1746dab6..9f7cb83a3a1f 100644 --- a/src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts +++ b/src/plugins/data/server/search/opensearch_search/opensearch_search_strategy.test.ts @@ -34,7 +34,7 @@ import { pluginInitializerContextConfigMock, } from '../../../../../core/server/mocks'; import { opensearchSearchStrategyProvider } from './opensearch_search_strategy'; -import { DataSourceError } from '../../../../data_source/server/lib/error'; +import { DataSourceError } from '../../../../data_source/common/data_sources'; import { DataSourcePluginSetup } from '../../../../data_source/server'; import { SearchUsage } from '../collectors'; diff --git a/src/plugins/data_source/common/data_sources/error.ts b/src/plugins/data_source/common/data_sources/error.ts new file mode 100644 index 000000000000..a1e5b83062f5 --- /dev/null +++ b/src/plugins/data_source/common/data_sources/error.ts @@ -0,0 +1,42 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ResponseError } from '@opensearch-project/opensearch/lib/errors'; +import { OsdError } from '../../../opensearch_dashboards_utils/common'; + +export class DataSourceError extends OsdError { + // must have statusCode to avoid route handler in search.ts to return 500 + statusCode: number; + body: any; + + constructor(error: any, context?: string, statusCode?: number) { + let message: string; + if (context) { + message = context; + } else if (isResponseError(error)) { + message = JSON.stringify(error.meta.body); + } else { + message = error.message; + } + + super('Data Source Error: ' + message); + + if (error.body) { + this.body = error.body; + } + + if (statusCode) { + this.statusCode = statusCode; + } else if (error.statusCode) { + this.statusCode = error.statusCode; + } else { + this.statusCode = 400; + } + } +} + +export const isResponseError = (error: any): error is ResponseError => { + return Boolean(error.body && error.statusCode && error.headers); +}; diff --git a/src/plugins/data_source/common/data_sources/types.ts b/src/plugins/data_source/common/data_sources/types.ts index c28b82861b32..7c32264da0eb 100644 --- a/src/plugins/data_source/common/data_sources/types.ts +++ b/src/plugins/data_source/common/data_sources/types.ts @@ -50,3 +50,5 @@ export enum SigV4ServiceName { OpenSearch = 'es', OpenSearchServerless = 'aoss', } + +export { DataSourceError } from './error'; diff --git a/src/plugins/data_source/opensearch_dashboards.json b/src/plugins/data_source/opensearch_dashboards.json index 871858403cf3..56a884109364 100644 --- a/src/plugins/data_source/opensearch_dashboards.json +++ b/src/plugins/data_source/opensearch_dashboards.json @@ -4,7 +4,7 @@ "opensearchDashboardsVersion": "opensearchDashboards", "server": true, "ui": true, - "requiredPlugins": [], + "requiredPlugins": ["opensearchDashboardsUtils"], "optionalPlugins": [], "extraPublicDirs": ["common/data_sources"] } diff --git a/src/plugins/data_source/server/index.ts b/src/plugins/data_source/server/index.ts index 156b76066fbb..b0564dfcedc7 100644 --- a/src/plugins/data_source/server/index.ts +++ b/src/plugins/data_source/server/index.ts @@ -24,4 +24,5 @@ export { DataSourcePluginSetup, DataSourcePluginStart, DataSourcePluginRequestContext, + DataSourceError, } from './types'; diff --git a/src/plugins/data_source/server/lib/error.test.ts b/src/plugins/data_source/server/lib/error.test.ts index 7dec9280a450..5cd125b6f1b9 100644 --- a/src/plugins/data_source/server/lib/error.test.ts +++ b/src/plugins/data_source/server/lib/error.test.ts @@ -10,7 +10,8 @@ import { ResponseError, } from '@opensearch-project/opensearch/lib/errors'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; -import { createDataSourceError, DataSourceError } from './error'; +import { createDataSourceError } from './error'; +import { DataSourceError } from '../../common/data_sources'; import { errors as LegacyErrors } from 'elasticsearch'; const createApiResponseError = ({ diff --git a/src/plugins/data_source/server/lib/error.ts b/src/plugins/data_source/server/lib/error.ts index 4b5c44153aae..d0cb8b760622 100644 --- a/src/plugins/data_source/server/lib/error.ts +++ b/src/plugins/data_source/server/lib/error.ts @@ -3,41 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { ResponseError } from '@opensearch-project/opensearch/lib/errors'; import { errors as LegacyErrors } from 'elasticsearch'; import { SavedObjectsErrorHelpers } from '../../../../../src/core/server'; -import { OsdError } from '../../../opensearch_dashboards_utils/common'; - -export class DataSourceError extends OsdError { - // must have statusCode to avoid route handler in search.ts to return 500 - statusCode: number; - body: any; - - constructor(error: any, context?: string, statusCode?: number) { - let message: string; - if (context) { - message = context; - } else if (isResponseError(error)) { - message = JSON.stringify(error.meta.body); - } else { - message = error.message; - } - - super('Data Source Error: ' + message); - - if (error.body) { - this.body = error.body; - } - - if (statusCode) { - this.statusCode = statusCode; - } else if (error.statusCode) { - this.statusCode = error.statusCode; - } else { - this.statusCode = 400; - } - } -} +import { DataSourceError, isResponseError } from '../../common/data_sources/error'; export const createDataSourceError = (error: any, message?: string): DataSourceError => { // handle saved object client error, while retrieve data source meta info @@ -58,7 +26,3 @@ export const createDataSourceError = (error: any, message?: string): DataSourceE // handle all other error that may or may not comes with statuscode return new DataSourceError(error, message); }; - -const isResponseError = (error: any): error is ResponseError => { - return Boolean(error.body && error.statusCode && error.headers); -}; diff --git a/src/plugins/data_source/server/types.ts b/src/plugins/data_source/server/types.ts index 1f7d297c8b87..e7dd0efd5d0b 100644 --- a/src/plugins/data_source/server/types.ts +++ b/src/plugins/data_source/server/types.ts @@ -17,7 +17,7 @@ import { } from '../common/data_sources'; import { CryptographyServiceSetup } from './cryptography_service'; -import { DataSourceError } from './lib/error'; +import { DataSourceError } from '../common/data_sources'; import { IAuthenticationMethodRegistry } from './auth_registry'; import { CustomApiSchemaRegistry } from './schema_registry'; diff --git a/src/plugins/data_source_management/public/components/utils.test.ts b/src/plugins/data_source_management/public/components/utils.test.ts index 1599454e7a1f..aa73a67d3577 100644 --- a/src/plugins/data_source_management/public/components/utils.test.ts +++ b/src/plugins/data_source_management/public/components/utils.test.ts @@ -38,7 +38,8 @@ import { getSingleDataSourceResponse, getDataSource, getDataSourceOptions, - getDataSourceByIdWithError, + getDataSourceByIdWithNotFoundError, + getDataSourceByIdWithNetworkError, } from '../mocks'; import { AuthType, @@ -166,12 +167,28 @@ describe('DataSourceManagement: Utils.ts', () => { expect(e).toBeTruthy(); } }); - test('failure: gets error when response contains error', async () => { + test('failure: gets error when response contains not found error', async () => { try { - mockResponseForSavedObjectsCalls(savedObjects.client, 'get', getDataSourceByIdWithError); + mockResponseForSavedObjectsCalls( + savedObjects.client, + 'get', + getDataSourceByIdWithNotFoundError + ); await getDataSourceById('alpha-test', savedObjects.client); } catch (e) { - expect(e).toBeTruthy(); + expect(e.statusCode).toBe(404); + } + }); + test('failure: gets error when response contains other error', async () => { + try { + mockResponseForSavedObjectsCalls( + savedObjects.client, + 'get', + getDataSourceByIdWithNetworkError + ); + await getDataSourceById('alpha-test', savedObjects.client); + } catch (e) { + expect(e.statusCode).toBe(500); } }); }); diff --git a/src/plugins/data_source_management/public/components/utils.ts b/src/plugins/data_source_management/public/components/utils.ts index 1ace30bd8eae..4f2db696f950 100644 --- a/src/plugins/data_source_management/public/components/utils.ts +++ b/src/plugins/data_source_management/public/components/utils.ts @@ -37,6 +37,7 @@ import { DataSourceSelectionService, defaultDataSourceSelection, } from '../service/data_source_selection_service'; +import { DataSourceError } from '../types'; export async function getDataSources(savedObjectsClient: SavedObjectsClientContract) { return savedObjectsClient @@ -186,7 +187,11 @@ export async function getDataSourceById( const response = await savedObjectsClient.get('data-source', id); if (!response || response.error) { - throw new Error('Unable to find data source'); + const statusCode = response.error?.statusCode; + if (statusCode === 404) { + throw new DataSourceError({ statusCode, body: 'Unable to find data source' }); + } + throw new DataSourceError({ statusCode, body: response.error?.message }); } const attributes: any = response?.attributes || {}; diff --git a/src/plugins/data_source_management/public/mocks.ts b/src/plugins/data_source_management/public/mocks.ts index abc4af28dd4c..696700196d7b 100644 --- a/src/plugins/data_source_management/public/mocks.ts +++ b/src/plugins/data_source_management/public/mocks.ts @@ -331,7 +331,7 @@ export const getDataSourceByIdWithoutCredential = { references: [], }; -export const getDataSourceByIdWithError = { +export const getDataSourceByIdWithNotFoundError = { attributes: { ...getDataSourceByIdWithCredential.attributes, Error: { @@ -341,6 +341,16 @@ export const getDataSourceByIdWithError = { }, references: [], }; +export const getDataSourceByIdWithNetworkError = { + attributes: { + ...getDataSourceByIdWithCredential.attributes, + Error: { + statusCode: 500, + errorMessage: 'Internal server error', + }, + }, + references: [], +}; export const mockResponseForSavedObjectsCalls = ( savedObjectsClient: SavedObjectsClientContract, diff --git a/src/plugins/data_source_management/public/types.ts b/src/plugins/data_source_management/public/types.ts index d7c3dfe213fa..f0fa1790cd8f 100644 --- a/src/plugins/data_source_management/public/types.ts +++ b/src/plugins/data_source_management/public/types.ts @@ -139,4 +139,5 @@ export { UsernamePasswordTypedContent, SigV4Content, DataSourceAttributes, + DataSourceError, } from '../../data_source/common/data_sources';