Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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': '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();
});
});
});
});