diff --git a/package-lock.json b/package-lock.json index 85ef8f06..747477d0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "cht-user-management", - "version": "1.0.13", + "version": "1.0.14", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "cht-user-management", - "version": "1.0.13", + "version": "1.0.14", "license": "ISC", "dependencies": { "@fastify/autoload": "^5.8.0", @@ -21,6 +21,7 @@ "@types/node": "^20.8.8", "@types/uuid": "^9.0.6", "axios": "^1.5.1", + "axios-retry": "^4.0.0", "csv": "^6.3.5", "dotenv": "^16.3.1", "fastify": "^4.23.2", @@ -876,6 +877,17 @@ "proxy-from-env": "^1.1.0" } }, + "node_modules/axios-retry": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/axios-retry/-/axios-retry-4.0.0.tgz", + "integrity": "sha512-F6P4HVGITD/v4z9Lw2mIA24IabTajvpDZmKa6zq/gGwn57wN5j1P3uWrAV0+diqnW6kTM2fTqmWNfgYWGmMuiA==", + "dependencies": { + "is-retry-allowed": "^2.2.0" + }, + "peerDependencies": { + "axios": "0.x || 1.x" + } + }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -2339,6 +2351,17 @@ "node": ">=8" } }, + "node_modules/is-retry-allowed": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/is-retry-allowed/-/is-retry-allowed-2.2.0.tgz", + "integrity": "sha512-XVm7LOeLpTW4jV19QSH38vkswxoLud8sQ57YwJVTPWdiaI9I8keEhGFpBlslyVsgdQy4Opg8QOLb8YRgsyZiQg==", + "engines": { + "node": ">=10" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/is-unicode-supported": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/is-unicode-supported/-/is-unicode-supported-0.1.0.tgz", diff --git a/package.json b/package.json index 4afbec51..8089d5ce 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "cht-user-management", - "version": "1.0.13", + "version": "1.0.14", "main": "dist/index.js", "dependencies": { "@fastify/autoload": "^5.8.0", @@ -15,6 +15,7 @@ "@types/node": "^20.8.8", "@types/uuid": "^9.0.6", "axios": "^1.5.1", + "axios-retry": "^4.0.0", "csv": "^6.3.5", "dotenv": "^16.3.1", "fastify": "^4.23.2", diff --git a/scripts/retry-logic-server.js b/scripts/retry-logic-server.js new file mode 100644 index 00000000..dd53c029 --- /dev/null +++ b/scripts/retry-logic-server.js @@ -0,0 +1,49 @@ +const express = require('express'); + +const app = express(); + +// Middleware function to set timeout for all POST requests +const timeoutMiddleware = (req, res, next) => { + const TIMEOUT_DURATION = 1000; + + req.setTimeout(TIMEOUT_DURATION, () => { + // Handle timeout + res.status(408).send('Request Timeout'); + }); + + next(); +}; + +app.post('/_session', (req, res) => { + res.set('Set-Cookie', 'AuthSession=abc123'); + res.status(200).send('OK'); +}); + +app.get('/medic/_design/medic-client/_view/contacts_by_type_freetext', (req, res) => { + const startkey = JSON.parse(req.query.startkey); + console.log('contacts_by_type_freetext', startkey); + const DATA = [ + // eslint-disable-next-line max-len + { id: 'e847f6e2-6dba-46dd-8128-5b153d0cd75f', key: ['b_sub_county', 'name:malava'], value: 'false false b_sub_county malava', doc: { _id: 'e847f6e2-6dba-46dd-8128-5b153d0cd75f', _rev: '1-cd20b7095c20172237867233b0375eda', parent: { _id: '95d9abd1-7c17-41b1-af98-595509f96631' }, type: 'contact', is_name_generated: 'false', name: 'Malava', external_id: '', contact: { _id: '1e3d8375-6ab4-4409-be3f-3324db7658e9' }, contact_type: 'b_sub_county', reported_date: 1702573623984 } }, + // eslint-disable-next-line max-len + { id: '2926bf4c-63eb-433d-a2b4-274fd05d2f1c', key: ['c_community_health_unit', 'name:chu'], value: 'false false c_community_health_unit chu', doc: { _id: '2926bf4c-63eb-433d-a2b4-274fd05d2f1c', _rev: '1-c15f26fe064f8357c19d1124286bf4c4', name: 'Chu', PARENT: 'Chepalungu', code: '123456', type: 'contact', contact_type: 'c_community_health_unit', parent: { _id: 'e847f6e2-6dba-46dd-8128-5b153d0cd75f', parent: { _id: '95d9abd1-7c17-41b1-af98-595509f96631' } }, contact: { _id: 'bb9ebc4c6af161ee0f53b42339001fb1' }, reported_date: 1701631255451 } }, + ]; + res.json({ + total_rows: 2, + offset: 0, + rows: DATA.filter(r => r.key[0] === startkey[0]) + }); +}); + +app.use(timeoutMiddleware); + +app.all('*', (req, res) => { + setTimeout(() => { + res.status(200).send('OK'); + }, 2000); +}); + +// Start the server +app.listen(3556, () => { + console.log(`Server is listening on port ${3556}`); +}); diff --git a/src/lib/cht-api.ts b/src/lib/cht-api.ts index 96df8209..9762a4cd 100644 --- a/src/lib/cht-api.ts +++ b/src/lib/cht-api.ts @@ -1,8 +1,12 @@ import _ from 'lodash'; import axios, { AxiosHeaders } from 'axios'; +import axiosRetry from 'axios-retry'; +import { axiosRetryConfig } from './retry-logic'; import { UserPayload } from '../services/user-payload'; import { AuthenticationInfo, Config, ContactType } from '../config'; +axiosRetry(axios, axiosRetryConfig); + const { NODE_ENV } = process.env; @@ -164,7 +168,11 @@ export class ChtApi { createUser = async (user: UserPayload): Promise => { const url = `${this.protocolAndHost}/api/v1/users`; console.log('axios.post', url); - await axios.post(url, user, this.authorizationOptions()); + const axiosRequestionConfig = { + ...this.authorizationOptions(), + 'axios-retry': { retries: 0 }, // upload-manager handles retries for this + }; + await axios.post(url, user, axiosRequestionConfig); }; getParentAndSibling = async (parentId: string, contactType: ContactType): Promise<{ parent: any; sibling: any }> => { diff --git a/src/lib/retry-logic.ts b/src/lib/retry-logic.ts new file mode 100644 index 00000000..17849357 --- /dev/null +++ b/src/lib/retry-logic.ts @@ -0,0 +1,72 @@ +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; +const RETRYABLE_STATUS_CODES = [500, 502, 503, 504, 511]; + +export const axiosRetryConfig = { + retries: RETRY_COUNT, + retryDelay: () => 1000, + retryCondition: (err: AxiosError) => { + const status = err.response?.status; + return (!status || RETRYABLE_STATUS_CODES.includes(status)) && isRetryAllowed(err); + }, + onRetry: (retryCount: number, error: AxiosError, requestConfig: AxiosRequestConfig) => { + console.log(`${requestConfig.url} failure. Retrying (${retryCount})`); + }, +}; + +export async function retryOnUpdateConflict(funcWithGetAndPut: () => Promise): Promise { + for (let retryCount = 0; retryCount < RETRY_COUNT; retryCount++) { + try { + return await funcWithGetAndPut(); + } 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; + } + + // no idea when/why some instances yield "response.data" as JSON vs some as string + const errorMessage = err.response?.data?.error?.message || err.response?.data; + console.error('createUser retry because', errorMessage); + if (errorMessage.includes('already taken.')) { + userPayload.makeUsernameMoreComplex(); + continue; + } + + const RETRY_PASSWORD_STRINGS = ['The password must be at least', 'The password is too easy to guess.']; + if (RETRY_PASSWORD_STRINGS.find(str => errorMessage.includes(str))) { + userPayload.regeneratePassword(); + continue; + } + + throw err; + } + } + + throw new Error('could not create user ' + userPayload.contact); +} diff --git a/src/services/upload-manager.ts b/src/services/upload-manager.ts index 340fd406..2d108039 100644 --- a/src/services/upload-manager.ts +++ b/src/services/upload-manager.ts @@ -1,13 +1,14 @@ import EventEmitter from 'events'; -import { ChtApi, PlacePayload } from '../lib/cht-api'; +import * as RetryLogic from '../lib/retry-logic'; +import { ChtApi, PlacePayload } from '../lib/cht-api'; +import { Config } from '../config'; import Place, { PlaceUploadState } from './place'; -import { UserPayload } from './user-payload'; +import RemotePlaceCache from '../lib/remote-place-cache'; import SessionCache, { SessionCacheUploadState } from './session-cache'; -import { UploadReplacementPlace } from './upload.replacement'; import { UploadNewPlace } from './upload.new'; -import { Config } from '../config'; -import RemotePlaceCache from '../lib/remote-place-cache'; +import { UploadReplacementPlace } from './upload.replacement'; +import { UserPayload } from './user-payload'; const UPLOAD_BATCH_SIZE = 10; @@ -54,7 +55,8 @@ 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(() => uploader.linkContactAndPlace(place, createdPlaceId)); if (!place.creationDetails.contactId) { throw Error('creationDetails.contactId not set'); @@ -62,7 +64,7 @@ export class UploadManager extends EventEmitter { 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; } @@ -73,7 +75,7 @@ export class UploadManager extends EventEmitter { console.log(`successfully created ${JSON.stringify(place.creationDetails)}`); this.eventedPlaceStateChange(place, PlaceUploadState.SUCCESS); } catch (err: any) { - const errorDetails = err.response?.data.error || err.toString(); + const errorDetails = err.response?.data?.error ? JSON.stringify(err.response.data.error) : err.toString(); console.log('error when creating user', errorDetails); place.uploadError = errorDetails; this.eventedPlaceStateChange(place, PlaceUploadState.FAILURE); @@ -100,32 +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 < 5; ++retryCount) { - try { - await chtApi.createUser(userPayload); - return userPayload; - } catch (err: any) { - if (err?.response?.status !== 400) { - throw err; - } - - const msg = err.response?.data?.error?.message || err.response?.data; - console.error('createUser retry because', msg); - if (msg?.includes('already taken')) { - userPayload.makeUsernameMoreComplex(); - continue; - } - - if (msg?.includes('password')) { // password too easy to guess - userPayload.regeneratePassword(); - continue; - } - - throw err; - } - } - - throw new Error('could not create user ' + userPayload.contact); -} diff --git a/src/services/upload.replacement.ts b/src/services/upload.replacement.ts index fb4e7dcd..524f64c6 100644 --- a/src/services/upload.replacement.ts +++ b/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(() => this.chtApi.updatePlace(payload, contactId)); const disabledUsers = await this.chtApi.disableUsersWithPlace(placeId); place.creationDetails.disabledUsers = disabledUsers; diff --git a/test/e2e-request.spec.ts b/test/e2e-request.spec.ts deleted file mode 100644 index 0f81fef9..00000000 --- a/test/e2e-request.spec.ts +++ /dev/null @@ -1,5 +0,0 @@ -describe('request e2e tests', () => { - it('given requests, assert the payload sent to CHT', async () => { - - }); -}); diff --git a/test/lib/retry-logic.spec.ts b/test/lib/retry-logic.spec.ts new file mode 100644 index 00000000..d4b0b0b9 --- /dev/null +++ b/test/lib/retry-logic.spec.ts @@ -0,0 +1,137 @@ +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'; +import Place from '../../src/services/place'; +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 (json)', + 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' + }, + { + desc: 'password too weak (string)', + axiosError: { + code: 'ERR_BAD_REQUEST', + response: { + status: 400, + data: 'The password is too easy to guess. Include a range of types of characters to increase the score.', + } + }, + 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(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(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); + } + }); + } + }); +}); diff --git a/test/services/upload-manager.spec.ts b/test/services/upload-manager.spec.ts index 026b3976..d8e94273 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -10,6 +10,7 @@ import { ChtApi, RemotePlace } from '../../src/lib/cht-api'; import RemotePlaceCache from '../../src/lib/remote-place-cache'; import { Config } from '../../src/config'; import RemotePlaceResolver from '../../src/lib/remote-place-resolver'; +import { UploadManagerRetryScenario } from '../lib/retry-logic.spec'; describe('upload-manager.ts', () => { beforeEach(() => { @@ -195,7 +196,7 @@ describe('upload-manager.ts', () => { const { remotePlace, sessionCache, chtApi } = await createMocks(); chtApi.createUser - .throws('timeout') + .throws({ response: { status: 404 }, toString: () => 'upload-error' }) .onSecondCall().resolves(); const chu_name = 'new chu'; @@ -204,7 +205,8 @@ describe('upload-manager.ts', () => { const uploadManager = new UploadManager(); await uploadManager.doUpload(sessionCache.getPlaces(), chtApi); expect(chu.isCreated).to.be.false; - expect(chu.uploadError).to.include('timeout'); + expect(chtApi.createUser.calledOnce).to.be.true; + expect(chu.uploadError).to.include('upload-error'); expect(chu.creationDetails).to.deep.eq({ contactId: 'created-contact-id', placeId: 'created-place-id', @@ -228,6 +230,21 @@ describe('upload-manager.ts', () => { expect(chtApi.updatePlace.called).to.be.false; expect(chtApi.disableUsersWithPlace.called).to.be.false; }); + + it(`createUser is retried`, async() => { + const { remotePlace, sessionCache, chtApi } = await createMocks(); + + chtApi.createUser.throws(UploadManagerRetryScenario.axiosError); + + const chu_name = 'new chu'; + const chu = await createChu(remotePlace, chu_name, sessionCache, chtApi); + + const uploadManager = new UploadManager(); + await uploadManager.doUpload(sessionCache.getPlaces(), chtApi); + expect(chu.isCreated).to.be.false; + expect(chtApi.createUser.callCount).to.be.gt(2); // retried + expect(chu.uploadError).to.include('could not create user'); + }); }); async function createChu(remotePlace: RemotePlace, chu_name: string, sessionCache: any, chtApi: ChtApi) { @@ -277,4 +294,3 @@ async function createMocks() { return { fakeFormData, contactType, sessionCache, chtApi, remotePlace }; } -