Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added network retries option #198

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
115 changes: 75 additions & 40 deletions lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which error cases does this cover?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Network-related errors
  • Connection errors
  • HTTP protocol errors and
  • Server errors(status code in the range of 500-599)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@aashishgurung aashishgurung Jul 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our APIs handles 500 and 503 errors and responds a JSON. Will this cover those cases as well?

https://www.omise.co/api-errors#500-internal-server-error

// 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';
Expand Down Expand Up @@ -100,6 +145,7 @@ function _prepareOptionsFor(method, options) {
method: method,
body: content['data'],
agent: agent,
scheme: options['scheme'] || 'https',
};
}

Expand All @@ -110,28 +156,17 @@ 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
? 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) {
Expand Down
112 changes: 112 additions & 0 deletions test/test_network_retries.js
Original file line number Diff line number Diff line change
@@ -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': '[email protected]',
'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, '[email protected]');
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, '[email protected]');
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();
});
});
});
});