Skip to content

Commit

Permalink
Adds new SavedObjectsRespository error type for 404 that do not origi…
Browse files Browse the repository at this point in the history 
…nate from Elasticsearch responses (#107301)

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
@TinaHeiligers @kibanamachine
TinaHeiligers and kibanamachine authored Aug 10, 2021
1 parent fa47c33 commit cdf90aa
Show file tree
Hide file tree
Showing 10 changed files with 248 additions and 38 deletions.
23 changes: 23 additions & 0 deletions ...core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsErrorHelpers](./kibana-plugin-core-server.savedobjectserrorhelpers.md) &gt; [createGenericNotFoundEsUnavailableError](./kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md)

## SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError() method

<b>Signature:</b>

```typescript
static createGenericNotFoundEsUnavailableError(type: string, id: string): DecoratedError;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| type | <code>string</code> | |
| id | <code>string</code> | |

<b>Returns:</b>

`DecoratedError`

1 change: 1 addition & 0 deletions docs/development/core/server/kibana-plugin-core-server.savedobjectserrorhelpers.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export declare class SavedObjectsErrorHelpers
| [createBadRequestError(reason)](./kibana-plugin-core-server.savedobjectserrorhelpers.createbadrequesterror.md) | <code>static</code> | |
| [createConflictError(type, id, reason)](./kibana-plugin-core-server.savedobjectserrorhelpers.createconflicterror.md) | <code>static</code> | |
| [createGenericNotFoundError(type, id)](./kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfounderror.md) | <code>static</code> | |
| [createGenericNotFoundEsUnavailableError(type, id)](./kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md) | <code>static</code> | |
| [createIndexAliasNotFoundError(alias)](./kibana-plugin-core-server.savedobjectserrorhelpers.createindexaliasnotfounderror.md) | <code>static</code> | |
| [createInvalidVersionError(versionInput)](./kibana-plugin-core-server.savedobjectserrorhelpers.createinvalidversionerror.md) | <code>static</code> | |
| [createTooManyRequestsError(type, id)](./kibana-plugin-core-server.savedobjectserrorhelpers.createtoomanyrequestserror.md) | <code>static</code> | |
Expand Down
5 changes: 3 additions & 2 deletions src/core/server/elasticsearch/client/mocks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,10 @@ export type MockedTransportRequestPromise<T> = TransportRequestPromise<T> & {

const createSuccessTransportRequestPromise = <T>(
body: T,
{ statusCode = 200 }: { statusCode?: number } = {}
{ statusCode = 200 }: { statusCode?: number } = {},
headers?: Record<string, string | string[]>
): MockedTransportRequestPromise<ApiResponse<T>> => {
const response = createApiResponse({ body, statusCode });
const response = createApiResponse({ body, statusCode, headers });
const promise = Promise.resolve(response);
(promise as MockedTransportRequestPromise<ApiResponse<T>>).abort = jest.fn();

Expand Down
1 change: 1 addition & 0 deletions src/core/server/elasticsearch/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ export type {
GetResponse,
DeleteDocumentResponse,
} from './client';
export { isSupportedEsServer } from './supported_server_response_check';
17 changes: 17 additions & 0 deletions src/core/server/elasticsearch/supported_server_response_check.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export const PRODUCT_RESPONSE_HEADER = 'x-elastic-product';
/**
* Response headers check to determine if the response is from Elasticsearch
* @param headers Response headers
* @returns boolean
*/
// This check belongs to the elasticsearch service as a dedicated helper method.
export const isSupportedEsServer = (headers: Record<string, string> | null) => {
return !!headers && headers[PRODUCT_RESPONSE_HEADER] === 'Elasticsearch';
};
41 changes: 41 additions & 0 deletions src/core/server/saved_objects/service/lib/errors.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,4 +439,45 @@ describe('savedObjectsClient/errorTypes', () => {
});
});
});

describe('NotFoundEsUnavailableError', () => {
it('makes an error identifiable as an EsUnavailable error', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(SavedObjectsErrorHelpers.isEsUnavailableError(error)).toBe(true);
});

it('returns a boom error', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(error).toHaveProperty('isBoom', true);
});

it('decorates the error message with the saved object that was not found', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(error.output.payload).toHaveProperty(
'message',
'x-elastic-product not present or not recognized: Saved object [foo/bar] not found'
);
});

describe('error.output', () => {
it('specifies the saved object that was not found', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(
'foo',
'bar'
);
expect(error.output.payload).toHaveProperty(
'message',
'x-elastic-product not present or not recognized: Saved object [foo/bar] not found'
);
});

it('sets statusCode to 503', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(
'foo',
'bar'
);
expect(error.output).toHaveProperty('statusCode', 503);
});
});
});
});
8 changes: 8 additions & 0 deletions src/core/server/saved_objects/service/lib/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,12 @@ export class SavedObjectsErrorHelpers {
public static isGeneralError(error: Error | DecoratedError) {
return isSavedObjectsClientError(error) && error[code] === CODE_GENERAL_ERROR;
}

public static createGenericNotFoundEsUnavailableError(type: string, id: string) {
const notFoundError = this.createGenericNotFoundError(type, id);
return this.decorateEsUnavailableError(
new Error(`${notFoundError.message}`),
`x-elastic-product not present or not recognized`
);
}
}
140 changes: 115 additions & 25 deletions src/core/server/saved_objects/service/lib/repository.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const createGenericNotFoundError = (...args) =>
SavedObjectsErrorHelpers.createGenericNotFoundError(...args).output.payload;
const createUnsupportedTypeError = (...args) =>
SavedObjectsErrorHelpers.createUnsupportedTypeError(...args).output.payload;
const createGenericNotFoundEsUnavailableError = (...args) =>
SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(...args).output.payload;

describe('SavedObjectsRepository', () => {
let client;
Expand Down Expand Up @@ -2202,6 +2204,11 @@ describe('SavedObjectsRepository', () => {
createGenericNotFoundError(type, id)
);
};
const expectNotFoundEsUnavailableError = async (type, id) => {
await expect(savedObjectsRepository.delete(type, id)).rejects.toThrowError(
createGenericNotFoundEsUnavailableError(type, id)
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
Expand All @@ -2221,20 +2228,42 @@ describe('SavedObjectsRepository', () => {

it(`throws when ES is unable to find the document during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ found: false },
undefined,
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the index during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{},
{ statusCode: 404 },
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the document during get with missing Elasticsearch header`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
);
await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
});

it(`throws when ES is unable to find the index during get with missing Elasticsearch header`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
);
await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
});

it(`throws when the type is multi-namespace and the document exists, but not in this namespace`, async () => {
const response = getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id }, namespace);
client.get.mockResolvedValueOnce(
Expand Down Expand Up @@ -2278,7 +2307,7 @@ describe('SavedObjectsRepository', () => {
client.delete.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' })
);
await expectNotFoundError(type, id);
await expectNotFoundEsUnavailableError(type, id);
expect(client.delete).toHaveBeenCalledTimes(1);
});

Expand All @@ -2288,7 +2317,7 @@ describe('SavedObjectsRepository', () => {
error: { type: 'index_not_found_exception' },
})
);
await expectNotFoundError(type, id);
await expectNotFoundEsUnavailableError(type, id);
expect(client.delete).toHaveBeenCalledTimes(1);
});

Expand Down Expand Up @@ -3170,7 +3199,11 @@ describe('SavedObjectsRepository', () => {
createGenericNotFoundError(type, id)
);
};

const expectNotFoundEsUnavailableError = async (type, id) => {
await expect(savedObjectsRepository.get(type, id)).rejects.toThrowError(
createGenericNotFoundEsUnavailableError(type, id)
);
};
it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.get(type, id, { namespace: ALL_NAMESPACES_STRING })
Expand All @@ -3189,15 +3222,23 @@ describe('SavedObjectsRepository', () => {

it(`throws when ES is unable to find the document during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ found: false },
undefined,
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(type, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the index during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{},
{ statusCode: 404 },
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(type, id);
expect(client.get).toHaveBeenCalledTimes(1);
Expand All @@ -3213,6 +3254,15 @@ describe('SavedObjectsRepository', () => {
});
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES does not return the correct header when finding the document during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
);
await expectNotFoundEsUnavailableError(type, id);

expect(client.get).toHaveBeenCalledTimes(1);
});
});

describe('returns', () => {
Expand Down Expand Up @@ -3314,9 +3364,12 @@ describe('SavedObjectsRepository', () => {

it('because alias is not used and actual object is not found', async () => {
const options = { namespace: undefined };
const response = { found: false };
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(response) // for actual target
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ found: false },
undefined,
{ 'x-elastic-product': 'Elasticsearch' }
) // for actual target
);

await expectNotFoundError(type, id, options);
Expand Down Expand Up @@ -3854,26 +3907,34 @@ describe('SavedObjectsRepository', () => {
if (registry.isMultiNamespace(type)) {
const mockGetResponse = getMockGetResponse({ type, id }, options?.namespace);
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(mockGetResponse)
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ ...mockGetResponse },
{ statusCode: 200 },
{ 'x-elastic-product': 'Elasticsearch' }
)
);
}
client.update.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({
_id: `${type}:${id}`,
...mockVersionProps,
result: 'updated',
// don't need the rest of the source for test purposes, just the namespace and namespaces attributes
get: {
_source: {
namespaces: [options?.namespace ?? 'default'],
namespace: options?.namespace,
elasticsearchClientMock.createSuccessTransportRequestPromise(
{
_id: `${type}:${id}`,
...mockVersionProps,
result: 'updated',
// don't need the rest of the source for test purposes, just the namespace and namespaces attributes
get: {
_source: {
namespaces: [options?.namespace ?? 'default'],
namespace: options?.namespace,

// "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the
// operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response.
...(includeOriginId && { originId }),
// "includeOriginId" is not an option for the operation; however, if the existing saved object contains an originId attribute, the
// operation will return it in the result. This flag is just used for test purposes to modify the mock cluster call response.
...(includeOriginId && { originId }),
},
},
},
})
{ statusCode: 200 },
{ 'x-elastic-product': 'Elasticsearch' }
)
);
const result = await savedObjectsRepository.update(type, id, attributes, options);
expect(client.get).toHaveBeenCalledTimes(registry.isMultiNamespace(type) ? 1 : 0);
Expand Down Expand Up @@ -4059,6 +4120,11 @@ describe('SavedObjectsRepository', () => {
createGenericNotFoundError(type, id)
);
};
const expectNotFoundEsUnavailableError = async (type, id) => {
await expect(savedObjectsRepository.update(type, id)).rejects.toThrowError(
createGenericNotFoundEsUnavailableError(type, id)
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
Expand All @@ -4078,20 +4144,44 @@ describe('SavedObjectsRepository', () => {

it(`throws when ES is unable to find the document during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ found: false },
undefined,
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the index during get`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
elasticsearchClientMock.createSuccessTransportRequestPromise(
{},
{ statusCode: 404 },
{ 'x-elastic-product': 'Elasticsearch' }
)
);
await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the document during get with missing Elasticsearch header`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
);
await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the index during get with missing Elasticsearch`, async () => {
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
);
await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when type is multi-namespace and the document exists, but not in this namespace`, async () => {
const response = getMockGetResponse({ type: MULTI_NAMESPACE_ISOLATED_TYPE, id }, namespace);
client.get.mockResolvedValueOnce(
Expand Down
Loading

0 comments on commit cdf90aa

Please sign in to comment.