Basically rewrite after testing update-conflict

src/lib/cht-api.ts

@@ -1,7 +1,7 @@
 import _ from 'lodash';
 import axios, { AxiosHeaders } from 'axios';
 import axiosRetry from 'axios-retry';
-import axiosRetryConfig from './axios-retry-config';
+import { axiosRetryConfig } from './retry-logic';
 import { UserPayload } from '../services/user-payload';
 import { AuthenticationInfo, Config, ContactType } from '../config';
 

src/lib/retry-logic.ts

@@ -0,0 +1,70 @@
+import { AxiosError, AxiosRequestConfig } from 'axios';
+import isRetryAllowed from 'is-retry-allowed';
+import { UserPayload } from '../services/user-payload';
+import { ChtApi } from './cht-api';
+
+const RETRY_COUNT = 4;
+
+export const axiosRetryConfig = {
+  retries: RETRY_COUNT,
+  retryDelay: () => 1000,
+  retryCondition: (err: AxiosError) => {
+    const status = err.response?.status;
+    return (!status || status >= 500) && isRetryAllowed(err);
+  },
+  onRetry: (retryCount: number, error: AxiosError, requestConfig: AxiosRequestConfig) => {
+    console.log(`${requestConfig.url} failure. Retrying (${retryCount})`);
+  },
+};
+
+export async function retryOnUpdateConflict<T>(funcWithPut: () => Promise<T>): Promise<T> {
+  for (let retryCount = 0; retryCount < RETRY_COUNT; retryCount++) {
+    try {
+      return await funcWithPut();
+    } catch (err : any) {
+      const statusCode = err.response?.status;
+      if (statusCode === 409) {
+        console.log(`Retrying on update-conflict (${retryCount})`);
+        continue;
+      }
+
+      throw err;
+    }
+  }
+
+  throw Error('update-conflict 409 persisted');
+}
+
+export async function createUserWithRetries(userPayload: UserPayload, chtApi: ChtApi): Promise<{ username: string; password: string }> {
+  for (let retryCount = 0; retryCount < RETRY_COUNT; ++retryCount) {
+    try {
+      await chtApi.createUser(userPayload);
+      return userPayload;
+    } catch (err: any) {      
+      if (axiosRetryConfig.retryCondition(err)) {
+        continue;
+      }
+
+      if (err.response?.status !== 400) {
+        throw err;
+      }
+
+      const translationKey = err.response?.data?.error?.translationKey;
+      console.error('createUser retry because', translationKey);
+      if (translationKey === 'username.taken') {
+        userPayload.makeUsernameMoreComplex();
+        continue;
+      }
+
+      const RETRY_PASSWORD_TRANSLATIONS = ['password.length.minimum', 'password.weak'];
+      if (RETRY_PASSWORD_TRANSLATIONS.includes(translationKey)) {
+        userPayload.regeneratePassword();
+        continue;
+      }
+
+      throw err;
+    }
+  }
+
+  throw new Error('could not create user ' + userPayload.contact);
+}

src/services/upload-manager.ts

@@ -1,6 +1,6 @@
 import EventEmitter from 'events';
 
-import axiosRetryConfig from '../lib/axios-retry-config';
+import * as RetryLogic from '../lib/retry-logic';
 import { ChtApi, PlacePayload } from '../lib/cht-api';
 import { Config } from '../config';
 import Place, { PlaceUploadState } from './place';
@@ -11,7 +11,6 @@ import { UploadReplacementPlace } from './upload.replacement';
 import { UserPayload } from './user-payload';
 
 const UPLOAD_BATCH_SIZE = 10;
-const RETRY_COUNT = 5;
 
 export interface Uploader {
    handleContact (payload: PlacePayload): Promise<string | undefined>;
@@ -56,15 +55,16 @@ export class UploadManager extends EventEmitter {
         place.creationDetails.placeId = placeId;
       }
 
-      await uploader.linkContactAndPlace(place, place.creationDetails?.placeId);
+      const createdPlaceId = place.creationDetails.placeId; // closure required for typescript
+      await RetryLogic.retryOnUpdateConflict<void>(() => uploader.linkContactAndPlace(place, createdPlaceId));
 
       if (!place.creationDetails.contactId) {
         throw Error('creationDetails.contactId not set');
       }
 
       if (!place.creationDetails.username) {
         const userPayload = new UserPayload(place, place.creationDetails.placeId, place.creationDetails.contactId);
-        const { username, password } = await tryCreateUser(userPayload, chtApi);
+        const { username, password } = await RetryLogic.createUserWithRetries(userPayload, chtApi);
         place.creationDetails.username = username;
         place.creationDetails.password = password;
       }
@@ -102,37 +102,3 @@ export class UploadManager extends EventEmitter {
     this.emit('places_state_change', state);
   };
 }
-
-async function tryCreateUser (userPayload: UserPayload, chtApi: ChtApi): Promise<{ username: string; password: string }> {
-  for (let retryCount = 0; retryCount < RETRY_COUNT; ++retryCount) {
-    try {
-      await chtApi.createUser(userPayload);
-      return userPayload;
-    } catch (err: any) {      
-      if (axiosRetryConfig.retryCondition(err)) {
-        continue;
-      }
-
-      if (err.response?.status !== 400) {
-        throw err;
-      }
-
-      const translationKey = err.response?.data?.error?.translationKey;
-      console.error('createUser retry because', translationKey);
-      if (translationKey === 'username.taken') {
-        userPayload.makeUsernameMoreComplex();
-        continue;
-      }
-
-      const RETRY_PASSWORD_TRANSLATIONS = ['password.length.minimum', 'password.weak'];
-      if (RETRY_PASSWORD_TRANSLATIONS.includes(translationKey)) {
-        userPayload.regeneratePassword();
-        continue;
-      }
-
-      throw err;
-    }
-  }
-
-  throw new Error('could not create user ' + userPayload.contact);
-}

src/services/upload.replacement.ts

@@ -1,5 +1,6 @@
 import { ChtApi, PlacePayload } from '../lib/cht-api';
 import Place from './place';
+import { retryOnUpdateConflict } from '../lib/retry-logic';
 import { Uploader } from './upload-manager';
 
 export class UploadReplacementPlace implements Uploader {
@@ -21,7 +22,7 @@ export class UploadReplacementPlace implements Uploader {
       throw Error('contactId and placeId are required');
     }
 
-    const updatedPlaceId = await this.chtApi.updatePlace(payload, contactId);
+    const updatedPlaceId = await retryOnUpdateConflict<string>(() => this.chtApi.updatePlace(payload, contactId));
     const disabledUsers = await this.chtApi.disableUsersWithPlace(placeId);
     place.creationDetails.disabledUsers = disabledUsers;
 

test/lib/retry-logic.spec.ts

@@ -0,0 +1,125 @@
+import Chai from 'chai';
+import sinon from 'sinon';
+
+import * as RetryLogic from '../../src/lib/retry-logic';
+import { UserPayload } from '../../src/services/user-payload';
+import { ChtApi } from '../../src/lib/cht-api';
+import { mockSimpleContactType } from '../mocks';
+
+import chaiAsPromised from 'chai-as-promised';
+Chai.use(chaiAsPromised);
+
+const { expect } = Chai;
+
+const RetryScenarios = [
+  { desc: '503', axiosError: { response: { status: 503 } }, retry: 'axios' },
+  { desc: 'axios timeout', axiosError: { code: 'ECONNABORTED' }, retry: 'axios' },
+  { desc: 'service timeout', axiosError: { code: 'ECONNRESET' }, retry: 'axios' },
+
+  { desc: 'update conflict', axiosError: { code: 'ERR_BAD_REQUEST', response: { status: 409 } }, retry: 'update-conflict' },
+
+  {
+    desc: 'username taken is not retried', 
+    axiosError: { 
+      code: 'ERR_BAD_REQUEST', 
+      response: {
+        status: 400,
+        data: { error: { message: 'Username "chu" already taken.', translationKey: 'username.taken' } },
+      } 
+    },
+    retry: 'upload-manager'
+  },
+  {
+    desc: 'password too short', 
+    axiosError: { 
+      code: 'ERR_BAD_REQUEST', 
+      response: {
+        status: 400,
+        data: { error: { message: 'The password must be at least 8 characters long.', translationKey: 'password.length.minimum' } },
+      } 
+    },
+    retry: 'upload-manager'
+  },
+  {
+    desc: 'password too weak', 
+    axiosError: { 
+      code: 'ERR_BAD_REQUEST', 
+      response: {
+        status: 400,
+        data: {
+          error: {
+            message: 'The password is too easy to guess. Include a range of types of characters to increase the score.',
+            translationKey: 'password.weak'
+          }
+        },
+      } 
+    },
+    retry: 'upload-manager'
+  },
+];
+
+export const UploadManagerRetryScenario = RetryScenarios[RetryScenarios.length - 1];
+const UpdateConflictScenario = RetryScenarios.find(s => s.retry === 'update-conflict');
+
+describe('lib/retry-logic', () => {
+  describe('axiosRetryConfig', () => {
+    for (const scenario of RetryScenarios) {
+      it(scenario.desc, () => {
+        const doRetry = RetryLogic.axiosRetryConfig.retryCondition(scenario.axiosError as any);
+        expect(doRetry).to.eq(scenario.retry === 'axios');
+      });
+    }
+  });
+
+  describe('retryOnUpdateConflict', () => {
+    for (const scenario of RetryScenarios) {
+      it(scenario.desc, async () => {
+        const output = 'foo';
+        const testFunction = sinon.stub()
+          .rejects(scenario.axiosError)
+          .onSecondCall().resolves(output);
+        const execute = RetryLogic.retryOnUpdateConflict<string>(testFunction);
+        const expectRetry = scenario.retry === 'update-conflict';
+        if (expectRetry) {
+          await expect(execute).to.eventually.eq(output);
+          expect(testFunction.callCount).to.eq(expectRetry ? 2 : 1);
+        } else {
+          await expect(execute).to.eventually.be.rejectedWith(scenario.axiosError);
+        }
+      });
+    }
+
+    it ('throws after persistent conflict', async () => {
+      const testFunction = sinon.stub().rejects(UpdateConflictScenario?.axiosError);
+      const execute = RetryLogic.retryOnUpdateConflict<string>(testFunction);
+      await expect(execute).to.eventually.be.rejectedWith('persisted');
+      expect(testFunction.callCount).to.eq(4);
+    });
+  });
+
+  describe('createUserWithRetries', () => {
+    for (const scenario of RetryScenarios) {
+      it(scenario.desc, async() => {
+        const chtApi = {
+          createUser: sinon.stub().throws(scenario.axiosError),
+        };
+        const place = {
+          generateUsername: sinon.stub().returns('username'),
+          type: mockSimpleContactType('string', 'bar'),
+          contact: { properties: {} },
+        };
+        const userPayload = new UserPayload(place as Place, 'place_id', 'contact_id');
+
+        const execute = RetryLogic.createUserWithRetries(userPayload as UserPayload, chtApi as ChtApi);
+        const expectRetry = ['upload-manager', 'axios'].includes(scenario.retry);
+        if (expectRetry) {
+          await expect(execute).to.eventually.be.rejectedWith('could not create user');
+          expect(chtApi.createUser.callCount).to.eq(4);
+        } else {
+          await expect(execute).to.eventually.be.rejectedWith(scenario.axiosError);
+          expect(chtApi.createUser.callCount).to.eq(1);
+        }
+      });
+    }
+  });
+});