inline writeErrors into createOrUpgradeConfig to simplify testing

src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.test.ts

@@ -20,6 +20,8 @@
 import sinon from 'sinon';
 import Chance from 'chance';
 
+import { SavedObjectsErrorHelpers } from '../../saved_objects';
+
 import { loggingServiceMock } from '../../logging/logging_service.mock';
 import * as getUpgradeableConfigNS from './get_upgradeable_config';
 import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config';
@@ -50,6 +52,7 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
         version,
         buildNum,
         log: logger.get(),
+        handleWriteErrors: false,
         ...options,
       });
 
@@ -173,85 +176,64 @@ describe('uiSettings/createOrUpgradeSavedConfig', function() {
     });
   });
 
-  describe('onWriteError()', () => {
-    it('is called with error and attributes when savedObjectsClient.create rejects', async () => {
-      const { run, savedObjectsClient } = setup();
+  describe('handleWriteErrors', () => {
+    describe('handleWriteErrors: false', () => {
+      it('throws write errors', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.callsFake(async () => {
+          throw error;
+        });
 
-      const error = new Error('foo');
-      savedObjectsClient.create.callsFake(async () => {
-        throw error;
-      });
-
-      const onWriteError = sinon.stub();
-      await run({ onWriteError });
-      sinon.assert.calledOnce(onWriteError);
-      sinon.assert.calledWithExactly(onWriteError, error, {
-        buildNum,
+        await expect(run({ handleWriteErrors: false })).rejects.toThrowError(error);
       });
     });
+    describe('handleWriteErrors:true', () => {
+      it('returns undefined for ConflictError', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateConflictError(error));
 
-    it('resolves with the return value of onWriteError()', async () => {
-      const { run, savedObjectsClient } = setup();
-
-      savedObjectsClient.create.callsFake(async () => {
-        throw new Error('foo');
+        expect(await run({ handleWriteErrors: true })).toBe(undefined);
       });
 
-      const result = await run({ onWriteError: () => 123 });
-      expect(result).toBe(123);
-    });
-
-    it('rejects with the error from onWriteError() if it rejects', async () => {
-      const { run, savedObjectsClient } = setup();
+      it('returns config attributes for NotAuthorizedError', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.throws(
+          SavedObjectsErrorHelpers.decorateNotAuthorizedError(error)
+        );
 
-      savedObjectsClient.create.callsFake(async () => {
-        throw new Error('foo');
+        expect(await run({ handleWriteErrors: true })).toEqual({
+          buildNum,
+        });
       });
 
-      try {
-        await run({
-          onWriteError: (error: Error) => Promise.reject(new Error(`${error.message} bar`)),
+      it('returns config attributes for ForbiddenError', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateForbiddenError(error));
+
+        expect(await run({ handleWriteErrors: true })).toEqual({
+          buildNum,
         });
-        throw new Error('expected run() to reject');
-      } catch (error) {
-        expect(error.message).toBe('foo bar');
-      }
-    });
+      });
 
-    it('rejects with the error from onWriteError() if it throws sync', async () => {
-      const { run, savedObjectsClient } = setup();
+      it('throws error for other SavedObjects exceptions', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.throws(SavedObjectsErrorHelpers.decorateGeneralError(error));
 
-      savedObjectsClient.create.callsFake(async () => {
-        throw new Error('foo');
+        await expect(run({ handleWriteErrors: true })).rejects.toThrowError(error);
       });
 
-      try {
-        await run({
-          onWriteError: (error: Error) => {
-            throw new Error(`${error.message} bar`);
-          },
-        });
-        throw new Error('expected run() to reject');
-      } catch (error) {
-        expect(error.message).toBe('foo bar');
-      }
-    });
+      it('throws error for all other exceptions', async () => {
+        const { run, savedObjectsClient } = setup();
+        const error = new Error('foo');
+        savedObjectsClient.create.throws(error);
 
-    it('rejects with the writeError if onWriteError() is undefined', async () => {
-      const { run, savedObjectsClient } = setup();
-
-      savedObjectsClient.create.callsFake(async () => {
-        throw new Error('foo');
+        await expect(run({ handleWriteErrors: true })).rejects.toThrowError(error);
       });
-
-      try {
-        await run({
-          onWriteError: undefined,
-        });
-        throw new Error('expected run() to reject');
-      } catch (error) {
-        expect(error.message).toBe('foo');
-      }
     });
   });
 });

src/core/server/ui_settings/create_or_upgrade_saved_config/create_or_upgrade_saved_config.ts

@@ -20,6 +20,7 @@
 import { defaults } from 'lodash';
 
 import { SavedObjectsClientContract, SavedObjectAttribute } from '../../saved_objects/types';
+import { SavedObjectsErrorHelpers } from '../../saved_objects/';
 import { Logger } from '../../logging';
 
 import { getUpgradeableConfig } from './get_upgradeable_config';
@@ -29,15 +30,13 @@ interface Options {
   version: string;
   buildNum: number;
   log: Logger;
-  onWriteError?: <T extends SavedObjectAttribute = any>(
-    error: Error,
-    attributes: Record<string, any>
-  ) => Record<string, T> | undefined;
+  handleWriteErrors: boolean;
 }
+
 export async function createOrUpgradeSavedConfig<T extends SavedObjectAttribute = any>(
   options: Options
 ): Promise<Record<string, T> | undefined> {
-  const { savedObjectsClient, version, buildNum, log, onWriteError } = options;
+  const { savedObjectsClient, version, buildNum, log, handleWriteErrors } = options;
 
   // try to find an older config we can upgrade
   const upgradeableConfig = await getUpgradeableConfig({
@@ -52,8 +51,17 @@ export async function createOrUpgradeSavedConfig<T extends SavedObjectAttribute
     // create the new SavedConfig
     await savedObjectsClient.create('config', attributes, { id: version });
   } catch (error) {
-    if (onWriteError) {
-      return onWriteError(error, attributes);
+    if (handleWriteErrors) {
+      if (SavedObjectsErrorHelpers.isConflictError(error)) {
+        return;
+      }
+
+      if (
+        SavedObjectsErrorHelpers.isNotAuthorizedError(error) ||
+        SavedObjectsErrorHelpers.isForbiddenError(error)
+      ) {
+        return attributes;
+      }
     }
 
     throw error;

src/core/server/ui_settings/create_or_upgrade_saved_config/integration_tests/create_or_upgrade.test.ts

@@ -93,6 +93,7 @@ describe('createOrUpgradeSavedConfig()', () => {
       version: '5.4.0',
       buildNum: 54099,
       log: logger,
+      handleWriteErrors: false,
     });
 
     const config540 = await savedObjectsClient.get('config', '5.4.0');
@@ -119,6 +120,7 @@ describe('createOrUpgradeSavedConfig()', () => {
       version: '5.4.1',
       buildNum: 54199,
       log: logger,
+      handleWriteErrors: false,
     });
 
     const config541 = await savedObjectsClient.get('config', '5.4.1');
@@ -145,6 +147,7 @@ describe('createOrUpgradeSavedConfig()', () => {
       version: '7.0.0-rc1',
       buildNum: 70010,
       log: logger,
+      handleWriteErrors: false,
     });
 
     const config700rc1 = await savedObjectsClient.get('config', '7.0.0-rc1');
@@ -172,6 +175,7 @@ describe('createOrUpgradeSavedConfig()', () => {
       version: '7.0.0',
       buildNum: 70099,
       log: logger,
+      handleWriteErrors: false,
     });
 
     const config700 = await savedObjectsClient.get('config', '7.0.0');
@@ -200,6 +204,7 @@ describe('createOrUpgradeSavedConfig()', () => {
       version: '6.2.3-rc1',
       buildNum: 62310,
       log: logger,
+      handleWriteErrors: false,
     });
 
     const config623rc1 = await savedObjectsClient.get('config', '6.2.3-rc1');

src/core/server/ui_settings/ui_settings_client.test.ts

@@ -120,7 +120,12 @@ describe('ui settings', () => {
 
       await uiSettings.setMany({ foo: 'bar' });
       sinon.assert.calledTwice(savedObjectsClient.update);
+
       sinon.assert.calledOnce(createOrUpgradeSavedConfig);
+      sinon.assert.calledWith(
+        createOrUpgradeSavedConfig,
+        sinon.match({ handleWriteErrors: false })
+      );
     });
 
     it('only tried to auto create once and throws NotFound', async () => {
@@ -136,6 +141,11 @@ describe('ui settings', () => {
 
       sinon.assert.calledTwice(savedObjectsClient.update);
       sinon.assert.calledOnce(createOrUpgradeSavedConfig);
+
+      sinon.assert.calledWith(
+        createOrUpgradeSavedConfig,
+        sinon.match({ handleWriteErrors: false })
+      );
     });
 
     it('throws CannotOverrideError if the key is overridden', async () => {
@@ -299,7 +309,9 @@ describe('ui settings', () => {
       expect(await uiSettings.getUserProvided()).to.eql({});
 
       sinon.assert.calledTwice(savedObjectsClient.get);
+
       sinon.assert.calledOnce(createOrUpgradeSavedConfig);
+      sinon.assert.calledWith(createOrUpgradeSavedConfig, sinon.match({ handleWriteErrors: true }));
     });
 
     it('returns result of savedConfig creation in case of notFound error', async () => {

src/core/server/ui_settings/ui_settings_client.ts

@@ -18,6 +18,7 @@
  */
 import { defaultsDeep } from 'lodash';
 
+import { SavedObjectsErrorHelpers } from '../saved_objects';
 import { SavedObjectsClientContract, SavedObjectAttribute } from '../saved_objects/types';
 import { Logger } from '../logging';
 import { createOrUpgradeSavedConfig } from './create_or_upgrade_saved_config';
@@ -163,8 +164,7 @@ export class UiSettingsClient implements IUiSettingsClient {
     try {
       await this.savedObjectsClient.update(this.type, this.id, changes);
     } catch (error) {
-      const { isNotFoundError } = this.savedObjectsClient.errors;
-      if (!isNotFoundError(error) || !autoCreateOrUpgradeIfMissing) {
+      if (!SavedObjectsErrorHelpers.isNotFoundError(error) || !autoCreateOrUpgradeIfMissing) {
         throw error;
       }
 
@@ -173,6 +173,7 @@ export class UiSettingsClient implements IUiSettingsClient {
         version: this.id,
         buildNum: this.buildNum,
         log: this.log,
+        handleWriteErrors: false,
       });
 
       await this.write({
@@ -186,37 +187,17 @@ export class UiSettingsClient implements IUiSettingsClient {
     ignore401Errors = false,
     autoCreateOrUpgradeIfMissing = true,
   }: ReadOptions = {}): Promise<Record<string, T>> {
-    const {
-      isConflictError,
-      isNotFoundError,
-      isForbiddenError,
-      isNotAuthorizedError,
-    } = this.savedObjectsClient.errors;
-
     try {
       const resp = await this.savedObjectsClient.get(this.type, this.id);
       return resp.attributes;
     } catch (error) {
-      if (isNotFoundError(error) && autoCreateOrUpgradeIfMissing) {
+      if (SavedObjectsErrorHelpers.isNotFoundError(error) && autoCreateOrUpgradeIfMissing) {
         const failedUpgradeAttributes = await createOrUpgradeSavedConfig<T>({
           savedObjectsClient: this.savedObjectsClient,
           version: this.id,
           buildNum: this.buildNum,
           log: this.log,
-          onWriteError(writeError, attributes) {
-            if (isConflictError(writeError)) {
-              // trigger `!failedUpgradeAttributes` check below, since another
-              // request caused the uiSettings object to be created so we can
-              // just re-read
-              return;
-            }
-
-            if (isNotAuthorizedError(writeError) || isForbiddenError(writeError)) {
-              return attributes;
-            }
-
-            throw writeError;
-          },
+          handleWriteErrors: true,
         });
 
         if (!failedUpgradeAttributes) {