Skip to content

Commit

Permalink
[RND-642] Adds QUERY_FAILURE_CONNECTION_ERROR as a QueryResult respon…
Browse files Browse the repository at this point in the history
…se (#299)

* Adds QUERY_FAILURE_CONNECTION_ERROR as a QueryResult response

* Error types.

* QUERY_FAILURE_INVALID_QUERY should return 500 and not 502
  • Loading branch information
DavidJGapCR authored Sep 25, 2023
1 parent aab3e45 commit 4b452e4
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,7 @@ export async function handleElasticSearchError(
switch (elasticSearchClientError.name) {
case 'ConfigurationError':
case 'ConnectionError':
case 'DeserializationError':
case 'NoLivingConnectionsError':
case 'NotCompatibleError':
case 'ElasticsearchClientError':
case 'RequestAbortedError':
case 'SerializationError':
case 'TimeoutError': {
if (elasticSearchClientError?.message !== undefined) {
Logger.error(
Expand All @@ -36,7 +31,7 @@ export async function handleElasticSearchError(
`(${elasticSearchClientError.name}) - ${elasticSearchClientError.message}`,
);
return {
response: 'QUERY_FAILURE_INVALID_QUERY',
response: 'QUERY_FAILURE_CONNECTION_ERROR',
documents: [],
failureMessage: elasticSearchClientError.message,
};
Expand Down Expand Up @@ -65,6 +60,11 @@ export async function handleElasticSearchError(
}
break;
}
case 'DeserializationError':
case 'NoLivingConnectionsError':
case 'NotCompatibleError':
case 'ElasticsearchClientError':
case 'SerializationError':
default: {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
// See the LICENSE and NOTICES files in the project root for more information.

import { Client } from '@elastic/elasticsearch';
import { Client, errors } from '@elastic/elasticsearch';
import Mock from '@elastic/elasticsearch-mock';
import {
PaginationParameters,
Expand Down Expand Up @@ -321,4 +321,82 @@ describe('when querying for students', () => {
});
});
});

describe('given there is an invalid query error', () => {
let queryResult: QueryResult;
const setupMockRequestConnectionError = (): Client => {
mock.add(
{
method: 'POST',
path: `/${indexFromResourceInfo(resourceInfo)}/_search`,
},
() =>
new errors.ResponseError({
body: { errors: {}, status: 500 },
statusCode: 500,
warnings: ['index_not_found_exception'],
meta: {} as any,
}),
);

const client = new Client({
node: 'http://localhost:9200',
Connection: mock.getConnection(),
});
return client;
};

beforeAll(async () => {
const client = setupMockRequestConnectionError();
const request = setupQueryRequest({ type: 'FULL_ACCESS' }, {}, {});

queryResult = await queryDocuments(request, client);
});

it('should return connection error', async () => {
expect(queryResult.failureMessage).toMatchInlineSnapshot('"{"errors":{},"status":500}"');
expect(queryResult.response).toBe('QUERY_FAILURE_INVALID_QUERY');
expect(queryResult.documents.length).toBe(0);
});

afterAll(() => {
mock.clearAll();
});
});

describe('given there is a connection error', () => {
let queryResult: QueryResult;
const setupMockRequestConnectionError = (): Client => {
mock.add(
{
method: 'POST',
path: `/${indexFromResourceInfo(resourceInfo)}/_search`,
},
() => new errors.ConnectionError('Connection error failure message'),
);

const client = new Client({
node: 'http://localhost:9200',
Connection: mock.getConnection(),
});
return client;
};

beforeAll(async () => {
const client = setupMockRequestConnectionError();
const request = setupQueryRequest({ type: 'FULL_ACCESS' }, {}, {});

queryResult = await queryDocuments(request, client);
});

it('should return connection error', async () => {
expect(queryResult.failureMessage).toBe('Connection error failure message');
expect(queryResult.response).toBe('QUERY_FAILURE_CONNECTION_ERROR');
expect(queryResult.documents.length).toBe(0);
});

afterAll(() => {
mock.clearAll();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,19 @@ export async function handleOpenSearchError(
switch (openSearchClientError.name) {
case 'ConfigurationError':
case 'ConnectionError':
case 'DeserializationError':
case 'NoLivingConnectionsError':
case 'NotCompatibleError':
case 'OpenSearchClientError':
case 'RequestAbortedError':
case 'SerializationError':
case 'TimeoutError': {
if (openSearchClientError?.message !== undefined) {
Logger.error(
`${moduleName} ${documentProcessError}`,
traceId,
`(${openSearchClientError.name}) - ${openSearchClientError.message}`,
);
return { response: 'QUERY_FAILURE_INVALID_QUERY', documents: [], failureMessage: openSearchClientError.message };
return {
response: 'QUERY_FAILURE_CONNECTION_ERROR',
documents: [],
failureMessage: openSearchClientError.message,
};
}
break;
}
Expand Down Expand Up @@ -94,6 +93,11 @@ export async function handleOpenSearchError(
}
break;
}
case 'DeserializationError':
case 'NoLivingConnectionsError':
case 'NotCompatibleError':
case 'OpenSearchClientError':
case 'SerializationError':
default: {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
// See the LICENSE and NOTICES files in the project root for more information.

import { Client } from '@opensearch-project/opensearch';
import { Client, errors } from '@opensearch-project/opensearch';
import Mock from '@short.io/opensearch-mock';
import {
PaginationParameters,
Expand Down Expand Up @@ -325,4 +325,40 @@ describe('when querying for students', () => {
});
});
});

describe('given there is a connection error', () => {
let queryResult: QueryResult;
const setupMockRequestConnectionError = (): Client => {
mock.add(
{
method: 'POST',
path: `/${indexFromResourceInfo(resourceInfo)}/_search`,
},
() => new errors.ConnectionError('Connection error failure message'),
);

const client = new Client({
node: 'http://localhost:9200',
Connection: mock.getConnection(),
});
return client;
};

beforeAll(async () => {
const client = setupMockRequestConnectionError();
const request = setupQueryRequest({ type: 'FULL_ACCESS' }, {}, {});

queryResult = await queryDocuments(request, client);
});

it('should return connection error', async () => {
expect(queryResult.failureMessage).toBe('Connection error failure message');
expect(queryResult.response).toBe('QUERY_FAILURE_CONNECTION_ERROR');
expect(queryResult.documents.length).toBe(0);
});

afterAll(() => {
mock.clearAll();
});
});
});
13 changes: 12 additions & 1 deletion Meadowlark-js/packages/meadowlark-core/src/handler/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,25 @@ export async function query(frontendRequest: FrontendRequest): Promise<FrontendR
}

if (response === 'QUERY_FAILURE_INVALID_QUERY') {
const invalidQueryHeaders = {
...frontendRequest.middleware.headerMetadata,
[TOTAL_COUNT_HEADER_NAME]: result.totalCount?.toString() ?? '0',
};
writeDebugStatusToLog(moduleName, frontendRequest, 'query', 500);
return {
statusCode: 500,
headers: invalidQueryHeaders,
};
}

if (response === 'QUERY_FAILURE_CONNECTION_ERROR') {
const invalidQueryHeaders = {
...frontendRequest.middleware.headerMetadata,
[TOTAL_COUNT_HEADER_NAME]: result.totalCount?.toString() ?? '0',
};
writeDebugStatusToLog(moduleName, frontendRequest, 'query', 502);
return {
statusCode: 502,
body: documents,
headers: invalidQueryHeaders,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export type QueryResult = {
| 'QUERY_FAILURE_INVALID_QUERY'
| 'QUERY_FAILURE_AUTHORIZATION'
| 'UNKNOWN_FAILURE'
| 'QUERY_FAILURE_INDEX_NOT_FOUND';
| 'QUERY_FAILURE_INDEX_NOT_FOUND'
| 'QUERY_FAILURE_CONNECTION_ERROR';
documents: Array<object>;
failureMessage?: string;
totalCount?: number;
Expand Down
66 changes: 66 additions & 0 deletions Meadowlark-js/packages/meadowlark-core/test/handler/Query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,72 @@ describe('given persistence is going to fail', () => {
});
});

describe('given persistence fails with connection error', () => {
let response: FrontendResponse;
let mockQueryHandler: any;
const expectedError = 'Error';

beforeAll(async () => {
mockQueryHandler = jest.spyOn(PluginLoader, 'getQueryHandler').mockReturnValue({
...NoDocumentStorePlugin,
queryDocuments: async () =>
Promise.resolve({
response: 'QUERY_FAILURE_CONNECTION_ERROR',
documents: [],
failureMessage: expectedError,
}),
});

// Act
response = await query(frontendRequest);
});

afterAll(() => {
mockQueryHandler.mockRestore();
});

it('returns status 502', () => {
expect(response.statusCode).toEqual(502);
});

it('does not return a message body', () => {
expect(response.body).toBeUndefined();
});
});

describe('given persistence fails with invalid query', () => {
let response: FrontendResponse;
let mockQueryHandler: any;
const expectedError = 'Error';

beforeAll(async () => {
mockQueryHandler = jest.spyOn(PluginLoader, 'getQueryHandler').mockReturnValue({
...NoDocumentStorePlugin,
queryDocuments: async () =>
Promise.resolve({
response: 'QUERY_FAILURE_INVALID_QUERY',
documents: [],
failureMessage: expectedError,
}),
});

// Act
response = await query(frontendRequest);
});

afterAll(() => {
mockQueryHandler.mockRestore();
});

it('returns status 500', () => {
expect(response.statusCode).toEqual(500);
});

it('does not return a message body', () => {
expect(response.body).toBeUndefined();
});
});

describe('given successful query result', () => {
let response: FrontendResponse;
let mockQueryHandler: any;
Expand Down

0 comments on commit 4b452e4

Please sign in to comment.