From 1d24b406bad30e72cd98a5d778e8b7094b86e0d3 Mon Sep 17 00:00:00 2001 From: Ewan Dennis Date: Tue, 4 Jul 2017 16:42:10 +0100 Subject: [PATCH 1/4] Optional automatic retry on 5xx --- lib/sparkpost.js | 33 ++++++++++++-- test/spec/sparkpost.spec.js | 86 +++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/lib/sparkpost.js b/lib/sparkpost.js index 5e28754..8100786 100644 --- a/lib/sparkpost.js +++ b/lib/sparkpost.js @@ -11,6 +11,7 @@ const defaults = { origin: 'https://api.sparkpost.com:443', apiVersion: 'v1', debug: false + /*retries: undefined*/ }; const resolveUri = function(origin, uri) { @@ -72,9 +73,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); @@ -88,6 +87,13 @@ 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 = (typeof options.retries === 'number') ? options.retries : defaults.retries; +}; + SparkPost.prototype.request = function(options, callback) { const baseUrl = `${this.origin}/api/${this.apiVersion}/`; @@ -116,8 +122,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; @@ -137,6 +150,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; diff --git a/test/spec/sparkpost.spec.js b/test/spec/sparkpost.spec.js index b3ab7cf..8bff7b4 100644 --- a/test/spec/sparkpost.spec.js +++ b/test/spec/sparkpost.spec.js @@ -83,6 +83,20 @@ 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); + + }); + function checkUserAgent(clientOptions, checkFn, done) { let req = { method: 'GET' @@ -207,6 +221,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') From 7b9c16f621b821291ce2df463d406b7562d467b9 Mon Sep 17 00:00:00 2001 From: Ewan Dennis Date: Wed, 5 Jul 2017 10:16:25 +0100 Subject: [PATCH 2/4] Updated readme --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 747dbc5..28f2a4f 100644 --- a/README.md +++ b/README.md @@ -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` @@ -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. From c2863d6f36e24a0099326b9198876dcdb79c43db Mon Sep 17 00:00:00 2001 From: Ewan Dennis Date: Wed, 5 Jul 2017 15:46:46 +0100 Subject: [PATCH 3/4] Added retry feature to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 702cafd..0cf91f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 From 56f7ea5dd956a3f7d1038f817c53bbe92406b7f4 Mon Sep 17 00:00:00 2001 From: Ewan Dennis Date: Fri, 11 Aug 2017 10:14:56 +0100 Subject: [PATCH 4/4] Limit retries --- lib/sparkpost.js | 7 ++++++- test/spec/sparkpost.spec.js | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/sparkpost.js b/lib/sparkpost.js index 8100786..0f003a9 100644 --- a/lib/sparkpost.js +++ b/lib/sparkpost.js @@ -6,6 +6,8 @@ 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', @@ -91,7 +93,10 @@ 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 = (typeof options.retries === 'number') ? options.retries : defaults.retries; + this.retries = defaults.retries; + if (typeof options.retries === 'number') { + this.retries = Math.min(options.retries, maxRetries); + } }; SparkPost.prototype.request = function(options, callback) { diff --git a/test/spec/sparkpost.spec.js b/test/spec/sparkpost.spec.js index 8bff7b4..198017f 100644 --- a/test/spec/sparkpost.spec.js +++ b/test/spec/sparkpost.spec.js @@ -97,6 +97,13 @@ describe('SparkPost Library', function() { }); + 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'