Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MD] Wrap datasource errors for route handlers to understand #2236

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/core/server/http/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,23 @@ export class Router implements IRouter {
if (LegacyOpenSearchErrorHelpers.isNotAuthorizedError(e)) {
return e;
}

if (isDataSourceConfigError(e)) {
return hapiResponseAdapter.handle(
opensearchDashboardsResponseFactory.badRequest({ body: e.message })
);
}
// TODO: add legacy data source client config error handling

return hapiResponseAdapter.toInternalError();
}
}
}

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

const convertOpenSearchUnauthorized = (
e: OpenSearchNotAuthorizedError
): ErrorHttpResponseOptions => {
Expand Down
56 changes: 24 additions & 32 deletions src/plugins/data_source/server/client/configure_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,16 @@
*/

import { Client } from '@opensearch-project/opensearch';
import {
Logger,
SavedObject,
SavedObjectsClientContract,
SavedObjectsErrorHelpers,
} from '../../../../../src/core/server';
import { Logger, SavedObject, SavedObjectsClientContract } from '../../../../../src/core/server';
import { DATA_SOURCE_SAVED_OBJECT_TYPE } from '../../common';

import {
AuthType,
DataSourceAttributes,
UsernamePasswordTypedContent,
} from '../../common/data_sources';
import { DataSourcePluginConfigType } from '../../config';
import { CryptographyClient } from '../cryptography';
import { DataSourceConfigError } from '../lib/error';
import { parseClientOptions } from './client_config';
import { OpenSearchClientPoolSetup } from './client_pool';

Expand All @@ -30,45 +25,42 @@ export const configureClient = async (
config: DataSourcePluginConfigType,
logger: Logger
): Promise<Client> => {
const dataSource = await getDataSource(dataSourceId, savedObjects);
const rootClient = getRootClient(dataSource.attributes, config, openSearchClientPoolSetup);
try {
const dataSource = await getDataSource(dataSourceId, savedObjects);
const rootClient = getRootClient(dataSource.attributes, config, openSearchClientPoolSetup);

return getQueryClient(rootClient, dataSource, cryptographyClient);
return await getQueryClient(rootClient, dataSource, cryptographyClient);
} catch (error: any) {
logger.error(`Fail to get data source client for dataSourceId: [${dataSourceId}]`);
logger.error(error);
// Re-throw as DataSourceConfigError
throw new DataSourceConfigError('Fail to get data source client: ', error);
}
};

export const getDataSource = async (
dataSourceId: string,
savedObjects: SavedObjectsClientContract
): Promise<SavedObject<DataSourceAttributes>> => {
try {
const dataSource = await savedObjects.get<DataSourceAttributes>(
DATA_SOURCE_SAVED_OBJECT_TYPE,
dataSourceId
);
return dataSource;
} catch (error: any) {
// it will cause 500 error when failed to get saved objects, need to handle such error gracefully
throw SavedObjectsErrorHelpers.createBadRequestError(error.message);
}
const dataSource = await savedObjects.get<DataSourceAttributes>(
DATA_SOURCE_SAVED_OBJECT_TYPE,
dataSourceId
);
return dataSource;
};

export const getCredential = async (
dataSource: SavedObject<DataSourceAttributes>,
cryptographyClient: CryptographyClient
): Promise<UsernamePasswordTypedContent> => {
try {
const { username, password } = dataSource.attributes.auth.credentials!;
const decodedPassword = await cryptographyClient.decodeAndDecrypt(password);
const credential = {
username,
password: decodedPassword,
};
const { username, password } = dataSource.attributes.auth.credentials!;
const decodedPassword = await cryptographyClient.decodeAndDecrypt(password);
const credential = {
username,
password: decodedPassword,
};

return credential;
} catch (error: any) {
// it will cause 500 error when failed to get saved objects, need to handle such error gracefully
throw SavedObjectsErrorHelpers.createBadRequestError(error.message);
}
return credential;
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data_source/server/data_source_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class DataSourceService {
async setup(config: DataSourcePluginConfigType) {
const openSearchClientPoolSetup = await this.openSearchClientPool.setup(config);

const getDataSourceClient = async (
const getDataSourceClient = (
dataSourceId: string,
savedObjects: SavedObjectsClientContract,
cryptographyClient: CryptographyClient
Expand Down
40 changes: 40 additions & 0 deletions src/plugins/data_source/server/lib/error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

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

describe('DataSourceConfigError', () => {
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({
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);
});

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({
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({
statusCode: 400,
message: 'test prefix: test reason message',
});
});
});
21 changes: 21 additions & 0 deletions src/plugins/data_source/server/lib/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { SavedObjectsErrorHelpers } from '../../../../../src/core/server';
import { OsdError } from '../../../../../src/plugins/opensearch_dashboards_utils/common';

export class DataSourceConfigError 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, 400 for both savedObject client
// 4xx errors, and other errors
this.statusCode = SavedObjectsErrorHelpers.isOpenSearchUnavailableError(error) ? 500 : 400;
}
}
24 changes: 8 additions & 16 deletions src/plugins/data_source/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { OpenSearchClientError } from '@opensearch-project/opensearch/lib/errors';
import { Observable } from 'rxjs';
import { first, map } from 'rxjs/operators';
import {
Expand Down Expand Up @@ -129,21 +128,14 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc
return {
opensearch: {
getClient: (dataSourceId: string) => {
try {
const auditor = auditTrailPromise.then((auditTrail) => auditTrail.asScoped(req));
this.logAuditMessage(auditor, dataSourceId, req);

return dataSourceService.getDataSourceClient(
dataSourceId,
context.core.savedObjects.client,
cryptographyClient
);
} catch (error: any) {
logger.error(
`Fail to get data source client for dataSourceId: [${dataSourceId}]. Detail: ${error.messages}`
);
throw new OpenSearchClientError(error.message);
}
const auditor = auditTrailPromise.then((auditTrail) => auditTrail.asScoped(req));
this.logAuditMessage(auditor, dataSourceId, req);

return dataSourceService.getDataSourceClient(
dataSourceId,
context.core.savedObjects.client,
cryptographyClient
);
},
},
};
Expand Down