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

Issue-213: Optional automatic retry on 5xx #214

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
## [Unreleased][unreleased]
### Added
- Example for [sending a transmission with an inline image](/examples/transmissions/send_inline_image.js) by @aydrian.
- The client can optionally retry API calls on 5xx status. Controlled by constructor `options.retries`.

## [2.1.2] - 2017-01-20
### Fixed
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ npm install sparkpost
* Required: no
* Type: `String`
* Default: `https://api.sparkpost.com:443`
* `options.retries`
* Required: no
* Type: `Number`
* Default: `0`
* the number of API call attempts the client makes when receiving a 5xx response
* `options.apiVersion`
* Required: no
* Type: `String`
Expand Down Expand Up @@ -61,6 +66,7 @@ npm install sparkpost
* `options` - [see request modules options](https://github.com/mikeal/request#requestoptions-callback)
* `options.uri` - can either be a full url or a path that is appended to `options.origin` used at initialization ([url.resolve](http://nodejs.org/api/url.html#url_url_resolve_from_to))
* `options.debug` - setting to `true` includes full response from request client for debugging purposes
* `options.retries` - the number of times the client will retry an API call after a 5xx response. Defaults to 0.
* **get | post | put | delete(options[, callback])**
* `options` - see request options
* Request method will be overwritten and set to the same value as the name of these methods.
Expand Down
38 changes: 34 additions & 4 deletions lib/sparkpost.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ const withCallback = require('./withCallback');
const request = require('request');
const _ = require('lodash');

const maxRetries = 3;

//REST API Config Defaults
const defaults = {
origin: 'https://api.sparkpost.com:443',
apiVersion: 'v1',
debug: false
/*retries: undefined*/
};

const resolveUri = function(origin, uri) {
Expand Down Expand Up @@ -72,9 +75,7 @@ const SparkPost = function(apiKey, options) {
}, options.headers);

//Optional client config
this.origin = options.origin;
this.apiVersion = options.apiVersion || defaults.apiVersion;
this.debug = (typeof options.debug === 'boolean') ? options.debug : defaults.debug;
this.optionalConfig(options);

this.inboundDomains = require('./inboundDomains')(this);
this.messageEvents = require('./messageEvents')(this);
Expand All @@ -88,6 +89,16 @@ const SparkPost = function(apiKey, options) {
this.webhooks = require('./webhooks')(this);
};

SparkPost.prototype.optionalConfig = function(options) {
this.origin = options.origin;
this.apiVersion = options.apiVersion || defaults.apiVersion;
this.debug = (typeof options.debug === 'boolean') ? options.debug : defaults.debug;
this.retries = defaults.retries;
if (typeof options.retries === 'number') {
this.retries = Math.min(options.retries, maxRetries);
}
};

SparkPost.prototype.request = function(options, callback) {
const baseUrl = `${this.origin}/api/${this.apiVersion}/`;

Expand Down Expand Up @@ -116,8 +127,15 @@ SparkPost.prototype.request = function(options, callback) {
// set debug
options.debug = (typeof options.debug === 'boolean') ? options.debug : this.debug;

// Use a request handler which supports retries
let requestFn = request;
if (this.retries) {
requestFn = requestWithRetry;
options.retries = this.retries;
}

return withCallback(new Promise(function(resolve, reject) {
request(options, function(err, res, body) {
requestFn(options, function(err, res, body) {
const invalidCodeRegex = /(5|4)[0-9]{2}/;
let response;

Expand All @@ -137,6 +155,18 @@ SparkPost.prototype.request = function(options, callback) {
}), callback);
};

function requestWithRetry(options, cb) {
request(options, function(err, res, body) {
if (!err && res.statusCode >= 500 && res.statusCode <= 599) {
options.retries--;
if (options.retries >= 0) {
return requestWithRetry(options, cb);
}
}
return cb(err, res, body);
});
}

SparkPost.prototype.get = function(options, callback) {
options.method = 'GET';
options.json = true;
Expand Down
93 changes: 93 additions & 0 deletions test/spec/sparkpost.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,27 @@ describe('SparkPost Library', function() {
expect(client.debug).to.equal(true);
});

it('should accept a retries option', function() {
const key = '12345678901234567890';
let client;

// testing default initialization
client = new SparkPost(key, {});
expect(client.retries).to.be.undefined;

// testing setting option
client = new SparkPost(key, { retries: 3 });
expect(client.retries).to.equal(3);

});

it('should limit retries', function() {
const bigRetries = 300;
const key = '12345678901234567890';
const client = new SparkPost(key, { retries: bigRetries });
expect(client.retries).to.be.below(bigRetries);
});

function checkUserAgent(clientOptions, checkFn, done) {
let req = {
method: 'GET'
Expand Down Expand Up @@ -207,6 +228,78 @@ describe('SparkPost Library', function() {
});
});

it('should not retry by default', function(done) {
const result1 = { ok: true };
const result2 = { errors: [] };
nock('https://api.sparkpost.com')
.post('/api/v1/post/test')
.reply(200, result1)
.post('/api/v1/post/test')
.reply(200, result2);

var options = {
method: 'POST'
, uri: 'post/test'
};

client.request(options, function(err, data) {
expect(data).to.be.defined;
expect(data).to.equal(JSON.stringify(result1));
done();
});
});

it('should retry on 5xx if requested', function(done) {
const testResult = {results: 'goodness'};
nock('https://api.sparkpost.com')
.post('/api/v1/post/test/retries')
.reply(503, { errors: [] })
.post('/api/v1/post/test/retries')
.reply(200, testResult);

var options = {
method: 'POST'
, uri: 'post/test/retries'
, retries: 2
};

const key = '12345678901234567890';
const retryClient = new SparkPost(key, { retries: 2 });

retryClient.request(options, function(err, data) {
expect(err).to.be.null;
expect(data).to.be.defined;
expect(data).to.equal(JSON.stringify(testResult));

// finish async test
done();
});
});

it('should obey the retries option', function(done) {
const testResult = {errors: []};
nock('https://api.sparkpost.com')
.post('/api/v1/post/test/two-retries')
.thrice()
.reply(503, testResult);

var options = {
method: 'POST'
, uri: 'post/test/two-retries'
, retries: 2
};

const key = '12345678901234567890';
const retryClient = new SparkPost(key, { retries: 2 });

retryClient.request(options, function(err, data) {
expect(err).to.be.defined;

// finish async test
done();
});
});

it('should return an error if statusCode not 2XX and there is no body', function(done) {
// simulate a timeout
nock('https://api.sparkpost.com')
Expand Down