[savedObjects] fix error handling when Kibana index is missing

src/server/saved_objects/client/__tests__/saved_objects_client.js

@@ -127,11 +127,9 @@ describe('SavedObjectsClient', () => {
         title: 'Logstash'
       });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWith(callAdminCluster, 'index');
       sinon.assert.calledOnce(onBeforeWrite);
-
-      const args = callAdminCluster.getCall(0).args;
-      expect(args[0]).to.be('index');
     });
 
     it('should use create action if ID defined and overwrite=false', async () => {
@@ -141,35 +139,35 @@ describe('SavedObjectsClient', () => {
         id: 'logstash-*',
       });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWith(callAdminCluster, 'create');
       sinon.assert.calledOnce(onBeforeWrite);
-
-      const args = callAdminCluster.getCall(0).args;
-      expect(args[0]).to.be('create');
     });
 
     it('allows for id to be provided', async () => {
       await savedObjectsClient.create('index-pattern', {
         title: 'Logstash'
       }, { id: 'logstash-*' });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.calledOnce(onBeforeWrite);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        id: 'index-pattern:logstash-*'
+      }));
 
-      const args = callAdminCluster.getCall(0).args;
-      expect(args[1].id).to.be('index-pattern:logstash-*');
+      sinon.assert.calledOnce(onBeforeWrite);
     });
 
     it('self-generates an ID', async () => {
       await savedObjectsClient.create('index-pattern', {
         title: 'Logstash'
       });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.calledOnce(onBeforeWrite);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        id: sinon.match(/index-pattern:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/)
+      }));
 
-      const args = callAdminCluster.getCall(0).args;
-      expect(args[1].id).to.match(/index-pattern:[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}/);
+      sinon.assert.calledOnce(onBeforeWrite);
     });
   });
 
@@ -182,26 +180,24 @@ describe('SavedObjectsClient', () => {
         { type: 'index-pattern', id: 'two', attributes: { title: 'Test Two' } }
       ]);
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.calledOnce(onBeforeWrite);
-
-      const args = callAdminCluster.getCall(0).args;
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, 'bulk', sinon.match({
+        body: [
+          { create: { _type: 'doc', _id: 'config:one' } },
+          { type: 'config', ...mockTimestampFields, config: { title: 'Test One' } },
+          { create: { _type: 'doc', _id: 'index-pattern:two' } },
+          { type: 'index-pattern', ...mockTimestampFields, 'index-pattern': { title: 'Test Two' } }
+        ]
+      }));
 
-      expect(args[0]).to.be('bulk');
-      expect(args[1].body).to.eql([
-        { create: { _type: 'doc', _id: 'config:one' } },
-        { type: 'config', ...mockTimestampFields, config: { title: 'Test One' } },
-        { create: { _type: 'doc', _id: 'index-pattern:two' } },
-        { type: 'index-pattern', ...mockTimestampFields, 'index-pattern': { title: 'Test Two' } }
-      ]);
+      sinon.assert.calledOnce(onBeforeWrite);
     });
 
     it('should overwrite objects if overwrite is truthy', async () => {
       callAdminCluster.returns({ items: [] });
 
       await savedObjectsClient.bulkCreate([{ type: 'foo', id: 'bar', attributes: {} }], { overwrite: false });
       sinon.assert.calledOnce(callAdminCluster);
-      sinon.assert.calledOnce(onBeforeWrite);
       sinon.assert.calledWithExactly(callAdminCluster, 'bulk', sinon.match({
         body: [
           // uses create because overwriting is not allowed
@@ -210,12 +206,13 @@ describe('SavedObjectsClient', () => {
         ]
       }));
 
+      sinon.assert.calledOnce(onBeforeWrite);
+
       callAdminCluster.reset();
       onBeforeWrite.reset();
 
       await savedObjectsClient.bulkCreate([{ type: 'foo', id: 'bar', attributes: {} }], { overwrite: true });
       sinon.assert.calledOnce(callAdminCluster);
-      sinon.assert.calledOnce(onBeforeWrite);
       sinon.assert.calledWithExactly(callAdminCluster, 'bulk', sinon.match({
         body: [
           // uses index because overwriting is allowed
@@ -224,7 +221,7 @@ describe('SavedObjectsClient', () => {
         ]
       }));
 
-
+      sinon.assert.calledOnce(onBeforeWrite);
     });
 
     it('returns document errors', async () => {
@@ -310,7 +307,9 @@ describe('SavedObjectsClient', () => {
 
   describe('#delete', () => {
     it('throws notFound when ES is unable to find the document',  async () => {
-      callAdminCluster.returns(Promise.resolve({ found: false }));
+      callAdminCluster.returns(Promise.resolve({
+        result: 'not_found'
+      }));
 
       try {
         await savedObjectsClient.delete('index-pattern', 'logstash-*');
@@ -323,20 +322,21 @@ describe('SavedObjectsClient', () => {
     });
 
     it('passes the parameters to callAdminCluster', async () => {
-      callAdminCluster.returns({});
+      callAdminCluster.returns({
+        result: 'deleted'
+      });
       await savedObjectsClient.delete('index-pattern', 'logstash-*');
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.calledOnce(onBeforeWrite);
-
-      const [method, args] = callAdminCluster.getCall(0).args;
-      expect(method).to.be('delete');
-      expect(args).to.eql({
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, 'delete', {
         type: 'doc',
         id: 'index-pattern:logstash-*',
         refresh: 'wait_for',
-        index: '.kibana-test'
+        index: '.kibana-test',
+        ignore: [404],
       });
+
+      sinon.assert.calledOnce(onBeforeWrite);
     });
   });
 
@@ -416,24 +416,26 @@ describe('SavedObjectsClient', () => {
     it('accepts per_page/page', async () => {
       await savedObjectsClient.find({ perPage: 10, page: 6 });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.notCalled(onBeforeWrite);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        size: 10,
+        from: 50
+      }));
 
-      const options = callAdminCluster.getCall(0).args[1];
-      expect(options.size).to.be(10);
-      expect(options.from).to.be(50);
+      sinon.assert.notCalled(onBeforeWrite);
     });
 
     it('can filter by fields', async () => {
       await savedObjectsClient.find({ fields: ['title'] });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.notCalled(onBeforeWrite);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        _source: [
+          '*.title', 'type', 'title'
+        ]
+      }));
 
-      const options = callAdminCluster.getCall(0).args[1];
-      expect(options._source).to.eql([
-        '*.title', 'type', 'title'
-      ]);
+      sinon.assert.notCalled(onBeforeWrite);
     });
   });
 
@@ -471,9 +473,11 @@ describe('SavedObjectsClient', () => {
       await savedObjectsClient.get('index-pattern', 'logstash-*');
 
       sinon.assert.notCalled(onBeforeWrite);
-      const [, args] = callAdminCluster.getCall(0).args;
-      expect(args.id).to.eql('index-pattern:logstash-*');
-      expect(args.type).to.eql('doc');
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        id: 'index-pattern:logstash-*',
+        type: 'doc'
+      }));
     });
   });
 
@@ -486,14 +490,17 @@ describe('SavedObjectsClient', () => {
         { id: 'two', type: 'index-pattern' }
       ]);
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.notCalled(onBeforeWrite);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        body: {
+          docs: [
+            { _type: 'doc', _id: 'config:one' },
+            { _type: 'doc', _id: 'index-pattern:two' }
+          ]
+        }
+      }));
 
-      const options = callAdminCluster.getCall(0).args[1];
-      expect(options.body.docs).to.eql([
-        { _type: 'doc', _id: 'config:one' },
-        { _type: 'doc', _id: 'index-pattern:two' }
-      ]);
+      sinon.assert.notCalled(onBeforeWrite);
     });
 
     it('returns early for empty objects argument', async () => {
@@ -502,7 +509,7 @@ describe('SavedObjectsClient', () => {
       const response = await savedObjectsClient.bulkGet([]);
 
       expect(response.saved_objects).to.have.length(0);
-      expect(callAdminCluster.notCalled).to.be(true);
+      sinon.assert.notCalled(callAdminCluster);
       sinon.assert.notCalled(onBeforeWrite);
     });
 
@@ -578,29 +585,29 @@ describe('SavedObjectsClient', () => {
         { version: newVersion - 1 }
       );
 
-      const esParams = callAdminCluster.getCall(0).args[1];
-      expect(esParams.version).to.be(newVersion - 1);
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, sinon.match.string, sinon.match({
+        version: newVersion - 1
+      }));
     });
 
     it('passes the parameters to callAdminCluster', async () => {
       await savedObjectsClient.update('index-pattern', 'logstash-*', { title: 'Testing' });
 
-      expect(callAdminCluster.calledOnce).to.be(true);
-      sinon.assert.calledOnce(onBeforeWrite);
-
-      const args = callAdminCluster.getCall(0).args;
-
-      expect(args[0]).to.be('update');
-      expect(args[1]).to.eql({
+      sinon.assert.calledOnce(callAdminCluster);
+      sinon.assert.calledWithExactly(callAdminCluster, 'update', {
         type: 'doc',
         id: 'index-pattern:logstash-*',
         version: undefined,
         body: {
           doc: { updated_at: mockTimestamp, 'index-pattern': { title: 'Testing' } }
         },
+        ignore: [404],
         refresh: 'wait_for',
         index: '.kibana-test'
       });
+
+      sinon.assert.calledOnce(onBeforeWrite);
     });
   });
 

src/server/saved_objects/client/lib/__tests__/decorate_es_error.js

@@ -66,11 +66,13 @@ describe('savedObjectsClient/decorateEsError', () => {
     expect(isForbiddenError(error)).to.be(true);
   });
 
-  it('makes es.NotFound a SavedObjectsClient/NotFound error', () => {
+  it('discards es.NotFound errors and returns a generic 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);
+    const genericError = decorateEsError(error);
+    expect(genericError).to.not.be(error);
+    expect(isNotFoundError(error)).to.be(false);
+    expect(isNotFoundError(genericError)).to.be(true);
   });
 
   it('makes es.BadRequest a SavedObjectsClient/BadRequest error', () => {

src/server/saved_objects/client/lib/__tests__/errors.js

@@ -8,7 +8,7 @@ import {
   isNotAuthorizedError,
   decorateForbiddenError,
   isForbiddenError,
-  decorateNotFoundError,
+  createGenericNotFoundError,
   isNotFoundError,
   decorateConflictError,
   isConflictError,
@@ -145,42 +145,26 @@ describe('savedObjectsClient/errorTypes', () => {
     });
   });
   describe('NotFound error', () => {
-    describe('decorateNotFoundError', () => {
-      it('returns original object', () => {
-        const error = new Error();
-        expect(decorateNotFoundError(error)).to.be(error);
-      });
-
-      it('makes the error identifiable as a NotFound error', () => {
-        const error = new Error();
-        expect(isNotFoundError(error)).to.be(false);
-        decorateNotFoundError(error);
+    describe('createGenericNotFoundError', () => {
+      it('makes an error identifiable as a NotFound error', () => {
+        const error = createGenericNotFoundError();
         expect(isNotFoundError(error)).to.be(true);
       });
 
-      it('adds boom properties', () => {
-        const error = decorateNotFoundError(new Error());
+      it('is a boom error, has boom properties', () => {
+        const error = createGenericNotFoundError();
+        expect(error).to.have.property('isBoom', true);
         expect(error.output).to.be.an('object');
         expect(error.output.statusCode).to.be(404);
       });
 
-      it('preserves boom properties of input', () => {
-        const error = Boom.forbidden();
-        decorateNotFoundError(error);
-        expect(error.output.statusCode).to.be(403);
-      });
-
       describe('error.output', () => {
-        it('defaults to message of erorr', () => {
-          const error = decorateNotFoundError(new Error('foobar'));
-          expect(error.output.payload).to.have.property('message', 'foobar');
-        });
-        it('prefixes message with passed reason', () => {
-          const error = decorateNotFoundError(new Error('foobar'), 'biz');
-          expect(error.output.payload).to.have.property('message', 'biz: foobar');
+        it('Uses "Not Found" message', () => {
+          const error = createGenericNotFoundError();
+          expect(error.output.payload).to.have.property('message', 'Not Found');
         });
         it('sets statusCode to 404', () => {
-          const error = decorateNotFoundError(new Error('foo'));
+          const error = createGenericNotFoundError();
           expect(error.output).to.have.property('statusCode', 404);
         });
       });

src/server/saved_objects/client/lib/decorate_es_error.js

@@ -17,7 +17,7 @@ import {
   decorateBadRequestError,
   decorateNotAuthorizedError,
   decorateForbiddenError,
-  decorateNotFoundError,
+  createGenericNotFoundError,
   decorateConflictError,
   decorateEsUnavailableError,
   decorateGeneralError,
@@ -51,7 +51,7 @@ export function decorateEsError(error) {
   }
 
   if (error instanceof NotFound) {
-    return decorateNotFoundError(error, reason);
+    return createGenericNotFoundError();
   }
 
   if (error instanceof BadRequest) {

src/server/saved_objects/client/lib/errors.js

@@ -54,8 +54,8 @@ export function isForbiddenError(error) {
 
 // 404 - Not Found
 const CODE_NOT_FOUND = 'SavedObjectsClient/notFound';
-export function decorateNotFoundError(error, reason) {
-  return decorate(error, CODE_NOT_FOUND, 404, reason);
+export function createGenericNotFoundError() {
+  return decorate(Boom.notFound(), CODE_NOT_FOUND, 404);
 }
 export function isNotFoundError(error) {
   return error && error[code] === CODE_NOT_FOUND;