Skip to content

Commit

Permalink
[5.6] [fix/UiSettings] ignore certain errors (#13079) (#13409)
Browse files Browse the repository at this point in the history
* [fix/UiSettings] ignore certain errors (#13079)

* [savedObjects/mappingFallback] check for body.error.type

* [savedObjectsClient] decorate non-es errors
  • Loading branch information
spalger authored Aug 22, 2017
1 parent 4151edf commit 2655237
Show file tree
Hide file tree
Showing 42 changed files with 1,203 additions and 174 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
"babel-register": "6.18.0",
"bluebird": "2.9.34",
"body-parser": "1.12.0",
"boom": "2.8.0",
"boom": "5.2.0",
"brace": "0.5.1",
"bunyan": "1.7.1",
"check-hash": "1.0.1",
Expand Down
6 changes: 4 additions & 2 deletions src/core_plugins/elasticsearch/lib/__tests__/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,12 @@ describe('plugins/elasticsearch', function () {

describe('wrap401Errors', () => {
let handler;
const error = new Error('Authentication required');
error.statusCode = 401;
let error;

beforeEach(() => {
error = new Error('Authentication required');
error.statusCode = 401;

handler = sinon.stub();
});

Expand Down
2 changes: 0 additions & 2 deletions src/core_plugins/kibana/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { mkdirp as mkdirpNode } from 'mkdirp';

import manageUuid from './server/lib/manage_uuid';
import search from './server/routes/api/search';
import settings from './server/routes/api/settings';
import { importApi } from './server/routes/api/import';
import { exportApi } from './server/routes/api/export';
import scripts from './server/routes/api/scripts';
Expand Down Expand Up @@ -153,7 +152,6 @@ module.exports = function (kibana) {
manageUuid(server);
// routes
search(server);
settings(server);
scripts(server);
importApi(server);
exportApi(server);
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/kibana/server/lib/handle_es_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module.exports = function handleESError(error) {
error instanceof esErrors.ServiceUnavailable ||
error instanceof esErrors.NoConnections ||
error instanceof esErrors.RequestTimeout) {
return Boom.serverTimeout(error);
return Boom.serverUnavailable(error);
} else if (error instanceof esErrors.Conflict || _.contains(error.message, 'index_template_already_exists')) {
return Boom.conflict(error);
} else if (error instanceof esErrors[403]) {
Expand Down
6 changes: 0 additions & 6 deletions src/core_plugins/kibana/server/routes/api/settings/index.js

This file was deleted.

This file was deleted.

15 changes: 0 additions & 15 deletions src/core_plugins/kibana/server/routes/api/settings/register_get.js

This file was deleted.

21 changes: 0 additions & 21 deletions src/core_plugins/kibana/server/routes/api/settings/register_set.js

This file was deleted.

This file was deleted.

2 changes: 2 additions & 0 deletions src/server/http/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ describe('routes', () => {

it('redirects shortened urls', (done) => {
kbnTestServer.makeRequest(kbnServer, shortenOptions, (res) => {
expect(res).to.have.property('statusCode', 200);

const gotoOptions = {
method: 'GET',
url: '/goto/' + res.payload
Expand Down
8 changes: 4 additions & 4 deletions src/server/http/__tests__/short_url_lookup.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import expect from 'expect.js';
import sinon from 'sinon';
import shortUrlLookupProvider from '../short_url_lookup';
import { SavedObjectsClient } from '../../saved_objects/client';

describe('shortUrlLookupProvider', () => {
const ID = 'bf00ad16941fc51420f91a93428b27a0';
Expand All @@ -17,7 +18,8 @@ describe('shortUrlLookupProvider', () => {
savedObjectsClient = {
get: sandbox.stub(),
create: sandbox.stub().returns(Promise.resolve({ id: ID })),
update: sandbox.stub()
update: sandbox.stub(),
errors: SavedObjectsClient.errors
};

req = { getSavedObjectsClient: () => savedObjectsClient };
Expand Down Expand Up @@ -58,10 +60,8 @@ describe('shortUrlLookupProvider', () => {
});

it('gracefully handles version conflict', async () => {
const error = new Error('version conflict');
error.data = { type: 'version_conflict_engine_exception' };
const error = savedObjectsClient.errors.decorateConflictError(new Error());
savedObjectsClient.create.throws(error);

const id = await shortUrl.generateUrlId(URL, req);
expect(id).to.eql(ID);
});
Expand Down
10 changes: 6 additions & 4 deletions src/server/http/short_url_lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,24 @@ export default function (server) {
return {
async generateUrlId(url, req) {
const id = crypto.createHash('md5').update(url).digest('hex');
const savedObjectsClient = req.getSavedObjectsClient();
const { isConflictError } = savedObjectsClient.errors;

try {
const doc = await req.getSavedObjectsClient().create('url', {
const doc = await savedObjectsClient.create('url', {
url,
accessCount: 0,
createDate: new Date(),
accessDate: new Date()
}, { id });

return doc.id;
} catch(e) {
if (get(e, 'data.type') === 'version_conflict_engine_exception') {
} catch (error) {
if (isConflictError(error)) {
return id;
}

throw e;
throw error;
}
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import expect from 'expect.js';
import sinon from 'sinon';

import { SavedObjectsClient } from '../saved_objects_client';
import { decorateEsError } from '../lib';
const { BadRequest } = elasticsearch.errors;

describe('SavedObjectsClient', () => {
Expand All @@ -23,11 +24,11 @@ describe('SavedObjectsClient', () => {

describe('#create', () => {
it('falls back to single-type mapping', async () => {
const error = new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', {
const error = decorateEsError(new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', {
body: {
error: illegalArgumentException
}
});
}));

callAdminCluster
.onFirstCall().throws(error)
Expand All @@ -49,11 +50,11 @@ describe('SavedObjectsClient', () => {

it('prepends id for single-type', async () => {
const id = 'foo';
const error = new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', {
const error = decorateEsError(new BadRequest('[illegal_argument_exception] Rejecting mapping update to [.kibana-v6]', {
body: {
error: illegalArgumentException
}
});
}));

callAdminCluster
.onFirstCall().throws(error)
Expand Down Expand Up @@ -154,13 +155,13 @@ describe('SavedObjectsClient', () => {
const type = 'index-pattern';
const version = 2;
const attributes = { title: 'Testing' };
const error = new BadRequest('[document_missing_exception] [config][logstash-*]: document missing', {
const error = decorateEsError(new BadRequest('[document_missing_exception] [config][logstash-*]: document missing', {
body: {
error: {
type: 'document_missing_exception'
}
}
});
}));

beforeEach(() => {
callAdminCluster
Expand Down
89 changes: 89 additions & 0 deletions src/server/saved_objects/client/lib/__tests__/decorate_es_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import expect from 'expect.js';
import { errors as esErrors } from 'elasticsearch';

import { decorateEsError } from '../decorate_es_error';
import {
isEsUnavailableError,
isConflictError,
isNotAuthorizedError,
isForbiddenError,
isNotFoundError,
isBadRequestError,
} from '../errors';

describe('savedObjectsClient/decorateEsError', () => {
it('always returns the same error it receives', () => {
const error = new Error();
expect(decorateEsError(error)).to.be(error);
});

it('makes es.ConnectionFault a SavedObjectsClient/EsUnavailable error', () => {
const error = new esErrors.ConnectionFault();
expect(isEsUnavailableError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isEsUnavailableError(error)).to.be(true);
});

it('makes es.ServiceUnavailable a SavedObjectsClient/EsUnavailable error', () => {
const error = new esErrors.ServiceUnavailable();
expect(isEsUnavailableError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isEsUnavailableError(error)).to.be(true);
});

it('makes es.NoConnections a SavedObjectsClient/EsUnavailable error', () => {
const error = new esErrors.NoConnections();
expect(isEsUnavailableError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isEsUnavailableError(error)).to.be(true);
});

it('makes es.RequestTimeout a SavedObjectsClient/EsUnavailable error', () => {
const error = new esErrors.RequestTimeout();
expect(isEsUnavailableError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isEsUnavailableError(error)).to.be(true);
});

it('makes es.Conflict a SavedObjectsClient/Conflict error', () => {
const error = new esErrors.Conflict();
expect(isConflictError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isConflictError(error)).to.be(true);
});

it('makes es.AuthenticationException a SavedObjectsClient/NotAuthorized error', () => {
const error = new esErrors.AuthenticationException();
expect(isNotAuthorizedError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isNotAuthorizedError(error)).to.be(true);
});

it('makes es.Forbidden a SavedObjectsClient/Forbidden error', () => {
const error = new esErrors.Forbidden();
expect(isForbiddenError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isForbiddenError(error)).to.be(true);
});

it('makes es.NotFound a SavedObjectsClient/NotFound error', () => {
const error = new esErrors.NotFound();
expect(isNotFoundError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isNotFoundError(error)).to.be(true);
});

it('makes es.BadRequest a SavedObjectsClient/BadRequest error', () => {
const error = new esErrors.BadRequest();
expect(isBadRequestError(error)).to.be(false);
expect(decorateEsError(error)).to.be(error);
expect(isBadRequestError(error)).to.be(true);
});

it('returns other errors as Boom errors', () => {
const error = new Error();
expect(error).to.not.have.property('isBoom');
expect(decorateEsError(error)).to.be(error);
expect(error).to.have.property('isBoom');
});
});
Loading

0 comments on commit 2655237

Please sign in to comment.