diff --git a/.changes/next-release/feature-MetadataService-053a415b.json b/.changes/next-release/feature-MetadataService-053a415b.json new file mode 100644 index 0000000000..ac4fc49b56 --- /dev/null +++ b/.changes/next-release/feature-MetadataService-053a415b.json @@ -0,0 +1,5 @@ +{ + "type": "feature", + "category": "MetadataService", + "description": "Adds retry logic to the EC2 Metadata Service, so that EC2MetadataCredentials will retry TimeoutError. This retry logic is also added to ECSCredentials. Resolves #692" +} \ No newline at end of file diff --git a/lib/credentials/ec2_metadata_credentials.js b/lib/credentials/ec2_metadata_credentials.js index 42d5480293..42e1f7ea96 100644 --- a/lib/credentials/ec2_metadata_credentials.js +++ b/lib/credentials/ec2_metadata_credentials.js @@ -9,16 +9,21 @@ require('../metadata_service'); * can connect, and credentials are available, these will be used with zero * configuration. * - * This credentials class will timeout after 1 second of inactivity by default. + * This credentials class will by default timeout after 1 second of inactivity + * and retry 3 times. * If your requests to the EC2 metadata service are timing out, you can increase - * the value by configuring them directly: + * these values by configuring them directly: * * ```javascript * AWS.config.credentials = new AWS.EC2MetadataCredentials({ - * httpOptions: { timeout: 5000 } // 5 second timeout + * httpOptions: { timeout: 5000 }, // 5 second timeout + * maxRetries: 10, // retry 10 times + * retryDelayOptions: { base: 200 } // see AWS.Config for information * }); * ``` * + * @see AWS.Config.retryDelayOptions + * * @!macro nobrowser */ AWS.EC2MetadataCredentials = AWS.util.inherit(AWS.Credentials, { @@ -26,6 +31,8 @@ AWS.EC2MetadataCredentials = AWS.util.inherit(AWS.Credentials, { AWS.Credentials.call(this); options = options ? AWS.util.copy(options) : {}; + options = AWS.util.merge( + {maxRetries: this.defaultMaxRetries}, options); if (!options.httpOptions) options.httpOptions = {}; options.httpOptions = AWS.util.merge( {timeout: this.defaultTimeout}, options.httpOptions); @@ -39,6 +46,11 @@ AWS.EC2MetadataCredentials = AWS.util.inherit(AWS.Credentials, { */ defaultTimeout: 1000, + /** + * @api private + */ + defaultMaxRetries: 3, + /** * Loads the credentials from the instance metadata service * diff --git a/lib/credentials/ecs_credentials.js b/lib/credentials/ecs_credentials.js index 38df3f7557..7eab957014 100644 --- a/lib/credentials/ecs_credentials.js +++ b/lib/credentials/ecs_credentials.js @@ -8,16 +8,21 @@ var AWS = require('../core'); * in the container. If valid credentials are returned in the response, these * will be used with zero configuration. * - * This credentials class will timeout after 1 second of inactivity by default. + * This credentials class will by default timeout after 1 second of inactivity + * and retry 3 times. * If your requests to the relative URI are timing out, you can increase * the value by configuring them directly: * * ```javascript * AWS.config.credentials = new AWS.ECSCredentials({ - * httpOptions: { timeout: 5000 } // 5 second timeout + * httpOptions: { timeout: 5000 }, // 5 second timeout + * maxRetries: 10, // retry 10 times + * retryDelayOptions: { base: 200 } // see AWS.Config for information * }); * ``` * + * @see AWS.Config.retryDelayOptions + * * @!macro nobrowser */ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { @@ -40,6 +45,11 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { */ host: '169.254.170.2', + /** + * @api private + */ + maxRetries: 3, + /** * Sets the name of the ECS environment variable to check for relative URI * If changed, please change the name in the documentation for defaultProvider @@ -69,22 +79,17 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { */ request: function request(path, callback) { path = path || '/'; - - var data = ''; - var http = AWS.HttpClient.getInstance(); var httpRequest = new AWS.HttpRequest('http://' + this.host + path); httpRequest.method = 'GET'; httpRequest.headers.Accept = 'application/json'; - var httpOptions = this.httpOptions; - - process.nextTick(function() { - http.handleRequest(httpRequest, httpOptions, function(httpResponse) { - httpResponse.on('data', function(chunk) { data += chunk.toString(); }); - httpResponse.on('end', function() { callback(null, data); }); - }, callback); - }); + AWS.util.handleRequestWithRetries(httpRequest, this, callback); }, + /** + * @api private + */ + refreshQueue: [], + /** * Loads the credentials from the relative URI specified by container * @@ -98,10 +103,25 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { */ refresh: function refresh(callback) { var self = this; + var refreshQueue = self.refreshQueue; if (!callback) callback = function(err) { if (err) throw err; }; + refreshQueue.push({ + provider: self, + errCallback: callback + }); + if (refreshQueue.length > 1) { return; } + + function callbacks(err, creds) { + var call, cb; + while ((call = refreshQueue.shift()) !== undefined) { + cb = call.errCallback; + if (!err) AWS.util.update(call.provider, creds); + cb(err); + } + } if (process === undefined) { - callback(AWS.util.error( + callbacks(AWS.util.error( new Error('No process info available'), { code: 'ECSCredentialsProviderFailure' } )); @@ -109,7 +129,7 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { } var relativeUri = this.getECSRelativeUri(); if (relativeUri === undefined) { - callback(AWS.util.error( + callbacks(AWS.util.error( new Error('Variable ' + this.environmentVar + ' not set.'), { code: 'ECSCredentialsProviderFailure' } )); @@ -119,13 +139,15 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { this.request(relativeUri, function(err, data) { if (!err) { try { - var creds = JSON.parse(data); - if (self.credsFormatIsValid(creds)) { - self.expired = false; - self.accessKeyId = creds.AccessKeyId; - self.secretAccessKey = creds.SecretAccessKey; - self.sessionToken = creds.Token; - self.expireTime = new Date(creds.Expiration); + data = JSON.parse(data); + if (self.credsFormatIsValid(data)) { + var creds = { + expired: false, + accessKeyId: data.AccessKeyId, + secretAccessKey: data.SecretAccessKey, + sessionToken: data.Token, + expireTime: new Date(data.Expiration) + }; } else { throw AWS.util.error( new Error('Response data is not in valid format'), @@ -136,7 +158,7 @@ AWS.ECSCredentials = AWS.util.inherit(AWS.Credentials, { err = dataError; } } - callback(err); + callbacks(err, creds); }); } }); diff --git a/lib/metadata_service.js b/lib/metadata_service.js index 0eaf453e83..08f5081680 100644 --- a/lib/metadata_service.js +++ b/lib/metadata_service.js @@ -44,6 +44,10 @@ AWS.MetadataService = inherit({ * * * **timeout** (Number) — a timeout value in milliseconds to wait * before aborting the connection. Set to 0 for no timeout. + * @option options maxRetries [Integer] the maximum number of retries to + * perform for timeout errors + * @option options retryDelayOptions [map] A set of options to configure the + * retry delay on retryable errors. See AWS.Config for details. */ constructor: function MetadataService(options) { AWS.util.update(this, options); @@ -61,19 +65,9 @@ AWS.MetadataService = inherit({ */ request: function request(path, callback) { path = path || '/'; - - var data = ''; - var http = AWS.HttpClient.getInstance(); var httpRequest = new AWS.HttpRequest('http://' + this.host + path); httpRequest.method = 'GET'; - var httpOptions = this.httpOptions; - - process.nextTick(function() { - http.handleRequest(httpRequest, httpOptions, function(httpResponse) { - httpResponse.on('data', function(chunk) { data += chunk.toString(); }); - httpResponse.on('end', function() { callback(null, data); }); - }, callback); - }); + AWS.util.handleRequestWithRetries(httpRequest, this, callback); }, /** diff --git a/lib/service.js b/lib/service.js index ac20878dfe..ed89135817 100644 --- a/lib/service.js +++ b/lib/service.js @@ -311,14 +311,7 @@ AWS.Service = inherit({ * @api private */ retryDelays: function retryDelays(retryCount) { - var retryDelayOptions = this.config.retryDelayOptions || {}; - var customBackoff = retryDelayOptions.customBackoff || null; - if (typeof customBackoff === 'function') { - return customBackoff(retryCount); - } - var base = retryDelayOptions.base || 30; - var delay = Math.random() * (Math.pow(2, retryCount) * base); - return delay; + return AWS.util.calculateRetryDelay(retryCount, this.config.retryDelayOptions); }, /** diff --git a/lib/util.js b/lib/util.js index e36c467922..addddffad2 100644 --- a/lib/util.js +++ b/lib/util.js @@ -793,6 +793,64 @@ var util = { if (typeof service !== 'string') service = service.serviceIdentifier; if (typeof service !== 'string' || !metadata.hasOwnProperty(service)) return false; return !!metadata[service].dualstackAvailable; + }, + + /** + * @api private + */ + calculateRetryDelay: function calculateRetryDelay(retryCount, retryDelayOptions) { + if (!retryDelayOptions) retryDelayOptions = {}; + var customBackoff = retryDelayOptions.customBackoff || null; + if (typeof customBackoff === 'function') { + return customBackoff(retryCount); + } + var base = retryDelayOptions.base || 100; + var delay = Math.random() * (Math.pow(2, retryCount) * base); + return delay; + }, + + /** + * @api private + */ + handleRequestWithRetries: function handleRequestWithRetries(httpRequest, options, cb) { + if (!options) options = {}; + var http = AWS.HttpClient.getInstance(); + var httpOptions = options.httpOptions || {}; + var retryCount = 0; + + var errCallback = function(err) { + var maxRetries = options.maxRetries || 0; + if (err && err.code === 'TimeoutError') err.retryable = true; + if (err && err.retryable && retryCount < maxRetries) { + retryCount++; + var delay = util.calculateRetryDelay(retryCount, options.retryDelayOptions); + setTimeout(sendRequest, delay + (err.retryAfter || 0)); + } else { + cb(err); + } + }; + + var sendRequest = function() { + var data = ''; + http.handleRequest(httpRequest, httpOptions, function(httpResponse) { + httpResponse.on('data', function(chunk) { data += chunk.toString(); }); + httpResponse.on('end', function() { + var statusCode = httpResponse.statusCode; + if (statusCode < 300) { + cb(null, data); + } else { + var retryAfter = parseInt(httpResponse.headers['retry-after'], 10) * 1000 || 0; + var err = util.error(new Error(), + { retryable: statusCode >= 500 || statusCode === 429 } + ); + if (retryAfter && err.retryable) err.retryAfter = retryAfter; + errCallback(err); + } + }); + }, errCallback); + }; + + process.nextTick(sendRequest); } }; diff --git a/test/credentials.spec.coffee b/test/credentials.spec.coffee index 8356c39239..b3cf0ecb0d 100644 --- a/test/credentials.spec.coffee +++ b/test/credentials.spec.coffee @@ -427,6 +427,12 @@ if AWS.util.isNode() describe 'AWS.ECSCredentials', -> creds = null + responseData = { + AccessKeyId: 'KEY', + SecretAccessKey: 'SECRET', + Token: 'TOKEN', + Expiration: (new Date(0)).toISOString() + } beforeEach -> creds = new AWS.ECSCredentials(host: 'host') @@ -437,13 +443,8 @@ if AWS.util.isNode() mockEndpoint = (expireTime) -> helpers.spyOn(creds, 'request').andCallFake (path, cb) -> - cb null, - JSON.stringify({ - AccessKeyId: 'KEY' - SecretAccessKey: 'SECRET' - Token: 'TOKEN' - Expiration: expireTime.toISOString() - }) + expiration = expireTime.toISOString() + cb null, JSON.stringify(AWS.util.merge(responseData, {Expiration: expiration})) describe 'constructor', -> it 'allows passing of options', -> @@ -479,16 +480,10 @@ if AWS.util.isNode() describe 'credsFormatIsValid', -> it 'returns false when data is missing required property', -> - responseData = {AccessKeyId: 'KEY', SecretAccessKey: 'SECRET', Token: 'TOKEN'} - expect(creds.credsFormatIsValid(responseData)).to.be.false + incompleteData = {AccessKeyId: 'KEY', SecretAccessKey: 'SECRET', Token: 'TOKEN'} + expect(creds.credsFormatIsValid(incompleteData)).to.be.false it 'returns true when data has all required properties', -> - responseData = { - AccessKeyId: 'KEY', - SecretAccessKey: 'SECRET', - Token: 'TOKEN', - Expiration: (new Date(0)).toISOString() - } expect(creds.credsFormatIsValid(responseData)).to.be.true describe 'needsRefresh', -> @@ -519,6 +514,39 @@ if AWS.util.isNode() expect(spy.calls.length).to.equal(0) expect(callbackErr).to.not.be.null + it 'retries up to specified maxRetries for timeout errors', (done) -> + process.env['AWS_CONTAINER_CREDENTIALS_RELATIVE_URI'] = '/path' + options = {maxRetries: 3} + creds = new AWS.ECSCredentials(options) + httpClient = AWS.HttpClient.getInstance() + spy = helpers.spyOn(httpClient, 'handleRequest').andCallFake (httpReq, httpOp, cb, errCb) -> + errCb({code: 'TimeoutError'}) + creds.refresh (err) -> + expect(err).to.not.be.null + expect(err.code).to.equal('TimeoutError') + expect(spy.calls.length).to.equal(4) + done() + + it 'makes only one request when multiple calls are made before first one finishes', (done) -> + concurrency = countdown = 10 + process.env['AWS_CONTAINER_CREDENTIALS_RELATIVE_URI'] = '/path' + spy = helpers.spyOn(AWS.ECSCredentials.prototype, 'request').andCallFake (path, cb) -> + respond = -> + cb null, JSON.stringify(responseData) + process.nextTick(respond) + providers = [] + callRefresh = (ind) -> + providers[ind] = new AWS.ECSCredentials(host: 'host') + providers[ind].refresh (err) -> + expect(err).to.equal(null) + expect(providers[ind].accessKeyId).to.equal('KEY') + countdown-- + if countdown == 0 + expect(spy.calls.length).to.equal(1) + done() + for x in [1..concurrency] + callRefresh(x - 1) + describe 'AWS.TemporaryCredentials', -> creds = null diff --git a/test/metadata_service.spec.coffee b/test/metadata_service.spec.coffee index 272164de1c..2a788a93b3 100644 --- a/test/metadata_service.spec.coffee +++ b/test/metadata_service.spec.coffee @@ -6,9 +6,14 @@ AWS = helpers.AWS if AWS.util.isNode() describe 'AWS.MetadataService', -> describe 'loadCredentials', -> - [server, port, service] = [null, 1024 + parseInt(Math.random() * 100), null] + [server, port, service, forceTimeout] = [null, 1024 + parseInt(Math.random() * 100), null, null] + httpClient = AWS.HttpClient.getInstance() beforeEach -> + forceTimeout = false + helpers.spyOn(http.ClientRequest.prototype, 'setTimeout').andCallFake (timeout, cb) -> + if forceTimeout + process.nextTick(cb) service = new AWS.MetadataService(host: '127.0.0.1:' + port) server = http.createServer (req, res) -> re = new RegExp('^/latest/meta-data/iam/security-credentials/(.*)$') @@ -52,7 +57,6 @@ if AWS.util.isNode() if countdown == 0 done() - it 'should fail if server is not up', (done) -> server.close(); server = null service = new AWS.MetadataService(host: '255.255.255.255') @@ -61,3 +65,58 @@ if AWS.util.isNode() expect(err).to.be.instanceOf(Error) expect(data).not.to.exist done() + + it 'should retry when request times out', (done) -> + options = { + host: '127.0.0.1:' + port, + maxRetries: 5 + } + + firstTry = true + spy = helpers.spyOn(httpClient, 'handleRequest').andCallFake -> + if firstTry + forceTimeout = true + firstTry = false + else + forceTimeout = false + return spy.origMethod.apply(httpClient, arguments) + + service = new AWS.MetadataService(options) + service.loadCredentials (err, data) -> + expect(err).to.be.null + expect(data.AccessKeyId).to.equal('KEY') + # 2 successful calls to retrieve credentials + 1 timeout + expect(spy.calls.length).to.equal(3) + done() + + it 'should retry up to the specified maxRetries when requests time out', (done) -> + options = { + host: '127.0.0.1:' + port, + maxRetries: 5 + } + forceTimeout = true + spy = helpers.spyOn(httpClient, 'handleRequest').andCallThrough() + + service = new AWS.MetadataService(options) + service.loadCredentials (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.code).to.equal('TimeoutError') + # 1st failed try + 5 retries + expect(spy.calls.length).to.equal(6) + done() + + it 'makes only one pair of requests when multiple calls are made before first one finishes', (done) -> + options = {host: '127.0.0.1:' + port} + spy = helpers.spyOn(httpClient, 'handleRequest').andCallThrough() + concurrency = countdown = 10 + services = [] + for x in [1..concurrency] + services[x - 1] = new AWS.MetadataService(options) + services[x - 1].loadCredentials (err, data) -> + expect(err).to.equal(null) + expect(data.AccessKeyId).to.equal('KEY') + countdown-- + if countdown == 0 + expect(spy.calls.length).to.equal(2) + done() diff --git a/test/util.spec.coffee b/test/util.spec.coffee index 21312c9e4e..0699bdbc78 100644 --- a/test/util.spec.coffee +++ b/test/util.spec.coffee @@ -722,3 +722,164 @@ describe 'AWS.util.isDualstackAvailable', -> expect(AWS.util.isDualstackAvailable(null)).to.be.false expect(AWS.util.isDualstackAvailable('invalid')).to.be.false expect(AWS.util.isDualstackAvailable({})).to.be.false + +describe 'AWS.util.calculateRetryDelay', -> + beforeEach -> + helpers.spyOn(Math, 'random').andReturn 1 + + it 'exponentially increases delay as retryCount increases', -> + delay1 = AWS.util.calculateRetryDelay(1) + delay2 = AWS.util.calculateRetryDelay(2) + delay3 = AWS.util.calculateRetryDelay(3) + expect(delay2).to.equal(delay1 * 2) + expect(delay3).to.equal(delay1 * 4) + + it 'has random jitter', -> + delay1 = AWS.util.calculateRetryDelay(1) + helpers.spyOn(Math, 'random').andReturn 0.5 + delay2 = AWS.util.calculateRetryDelay(1) + expect(delay2).to.not.equal(delay1) + + it 'allows configuration of base delay', -> + delay = AWS.util.calculateRetryDelay(1, { base: 1000 }) + expect(delay).to.equal(2000) + + it 'allows custom backoff function', -> + customBackoff = (retryCount) -> + return 100 * Math.pow(3, retryCount) + delay = AWS.util.calculateRetryDelay(2, { customBackoff: customBackoff }) + expect(delay).to.equal(900) + +if AWS.util.isNode() + describe 'AWS.util.handleRequestWithRetries', -> + http = require('http') + app = null; httpRequest = null; spy = null + options = {maxRetries: 2} + httpClient = AWS.HttpClient.getInstance() + + sendRequest = (cb) -> + AWS.util.handleRequestWithRetries httpRequest, options, cb + + getport = (cb, startport) -> + port = startport or 45678 + srv = require('net').createServer() + srv.on 'error', -> getport(cb, port + 1) + srv.listen port, -> + srv.once 'close', -> cb(port) + srv.close() + + server = http.createServer (req, resp) -> + app(req, resp) + + beforeEach (done) -> + httpRequest = new AWS.HttpRequest('http://127.0.0.1') + spy = helpers.spyOn(httpClient, 'handleRequest').andCallThrough() + getport (port) -> + httpRequest.endpoint.port = port + server.listen(port) + done() + + afterEach -> + server.close() + + it 'does not retry if request is successful', (done) -> + app = (req, resp) -> + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(err).to.be.null + expect(data).to.equal('FOOBAR') + expect(spy.calls.length).to.equal(1) + done() + + it 'retries for TimeoutError', (done) -> + forceTimeout = true + helpers.spyOn(http.ClientRequest.prototype, 'setTimeout').andCallFake (timeout, cb) -> + if forceTimeout + process.nextTick(cb) + forceTimeout = false + app = (req, resp) -> + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(err).to.be.null + expect(data).to.equal('FOOBAR') + expect(spy.calls.length).to.equal(2) + done() + + it 'retries up to the maxRetries specified', (done) -> + helpers.spyOn(http.ClientRequest.prototype, 'setTimeout').andCallFake (timeout, cb) -> + process.nextTick(cb) + app = (req, resp) -> + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.code).to.equal('TimeoutError') + expect(err.retryable).to.be.true + expect(spy.calls.length).to.equal(options.maxRetries + 1) + done() + + it 'retries errors with status code 5xx', (done) -> + app = (req, resp) -> + resp.writeHead(500, {}) + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.retryable).to.be.true + expect(spy.calls.length).to.equal(options.maxRetries + 1) + done() + + it 'retries errors with status code 429', (done) -> + app = (req, resp) -> + resp.writeHead(429, {}) + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.retryable).to.be.true + expect(spy.calls.length).to.equal(options.maxRetries + 1) + done() + + it 'does not retry non-retryable errors', (done) -> + app = (req, resp) -> + resp.writeHead(400, {}) + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.retryable).to.be.false + expect(spy.calls.length).to.equal(1) + done() + + it 'retries errors with retryable set to true', (done) -> + helpers.spyOn(AWS.util, 'error').andReturn {retryable: true} + app = (req, resp) -> + resp.writeHead(400, {}) + resp.write('FOOBAR') + resp.end() + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(err.retryable).to.be.true + expect(spy.calls.length).to.equal(options.maxRetries + 1) + done() + + it 'defaults to not retrying if maxRetries not specified', (done) -> + helpers.spyOn(AWS.util, 'error').andReturn {retryable: true} + app = (req, resp) -> + resp.writeHead(400, {}) + resp.write('FOOBAR') + resp.end() + options = {} + sendRequest (err, data) -> + expect(data).to.be.undefined + expect(err).to.not.be.null + expect(spy.calls.length).to.equal(1) + done() +