From 5d0d471e7db55076db5360a377e7a26e3cfb4233 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Tue, 20 Jun 2023 12:50:55 +0700 Subject: [PATCH 1/3] added network retries option --- README.md | 11 ++++ lib/api.js | 114 +++++++++++++++++++++++------------ test/test_network_retries.js | 112 ++++++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 40 deletions(-) create mode 100644 test/test_network_retries.js diff --git a/README.md b/README.md index 0abcc87..478a634 100644 --- a/README.md +++ b/README.md @@ -230,6 +230,17 @@ The following API methods are available. Please see [https://www.omise.co/docs]( * search * `list(data)` +## Network retries + +To enable automatic network retries, set the `maxNetworkRetries` configuration option. Requests will be retried with exponential backoff if there are intermittent network issues. + +``` +var omise = require('omise')({ + ... + maxNetworkRetries : 2 +}); +``` + ## Testing There are two modes of testing, to test without connecting to remote API server: diff --git a/lib/api.js b/lib/api.js index 234251a..76e6994 100644 --- a/lib/api.js +++ b/lib/api.js @@ -11,35 +11,80 @@ const ApiError = require('./errors/api-error'); const httpsProxyAgent = require('https-proxy-agent'); -function _responseHandler(req) { +const DEFAULT_MAX_NETWORK_RETRY = 0; +const MAX_NETWORK_RETRY_DELAY = 2000; // in ms +const INITIAL_NETWORK_RETRY_DELAY = 500; // in ms + +function _processHttpRequest( + requestOptions, + maxNetworkRetries, + totalRetry = 0 +) { return new Promise(function(resolve, reject) { + const scheme = process.env.OMISE_SCHEME || requestOptions.scheme; + const req = scheme == 'http' + ? http.request(requestOptions) + : https.request(requestOptions); + + if (_requestMethodWithBody(requestOptions.method)) { + req.write(requestOptions['body'], 'utf8'); + } + req.on('response', function(res) { - let resp = ''; - res.setEncoding('utf8'); - res.on('data', function(chunk) { - resp += chunk; - }); - res.on('end', function() { - try { - logger.log('info', resp); - if (!_isValidJsonString(resp)) { - reject(resp); - } - resp = JSON.parse(resp); - if (resp.object === 'error') { - reject(resp); - } else { - resolve(resp); - } - } catch (err) { - reject(err); - } - }); - }) - .on('error', reject); + _handleHttpResponse(res, resolve, reject); + }).on('error', (err) => { + // retry again + if (totalRetry < maxNetworkRetries) { + setTimeout(() => { + return _processHttpRequest( + requestOptions, + maxNetworkRetries, + totalRetry + 1 + ).then(resolve) + .catch(reject); + }, _getNetworkRetryDelay(totalRetry)); + } else { + reject(err); + } + }).end(); + }); +} + +function _handleHttpResponse(response, resolve, reject) { + let result = ''; + response.setEncoding('utf8'); + + response.on('data', function(chunk) { + result += chunk; + }); + + response.on('end', function() { + try { + logger.log('info', result); + if (!_isValidJsonString(result)) { + reject(result); + } + result = JSON.parse(result); + if (result.object === 'error') { + reject(result); + } else { + resolve(result); + } + } catch (err) { + reject(err); + } }); } +function _getNetworkRetryDelay(retryCount) { + // Apply exponential backoff of the retry delay with + // maximum delay MAX_NETWORK_RETRY_DELAY + return Math.min( + MAX_NETWORK_RETRY_DELAY, + INITIAL_NETWORK_RETRY_DELAY * Math.pow(1.5, retryCount) + ); +} + function _buildContentHeaders(options) { const reqData = JSON.stringify(options['body']) || ''; const contentType = 'application/json'; @@ -100,6 +145,7 @@ function _prepareOptionsFor(method, options) { method: method, body: content['data'], agent: agent, + scheme: options['scheme'] || 'https', }; } @@ -110,28 +156,16 @@ function _httpsRequest(method, options, callback) { const requestOptions = _prepareOptionsFor(method, options); logger.log('info', 'request options: ' + JSON.stringify(requestOptions)); - const protocol = process.env.OMISE_SCHEME || options['scheme'] || 'https'; - const request = protocol == 'http' - ? http.request(requestOptions) - : https.request(requestOptions); - - let resolve; + const maxNetworkRetries = + options.maxNetworkRetries ?? DEFAULT_MAX_NETWORK_RETRY; if (callback) { - _responseHandler(request) + _processHttpRequest(requestOptions, maxNetworkRetries) .then((res) => callback(null, res)) .catch((err) => callback(err, null)); } else { - resolve = _responseHandler(request); - } - - if (_requestMethodWithBody(method)) { - request.write(requestOptions['body'], 'utf8'); + return _processHttpRequest(requestOptions, maxNetworkRetries); } - - request.end(); - - return resolve; } function _httpRequestFactory(method) { diff --git a/test/test_network_retries.js b/test/test_network_retries.js new file mode 100644 index 0000000..a02ba2b --- /dev/null +++ b/test/test_network_retries.js @@ -0,0 +1,112 @@ +const {assert} = require('chai'); +const config = require('./config'); +const nock = require('nock'); +const omiseInstance = require('../index'); + +function mockFailedResponse(times) { + nock('https://api.omise.co') + .get('/account') + .times(times) + .replyWithError('Network error'); +} + +function mockSuccessResponse() { + nock('https://api.omise.co') + .persist() + .get('/account') + .reply(200, { + 'object': 'account', + 'id': 'acct_123', + 'email': 'test@omise.co', + 'created': '2015-02-02T13:19:17Z', + }, { + 'server': 'nginx/1.1', + 'content-type': 'application/json', + }); +} + +describe('Omise', function() { + describe('#Network Retries', function() { + // Testing when api get success response after failed for 2 times. + // since maxNetworkRetries is set to 3, it should retry for 3 times + // and get success response + it('should be able to retrieve data when maxNetworkRetries is set', + (done) => { + // cleaning for previous mock + nock.cleanAll(); + + // setting network failed for 2 times + mockFailedResponse(2); + + // set network success after 2 times failed + mockSuccessResponse(); + + // override config to set maxNetworkRetries + const omise = omiseInstance({...config, maxNetworkRetries: 3}); + + omise.account.retrieve(function(err, resp) { + if (err) done(err); + assert.equal(resp.object, 'account'); + assert.equal(resp.id, 'acct_123'); + assert.equal(resp.email, 'test@omise.co'); + done(); + }); + }); + + it('should throw error when maxNetworkRetries is not set', (done) => { + // cleaning for previous mock + nock.cleanAll(); + + // mock api to throw network error + mockFailedResponse(1); + + const omise = omiseInstance(config); + + omise.account.retrieve(function(err, resp) { + assert.equal(err.message, 'Network error'); + assert.typeOf(err, 'Error'); + done(); + }); + }); + + it('testing for normal behavior when maxNetworkRetries is set ', (done) => { + // cleaning for previous mock + nock.cleanAll(); + + // mock api to throw success response + mockSuccessResponse(); + + // set maxNetworkRetries to 3 + const omise = omiseInstance({...config, maxNetworkRetries: 3}); + + omise.account.retrieve(function(err, resp) { + if (err) done(err); + assert.equal(resp.object, 'account'); + assert.equal(resp.id, 'acct_123'); + assert.equal(resp.email, 'test@omise.co'); + done(); + }); + }); + + it('should throw error when failed response is over maxNetworkRetries', + (done) => { + // cleaning previous mock + nock.cleanAll(); + + // mock failed for 2 times + mockFailedResponse(2); + + // success response after 2 times failed + mockSuccessResponse(); + + // set maxNetworkRetries to 1 + const omise = omiseInstance({...config, maxNetworkRetries: 1}); + + omise.account.retrieve(function(err, resp) { + assert.equal(err.message, 'Network error'); + assert.typeOf(err, 'Error'); + done(); + }); + }); + }); +}); From 0b185d42aee9fd3549e01ad29b03920982a07431 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Tue, 20 Jun 2023 12:55:18 +0700 Subject: [PATCH 2/3] fix incompatible with node v12.x --- lib/api.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/api.js b/lib/api.js index 76e6994..f4a70dc 100644 --- a/lib/api.js +++ b/lib/api.js @@ -156,8 +156,9 @@ function _httpsRequest(method, options, callback) { const requestOptions = _prepareOptionsFor(method, options); logger.log('info', 'request options: ' + JSON.stringify(requestOptions)); - const maxNetworkRetries = - options.maxNetworkRetries ?? DEFAULT_MAX_NETWORK_RETRY; + const maxNetworkRetries = options.maxNetworkRetries + ? options.maxNetworkRetries + : DEFAULT_MAX_NETWORK_RETRY; if (callback) { _processHttpRequest(requestOptions, maxNetworkRetries) From c8982c58a47800ffe866ecde36c8a1fc5cfa3678 Mon Sep 17 00:00:00 2001 From: Zin Kyaw Kyaw <108650842+ajzkk@users.noreply.github.com> Date: Tue, 20 Jun 2023 13:18:08 +0700 Subject: [PATCH 3/3] update readme --- README.md | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/README.md b/README.md index 478a634..0abcc87 100644 --- a/README.md +++ b/README.md @@ -230,17 +230,6 @@ The following API methods are available. Please see [https://www.omise.co/docs]( * search * `list(data)` -## Network retries - -To enable automatic network retries, set the `maxNetworkRetries` configuration option. Requests will be retried with exponential backoff if there are intermittent network issues. - -``` -var omise = require('omise')({ - ... - maxNetworkRetries : 2 -}); -``` - ## Testing There are two modes of testing, to test without connecting to remote API server: