From c953fbeb150b811a87d20e1a8e188818ee40dccd Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Fri, 9 Feb 2024 15:55:52 +0700 Subject: [PATCH 1/6] Quick prototype --- package-lock.json | 23 +++++++++++++++++++ package.json | 1 + scripts/retry-logic-server.js | 34 ++++++++++++++++++++++++++++ src/lib/axios-retry-config.ts | 13 +++++++++++ src/lib/cht-api.ts | 10 +++++++- src/services/upload-manager.ts | 17 +++++++++----- test/services/upload-manager.spec.ts | 4 ++-- 7 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 scripts/retry-logic-server.js create mode 100644 src/lib/axios-retry-config.ts diff --git a/package-lock.json b/package-lock.json index 11aa4583..3d071adf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,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", @@ -868,6 +869,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", @@ -2321,6 +2333,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 87a09b47..971b5188 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,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..7af90dcb --- /dev/null +++ b/scripts/retry-logic-server.js @@ -0,0 +1,34 @@ +const express = require('express'); + +const app = express(); + +// Middleware function to set timeout for all POST requests +const timeoutMiddleware = (req, res, next) => { + // Set timeout duration (in milliseconds) + const TIMEOUT_DURATION = 5000; // 5 seconds + + // Set timeout for this request + req.setTimeout(TIMEOUT_DURATION, () => { + // Handle timeout + res.status(408).send('Request Timeout'); + }); + + // Continue to next middleware + next(); +}; + +// Apply timeout middleware for all routes +app.use(timeoutMiddleware); + +// Route for handling POST requests +app.all('*', (req, res) => { + // Simulate long processing time to trigger timeout + setTimeout(() => { + res.send('POST request handled successfully'); + }, 6000); // This will exceed the timeout and trigger a timeout response +}); + +// Start the server +app.listen(3556, () => { + console.log(`Server is listening on port ${3556}`); +}); diff --git a/src/lib/axios-retry-config.ts b/src/lib/axios-retry-config.ts new file mode 100644 index 00000000..a59cb090 --- /dev/null +++ b/src/lib/axios-retry-config.ts @@ -0,0 +1,13 @@ +import { AxiosError } from 'axios'; +import isRetryAllowed from 'is-retry-allowed'; + +const axiosRetryConfig = { + retries: 5, + retryDelay: () => 1000, + retryCondition: isRetryAllowed, + onRetry: (retryCount: number, error: AxiosError) => { + console.log(`${retryCount} retry for ${error.request.url}`); + }, +}; + +export default axiosRetryConfig; \ No newline at end of file diff --git a/src/lib/cht-api.ts b/src/lib/cht-api.ts index 96df8209..d6e18313 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 './axios-retry-config'; 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/services/upload-manager.ts b/src/services/upload-manager.ts index bf372b62..e57f0642 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 axiosRetryConfig from '../lib/axios-retry-config'; +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; @@ -108,7 +109,11 @@ async function tryCreateUser (userPayload: UserPayload, chtApi: ChtApi): Promise return userPayload; } catch (err: any) { if (err?.response?.status !== 400) { - throw err; + if (axiosRetryConfig.retryCondition(err)) { + continue; + } else { + throw err; + } } const msg = err.response?.data?.error?.message || err.response?.data; diff --git a/test/services/upload-manager.spec.ts b/test/services/upload-manager.spec.ts index 026b3976..4823fee2 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -195,7 +195,7 @@ describe('upload-manager.ts', () => { const { remotePlace, sessionCache, chtApi } = await createMocks(); chtApi.createUser - .throws('timeout') + .throws({ code: 'CERT_REJECTED', toString: () => 'upload-error' }) .onSecondCall().resolves(); const chu_name = 'new chu'; @@ -204,7 +204,7 @@ 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(chu.uploadError).to.include('upload-error'); expect(chu.creationDetails).to.deep.eq({ contactId: 'created-contact-id', placeId: 'created-place-id', From 631b61708d1d71182e436b0ac3df03c89ac35f60 Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Sun, 11 Feb 2024 11:16:09 +0700 Subject: [PATCH 2/6] Test end to end. upload-manager tests --- scripts/retry-logic-server.js | 31 ++++++++++++----- src/lib/axios-retry-config.ts | 13 ++++--- src/services/upload-manager.ts | 26 +++++++------- test/e2e-request.spec.ts | 5 --- test/lib/axios-retry-config.spec.ts | 26 ++++++++++++++ test/services/upload-manager.spec.ts | 51 +++++++++++++++++++++++++++- 6 files changed, 120 insertions(+), 32 deletions(-) delete mode 100644 test/e2e-request.spec.ts create mode 100644 test/lib/axios-retry-config.spec.ts diff --git a/scripts/retry-logic-server.js b/scripts/retry-logic-server.js index 7af90dcb..1c6ca1cb 100644 --- a/scripts/retry-logic-server.js +++ b/scripts/retry-logic-server.js @@ -4,28 +4,41 @@ const app = express(); // Middleware function to set timeout for all POST requests const timeoutMiddleware = (req, res, next) => { - // Set timeout duration (in milliseconds) - const TIMEOUT_DURATION = 5000; // 5 seconds + const TIMEOUT_DURATION = 1000; - // Set timeout for this request req.setTimeout(TIMEOUT_DURATION, () => { // Handle timeout res.status(408).send('Request Timeout'); }); - // Continue to next middleware next(); }; -// Apply timeout middleware for all routes +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 = [ + {"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"},"geolocation":"","meta":{"created_by":"medic","created_by_person_uuid":"","created_by_place_uuid":""},"contact_type":"b_sub_county","reported_date":1702573623984,"form_version":{"time":1700734737921,"sha256":"755eac18545f4282c2a3e04c6a8857a2c4d5142f36ed86f5c93b3228ec6fad7a"}}}, + {"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); -// Route for handling POST requests app.all('*', (req, res) => { - // Simulate long processing time to trigger timeout setTimeout(() => { - res.send('POST request handled successfully'); - }, 6000); // This will exceed the timeout and trigger a timeout response + res.status(200).send('OK'); + }, 2000); }); // Start the server diff --git a/src/lib/axios-retry-config.ts b/src/lib/axios-retry-config.ts index a59cb090..b5858fc7 100644 --- a/src/lib/axios-retry-config.ts +++ b/src/lib/axios-retry-config.ts @@ -1,12 +1,15 @@ -import { AxiosError } from 'axios'; +import { AxiosError, AxiosRequestConfig } from 'axios'; import isRetryAllowed from 'is-retry-allowed'; const axiosRetryConfig = { - retries: 5, + retries: 3, retryDelay: () => 1000, - retryCondition: isRetryAllowed, - onRetry: (retryCount: number, error: AxiosError) => { - console.log(`${retryCount} retry for ${error.request.url}`); + retryCondition: (err: AxiosError) => { + const status = err.response?.status; + return (!status || status === 409 || status >= 500) && isRetryAllowed(err); + }, + onRetry: (retryCount: number, error: AxiosError, requestConfig: AxiosRequestConfig) => { + console.log(`${requestConfig.url} failure. Retrying (${retryCount})`); }, }; diff --git a/src/services/upload-manager.ts b/src/services/upload-manager.ts index e57f0642..48c70848 100644 --- a/src/services/upload-manager.ts +++ b/src/services/upload-manager.ts @@ -11,6 +11,7 @@ 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; @@ -74,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.toString() || err.toString(); + const errorDetails = err.response?.data ? JSON.stringify(err.response?.data) : err.toString(); console.log('error when creating user', errorDetails); place.uploadError = errorDetails; this.eventedPlaceStateChange(place, PlaceUploadState.FAILURE); @@ -103,27 +104,28 @@ export class UploadManager extends EventEmitter { } async function tryCreateUser (userPayload: UserPayload, chtApi: ChtApi): Promise<{ username: string; password: string }> { - for (let retryCount = 0; retryCount < 5; ++retryCount) { + for (let retryCount = 0; retryCount < RETRY_COUNT; ++retryCount) { try { await chtApi.createUser(userPayload); return userPayload; } catch (err: any) { - if (err?.response?.status !== 400) { - if (axiosRetryConfig.retryCondition(err)) { - continue; - } else { - throw err; - } + if (axiosRetryConfig.retryCondition(err)) { + continue; } - const msg = err.response?.data?.error?.message || err.response?.data; - console.error('createUser retry because', msg); - if (msg?.includes('already taken')) { + 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; } - if (msg?.includes('password')) { // password too easy to guess + const RETRY_PASSWORD_TRANSLATIONS = ['password.length.minimum', 'password.weak']; + if (RETRY_PASSWORD_TRANSLATIONS.includes(translationKey)) { userPayload.regeneratePassword(); continue; } 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/axios-retry-config.spec.ts b/test/lib/axios-retry-config.spec.ts new file mode 100644 index 00000000..b2723206 --- /dev/null +++ b/test/lib/axios-retry-config.spec.ts @@ -0,0 +1,26 @@ +import { expect } from 'chai'; +import AxiosRetryConfig from '../../src/lib/axios-retry-config'; +import { UploadManagerRetryScenarios } from '../services/upload-manager.spec'; + +const scenarios = [ + { desc: '503', axiosError: { response: { status: 503 } } }, + { desc: 'axios timeout', axiosError: { code: 'ECONNABORTED' } }, + { desc: 'service timeout', axiosError: { code: 'ECONNRESET' } }, + { desc: 'update conflict', axiosError: { code: 'ERR_BAD_REQUEST', response: { status: 409 } } }, +]; + +describe('lib/axios-retry-config', () => { + for (const scenario of scenarios) { + it(scenario.desc, () => { + const doRetry = AxiosRetryConfig.retryCondition(scenario.axiosError as any); + expect(doRetry).to.eq(true); + }); + } + + for (const scenario of UploadManagerRetryScenarios) { + it(scenario.desc, () => { + const doRetry = AxiosRetryConfig.retryCondition(scenario.axiosError as any); + expect(doRetry).to.eq(false); + }); + } +}); diff --git a/test/services/upload-manager.spec.ts b/test/services/upload-manager.spec.ts index 4823fee2..4c5ffa59 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -11,6 +11,39 @@ import RemotePlaceCache from '../../src/lib/remote-place-cache'; import { Config } from '../../src/config'; import RemotePlaceResolver from '../../src/lib/remote-place-resolver'; +export const UploadManagerRetryScenarios = [ + { + desc: 'username taken is not retried', + axiosError: { + code: 'ERR_BAD_REQUEST', + response: { + status: 400, + data: { error: { message: 'Username "chu" already taken.', translationKey: 'username.taken' } }, + } + }, + }, + { + 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' } }, + } + }, + }, + { + 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' } }, + } + }, + }, +]; + describe('upload-manager.ts', () => { beforeEach(() => { RemotePlaceCache.clear({}); @@ -228,6 +261,23 @@ describe('upload-manager.ts', () => { expect(chtApi.updatePlace.called).to.be.false; expect(chtApi.disableUsersWithPlace.called).to.be.false; }); + + for (const scenario of UploadManagerRetryScenarios) { + it(`retry: ${scenario.desc}`, async() => { + const { remotePlace, sessionCache, chtApi } = await createMocks(); + + chtApi.createUser.throws(scenario.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 +327,3 @@ async function createMocks() { return { fakeFormData, contactType, sessionCache, chtApi, remotePlace }; } - From 2712be2517888d081138a520f83722bcfaa7a1b5 Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Sun, 11 Feb 2024 11:19:07 +0700 Subject: [PATCH 3/6] Test for no-retry --- test/services/upload-manager.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/services/upload-manager.spec.ts b/test/services/upload-manager.spec.ts index 4c5ffa59..0585826a 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -228,7 +228,7 @@ describe('upload-manager.ts', () => { const { remotePlace, sessionCache, chtApi } = await createMocks(); chtApi.createUser - .throws({ code: 'CERT_REJECTED', toString: () => 'upload-error' }) + .throws({ response: { status: 404 }, toString: () => 'upload-error' }) .onSecondCall().resolves(); const chu_name = 'new chu'; @@ -237,6 +237,7 @@ describe('upload-manager.ts', () => { const uploadManager = new UploadManager(); await uploadManager.doUpload(sessionCache.getPlaces(), chtApi); expect(chu.isCreated).to.be.false; + expect(chtApi.createUser.calledOnce).to.be.true; expect(chu.uploadError).to.include('upload-error'); expect(chu.creationDetails).to.deep.eq({ contactId: 'created-contact-id', From ed50b0346644ea4765b85e2966b92787d9ccdb97 Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Sun, 11 Feb 2024 11:26:53 +0700 Subject: [PATCH 4/6] Fix eslint --- scripts/retry-logic-server.js | 12 +++++++----- src/lib/axios-retry-config.ts | 2 +- test/services/upload-manager.spec.ts | 7 ++++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/retry-logic-server.js b/scripts/retry-logic-server.js index 1c6ca1cb..dd53c029 100644 --- a/scripts/retry-logic-server.js +++ b/scripts/retry-logic-server.js @@ -23,13 +23,15 @@ 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 = [ - {"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"},"geolocation":"","meta":{"created_by":"medic","created_by_person_uuid":"","created_by_place_uuid":""},"contact_type":"b_sub_county","reported_date":1702573623984,"form_version":{"time":1700734737921,"sha256":"755eac18545f4282c2a3e04c6a8857a2c4d5142f36ed86f5c93b3228ec6fad7a"}}}, - {"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}}, + // 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]) + total_rows: 2, + offset: 0, + rows: DATA.filter(r => r.key[0] === startkey[0]) }); }); diff --git a/src/lib/axios-retry-config.ts b/src/lib/axios-retry-config.ts index b5858fc7..bb60aee5 100644 --- a/src/lib/axios-retry-config.ts +++ b/src/lib/axios-retry-config.ts @@ -13,4 +13,4 @@ const axiosRetryConfig = { }, }; -export default axiosRetryConfig; \ No newline at end of file +export default axiosRetryConfig; diff --git a/test/services/upload-manager.spec.ts b/test/services/upload-manager.spec.ts index 0585826a..8d614527 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -38,7 +38,12 @@ export const UploadManagerRetryScenarios = [ 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' } }, + data: { + error: { + message: 'The password is too easy to guess. Include a range of types of characters to increase the score.', + translationKey: 'password.weak' + } + }, } }, }, From 974d5674ecd1aa04262c1f9d8bc3da6b1c606b8e Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Mon, 12 Feb 2024 11:55:33 +0700 Subject: [PATCH 5/6] Basically rewrite after testing update-conflict --- src/lib/axios-retry-config.ts | 16 ---- src/lib/cht-api.ts | 2 +- src/lib/retry-logic.ts | 70 +++++++++++++++ src/services/upload-manager.ts | 42 +-------- src/services/upload.replacement.ts | 3 +- test/lib/axios-retry-config.spec.ts | 26 ------ test/lib/retry-logic.spec.ts | 125 +++++++++++++++++++++++++++ test/services/upload-manager.spec.ts | 67 +++----------- 8 files changed, 216 insertions(+), 135 deletions(-) delete mode 100644 src/lib/axios-retry-config.ts create mode 100644 src/lib/retry-logic.ts delete mode 100644 test/lib/axios-retry-config.spec.ts create mode 100644 test/lib/retry-logic.spec.ts diff --git a/src/lib/axios-retry-config.ts b/src/lib/axios-retry-config.ts deleted file mode 100644 index bb60aee5..00000000 --- a/src/lib/axios-retry-config.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { AxiosError, AxiosRequestConfig } from 'axios'; -import isRetryAllowed from 'is-retry-allowed'; - -const axiosRetryConfig = { - retries: 3, - retryDelay: () => 1000, - retryCondition: (err: AxiosError) => { - const status = err.response?.status; - return (!status || status === 409 || status >= 500) && isRetryAllowed(err); - }, - onRetry: (retryCount: number, error: AxiosError, requestConfig: AxiosRequestConfig) => { - console.log(`${requestConfig.url} failure. Retrying (${retryCount})`); - }, -}; - -export default axiosRetryConfig; diff --git a/src/lib/cht-api.ts b/src/lib/cht-api.ts index d6e18313..9762a4cd 100644 --- a/src/lib/cht-api.ts +++ b/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'; diff --git a/src/lib/retry-logic.ts b/src/lib/retry-logic.ts new file mode 100644 index 00000000..d29afb8a --- /dev/null +++ b/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(funcWithPut: () => Promise): Promise { + 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); +} diff --git a/src/services/upload-manager.ts b/src/services/upload-manager.ts index 97ae19e6..2d108039 100644 --- a/src/services/upload-manager.ts +++ b/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; @@ -56,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'); @@ -64,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; } @@ -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); -} 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/lib/axios-retry-config.spec.ts b/test/lib/axios-retry-config.spec.ts deleted file mode 100644 index b2723206..00000000 --- a/test/lib/axios-retry-config.spec.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { expect } from 'chai'; -import AxiosRetryConfig from '../../src/lib/axios-retry-config'; -import { UploadManagerRetryScenarios } from '../services/upload-manager.spec'; - -const scenarios = [ - { desc: '503', axiosError: { response: { status: 503 } } }, - { desc: 'axios timeout', axiosError: { code: 'ECONNABORTED' } }, - { desc: 'service timeout', axiosError: { code: 'ECONNRESET' } }, - { desc: 'update conflict', axiosError: { code: 'ERR_BAD_REQUEST', response: { status: 409 } } }, -]; - -describe('lib/axios-retry-config', () => { - for (const scenario of scenarios) { - it(scenario.desc, () => { - const doRetry = AxiosRetryConfig.retryCondition(scenario.axiosError as any); - expect(doRetry).to.eq(true); - }); - } - - for (const scenario of UploadManagerRetryScenarios) { - it(scenario.desc, () => { - const doRetry = AxiosRetryConfig.retryCondition(scenario.axiosError as any); - expect(doRetry).to.eq(false); - }); - } -}); diff --git a/test/lib/retry-logic.spec.ts b/test/lib/retry-logic.spec.ts new file mode 100644 index 00000000..2143fc51 --- /dev/null +++ b/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(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 8d614527..d8e94273 100644 --- a/test/services/upload-manager.spec.ts +++ b/test/services/upload-manager.spec.ts @@ -10,44 +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'; - -export const UploadManagerRetryScenarios = [ - { - desc: 'username taken is not retried', - axiosError: { - code: 'ERR_BAD_REQUEST', - response: { - status: 400, - data: { error: { message: 'Username "chu" already taken.', translationKey: 'username.taken' } }, - } - }, - }, - { - 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' } }, - } - }, - }, - { - 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' - } - }, - } - }, - }, -]; +import { UploadManagerRetryScenario } from '../lib/retry-logic.spec'; describe('upload-manager.ts', () => { beforeEach(() => { @@ -268,22 +231,20 @@ describe('upload-manager.ts', () => { expect(chtApi.disableUsersWithPlace.called).to.be.false; }); - for (const scenario of UploadManagerRetryScenarios) { - it(`retry: ${scenario.desc}`, async() => { - const { remotePlace, sessionCache, chtApi } = await createMocks(); + it(`createUser is retried`, async() => { + const { remotePlace, sessionCache, chtApi } = await createMocks(); - chtApi.createUser.throws(scenario.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'); - }); - } + 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) { From ba3103fb66e634a6f8cff45298c2275b6e0174b3 Mon Sep 17 00:00:00 2001 From: "kennsippell@gmail.com" Date: Mon, 12 Feb 2024 11:56:42 +0700 Subject: [PATCH 6/6] Eslint --- test/lib/retry-logic.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/lib/retry-logic.spec.ts b/test/lib/retry-logic.spec.ts index 2143fc51..5325afa2 100644 --- a/test/lib/retry-logic.spec.ts +++ b/test/lib/retry-logic.spec.ts @@ -7,6 +7,7 @@ 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; @@ -89,7 +90,7 @@ describe('lib/retry-logic', () => { }); } - it ('throws after persistent conflict', async () => { + 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');