[MDS] Use DataSourceError to replace Error in getDataSourceById call (o…

…pensearch-project#6903) * Use DataSourceError to replace Error in getDataSourceById call Signed-off-by: yujin-emma <[email protected]> * Relocate the DataSourceError to make it can be used cross plugin folder Signed-off-by: yujin-emma <[email protected]> * add the missing bundles Signed-off-by: yujin-emma <[email protected]> * Fix failed integration test Signed-off-by: yujin-emma <[email protected]> --------- Signed-off-by: yujin-emma <[email protected]> Co-authored-by: Lu Yu <[email protected]> Co-authored-by: ZilongX <[email protected]>
yujin-emma · Jun 6, 2024 · fbb8973 · fbb8973
1 parent 9674147
commit fbb8973
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 47 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -139,4 +139,5 @@ export {
   UsernamePasswordTypedContent,
   SigV4Content,
   DataSourceAttributes,
+  DataSourceError,
 } from '../../data_source/common/data_sources';