Skip to content

Commit

Permalink
refactor data source error handling
Browse files Browse the repository at this point in the history
Signed-off-by: Su <szhongna@amazon.com>
  • Loading branch information
zhongnansu committed Oct 25, 2022
1 parent 25fb4b4 commit 5c57559
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 38 deletions.
6 changes: 3 additions & 3 deletions src/core/server/http/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export class Router implements IRouter {
return e;
}

if (isDataSourceConfigError(e)) {
if (isDataSourceError(e)) {
return hapiResponseAdapter.handle(
opensearchDashboardsResponseFactory.badRequest({ body: e.message })
);
Expand All @@ -312,8 +312,8 @@ export class Router implements IRouter {
}
}

const isDataSourceConfigError = (error: any) => {
return error.constructor.name === 'DataSourceConfigError' && error.statusCode === 400;
const isDataSourceError = (error: any) => {
return error.constructor.name === 'DataSourceError' && error.statusCode === 400;
};

const convertOpenSearchUnauthorized = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ import { first } from 'rxjs/operators';
import { SearchResponse } from 'elasticsearch';
import { Observable } from 'rxjs';
import { ApiResponse } from '@opensearch-project/opensearch';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { isResponseError } from '../../../../../core/server/opensearch/client/errors';
import { SharedGlobalConfig, Logger, SavedObjectsErrorHelpers } from '../../../../../core/server';
import { DataSourceConfigError } from '../../../../data_source/server';
import { createDataSourceError } from '../../../../data_source/server';
import { SharedGlobalConfig, Logger } from '../../../../../core/server';
import { SearchUsage } from '../collectors/usage';
import { toSnakeCase } from './to_snake_case';
import {
Expand Down Expand Up @@ -92,11 +90,7 @@ export const opensearchSearchStrategyProvider = (
} catch (e: any) {
if (usage) usage.trackError();
if (request.dataSourceId) {
const errorMessage = isResponseError(e) ? JSON.stringify(e.meta.body) : undefined;
throw new DataSourceConfigError(
`Failed to search against data source id [${request.dataSourceId}]: `,
SavedObjectsErrorHelpers.decorateBadRequestError(e, errorMessage)
);
throw createDataSourceError(e);
}
throw e;
}
Expand Down
10 changes: 5 additions & 5 deletions src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from '../../common/data_sources';
import { DataSourcePluginConfigType } from '../../config';
import { CryptographyServiceSetup } from '../cryptography_service';
import { DataSourceConfigError } from '../lib/error';
import { createDataSourceError, DataSourceError } from '../lib/error';
import { DataSourceClientParams } from '../types';
import { parseClientOptions } from './client_config';
import { OpenSearchClientPoolSetup } from './client_pool';
Expand All @@ -32,8 +32,8 @@ export const configureClient = async (
} catch (error: any) {
logger.error(`Failed to get data source client for dataSourceId: [${dataSourceId}]`);
logger.error(error);
// Re-throw as DataSourceConfigError
throw new DataSourceConfigError('Failed to get data source client: ', error);
// Re-throw as DataSourceError
throw createDataSourceError(error);
}
};

Expand All @@ -59,8 +59,8 @@ export const getCredential = async (
const { decryptedText, encryptionContext } = await cryptography
.decodeAndDecrypt(password)
.catch((err: any) => {
// Re-throw as DataSourceConfigError
throw new DataSourceConfigError('Unable to decrypt "auth.credentials.password".', err);
// Re-throw as DataSourceError
throw createDataSourceError(err);
});

if (encryptionContext!.endpoint !== endpoint) {
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data_source/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ export function plugin(initializerContext: PluginInitializerContext) {

export { DataSourcePluginSetup, DataSourcePluginStart } from './types';

export { DataSourceConfigError } from './lib/error';
export { DataSourceError, createDataSourceError } from './lib/error';
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { CryptographyServiceSetup } from '../cryptography_service';
import { DataSourceClientParams, LegacyClientCallAPIParams } from '../types';
import { OpenSearchClientPoolSetup, getCredential, getDataSource } from '../client';
import { parseClientOptions } from './client_config';
import { DataSourceConfigError } from '../lib/error';
import { createDataSourceError, DataSourceError } from '../lib/error';

export const configureLegacyClient = async (
{ dataSourceId, savedObjects, cryptography }: DataSourceClientParams,
Expand All @@ -40,8 +40,8 @@ export const configureLegacyClient = async (
} catch (error: any) {
logger.error(`Failed to get data source client for dataSourceId: [${dataSourceId}]`);
logger.error(error);
// Re-throw as DataSourceConfigError
throw new DataSourceConfigError('Failed to get data source client: ', error);
// Re-throw as DataSourceError
throw createDataSourceError(error);
}
};

Expand Down Expand Up @@ -140,7 +140,6 @@ const callAPI = async (
if (!options.wrap401Errors || err.statusCode !== 401) {
throw err;
}

throw LegacyOpenSearchErrorHelpers.decorateNotAuthorizedError(err);
}
};
Expand Down
12 changes: 6 additions & 6 deletions src/plugins/data_source/server/lib/error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,35 @@
*/

import { SavedObjectsErrorHelpers } from '../../../../../src/core/server';
import { DataSourceConfigError } from './error';
import { DataSourceError } from './error';

describe('DataSourceConfigError', () => {
describe('DataSourceError', () => {
it('create from savedObject bad request error should be 400 error', () => {
const error = SavedObjectsErrorHelpers.createBadRequestError('test reason message');
expect(new DataSourceConfigError('test prefix: ', error)).toMatchObject({
expect(new DataSourceError('test prefix: ', error)).toMatchObject({
statusCode: 400,
message: 'test prefix: test reason message: Bad Request',
});
});

it('create from savedObject not found error should be 400 error', () => {
const error = SavedObjectsErrorHelpers.decorateNotAuthorizedError(new Error());
expect(new DataSourceConfigError('test prefix: ', error)).toHaveProperty('statusCode', 400);
expect(new DataSourceError('test prefix: ', error)).toHaveProperty('statusCode', 400);
});

it('create from savedObject service unavailable error should be a 500 error', () => {
const error = SavedObjectsErrorHelpers.decorateOpenSearchUnavailableError(
new Error('test reason message')
);
expect(new DataSourceConfigError('test prefix: ', error)).toMatchObject({
expect(new DataSourceError('test prefix: ', error)).toMatchObject({
statusCode: 500,
message: 'test prefix: test reason message',
});
});

it('create from non savedObject error should always be a 400 error', () => {
const error = new Error('test reason message');
expect(new DataSourceConfigError('test prefix: ', error)).toMatchObject({
expect(new DataSourceError('test prefix: ', error)).toMatchObject({
statusCode: 400,
message: 'test prefix: test reason message',
});
Expand Down
57 changes: 47 additions & 10 deletions src/plugins/data_source/server/lib/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,56 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { ResponseError } from '@opensearch-project/opensearch/lib/errors';
import { errors } from 'elasticsearch';
import { SavedObjectsErrorHelpers } from '../../../../../src/core/server';
import { OsdError } from '../../../opensearch_dashboards_utils/common';

export class DataSourceConfigError extends OsdError {
export class DataSourceError extends OsdError {
// must have statusCode to avoid route handler in search.ts to return 500
statusCode: number;
constructor(messagePrefix: string, error: any) {
const messageContent = SavedObjectsErrorHelpers.isSavedObjectsClientError(error)
? error.output.payload.message
: error.message;
super(messagePrefix + messageContent);
// Cast all 5xx error returned by saveObjectClient to 500.
// Cast both savedObject client 4xx errors, and other errors to 400
this.statusCode = SavedObjectsErrorHelpers.isOpenSearchUnavailableError(error) ? 500 : 400;
constructor(error: any, message?: string, statusCode?: number) {
message = message ? message : error.message;
super('Data Source Error: ' + message);
if (statusCode) {
this.statusCode = statusCode;
} else if (error.statusCode) {
this.statusCode = error.statusCode;
} else {
this.statusCode = 400;
}
}
}

export const createDataSourceError = (error: any, message?: string) => {
// handle saved object client error, while retrieve data source meta info
if (SavedObjectsErrorHelpers.isSavedObjectsClientError(error)) {
const statusCode = SavedObjectsErrorHelpers.isOpenSearchUnavailableError(error)
? 500
: error.output.statusCode;
return new DataSourceError(error, error.output.payload.message, statusCode);
}

// cast OpenSearch client 500 response error to 400
// cast OpenSearch client 401 response error to 400, due to https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2591
if (
(isResponseError(error) && error.statusCode === 500) ||
(isResponseError(error) && error.statusCode === 401)
) {
return new DataSourceError(error, JSON.stringify(error.meta.body), 400);
}

// cast legacy client 500, 401 response error to 400
if (
error instanceof errors.AuthenticationException ||
error instanceof errors.InternalServerError
) {
return new DataSourceError(error, error.message, 400);
}

// 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);
};

0 comments on commit 5c57559

Please sign in to comment.