Skip to content

Support client retries #227

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

Merged
merged 9 commits into from
Oct 21, 2021
Merged
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ _Client constructor supports the following mandatory parameters:_
- **key**. Client key to use when connecting to the `apihost` (if `nginx_ssl_verify_client` is turned on in your apihost)
- **proxy.** HTTP(s) URI for proxy service to forwards requests through. Uses Needle's [built-in proxy support](https://github.com/tomas/needle#request-options).
- **agent.** Provide custom [http.Agent](https://nodejs.org/api/http.html#http_class_http_agent) implementation.

- **retry**. Provide a retry options to retry on errors, for example, `{ retries: 2 }`. By default, no retries will be done. Uses [async-retry options](https://github.com/vercel/async-retry#api). Default values are different from async-retry, please refer to the api doc.

### environment variables

Expand Down
42 changes: 40 additions & 2 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const OpenWhiskError = require('./openwhisk_error')
const needle = require('needle')
const url = require('url')
const http = require('http')
const retry = require('async-retry')

/**
* This implements a request-promise-like facade over the needle
Expand Down Expand Up @@ -77,6 +78,13 @@ const rp = opts => {
})
}

const rpWithRetry = opts => {
return retry(bail => {
// will retry on exception
return rp(opts)
}, opts.retry)
}

class Client {
/**
* @constructor
Expand All @@ -93,6 +101,13 @@ class Client {
* @param {boolean} [options.noUserAgent]
* @param {string} [options.cert]
* @param {string} [options.key]
* @param {object} [options.retry]
* @param {number} [options.retry.retries] Number of retries on top of the initial request, default is 2.
* @param {number} [options.retry.factor] Exponential factor, default is 2.
* @param {number} [options.retry.minTimeout] Milliseconds before the first retry, default is 100.
* @param {number} [options.retry.maxTimeout] Max milliseconds in between two retries, default is infinity.
* @param {boolean} [options.retry.randomize] Randomizes the timeouts by multiplying with a factor between 1 to 2. Default is true.
* @param {Function} [options.retry.onRetry] An optional Function that is invoked after a new retry is performed. It's passed the Error that triggered it as a parameter.
*/
constructor (options) {
this.options = this.parseOptions(options || {})
Expand Down Expand Up @@ -135,7 +150,21 @@ class Client {
throw new Error(`${messages.INVALID_OPTIONS_ERROR} Missing either api or apihost parameters.`)
}

return { apiKey: apiKey, api, apiVersion: apiversion, ignoreCerts: ignoreCerts, namespace: options.namespace, apigwToken: apigwToken, apigwSpaceGuid: apigwSpaceGuid, authHandler: options.auth_handler, noUserAgent: options.noUserAgent, cert: options.cert, key: options.key, proxy, agent }
// gather retry options
const retry = options.retry
if (retry && typeof options.retry !== 'object') {
throw new Error(`${messages.INVALID_OPTIONS_ERROR} 'retry' option must be an object, e.g. '{ retries: 2 }'.`)
}
if (retry) {
// overwrite async-retry defaults, see https://github.com/vercel/async-retry#api for more details
retry.retries = retry.retries || 2
retry.factor = retry.factor || 2
retry.minTimeout = retry.minTimeout || 100
retry.maxTimeout = retry.maxTimeout || Infinity
retry.randomize = retry.randomize || true
}

return { apiKey: apiKey, api, apiVersion: apiversion, ignoreCerts: ignoreCerts, namespace: options.namespace, apigwToken: apigwToken, apigwSpaceGuid: apigwSpaceGuid, authHandler: options.auth_handler, noUserAgent: options.noUserAgent, cert: options.cert, key: options.key, proxy, agent, retry }
}

urlFromApihost (apihost, apiversion = 'v1') {
Expand All @@ -152,7 +181,12 @@ class Client {

request (method, path, options) {
const params = this.params(method, path, options)
return params.then(req => rp(req)).catch(err => this.handleErrors(err))
return params.then(req => {
if (req.retry) {
return rpWithRetry(req)
}
return rp(req)
}).catch(err => this.handleErrors(err))
}

params (method, path, options) {
Expand Down Expand Up @@ -194,6 +228,10 @@ class Client {
parms.agent = this.options.agent
}

if (this.options.retry) {
parms.retry = this.options.retry
}

return parms
})
}
Expand Down
40 changes: 32 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"standard": "^12.0.1"
},
"dependencies": {
"async-retry": "^1.3.3",
"needle": "^2.4.0"
}
}
46 changes: 44 additions & 2 deletions test/unit/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,60 @@ const Client = require('../../lib/client')
const http = require('http')
const nock = require('nock')

// Note: this has to come before any of the proxy tests, as they interfere
// Note: All client.request tests have to come before any of the proxy tests, as they interfere

test('should return response', async t => {
const client = new Client({ api_key: 'secret', apihost: 'test_host', proxy: '' })
const METHOD = 'GET'
const PATH = '/some/path'

const interceptor = nock('https://test_host').get(PATH).times(1).reply(200, 'all good')
const result = await client.request(METHOD, PATH, {})
t.is(result.toString(), 'all good')
nock.removeInterceptor(interceptor)
})

test('should handle http request errors', async t => {
const client = new Client({ api_key: 'secret', apihost: 'test_host', proxy: '' })
const METHOD = 'GET'
const PATH = '/some/path'

nock('https://test_host').get(PATH).replyWithError('simulated error')
const interceptor = nock('https://test_host').get(PATH).times(1).replyWithError('simulated error')
const error = await t.throwsAsync(client.request(METHOD, PATH, {}))
t.truthy(error.message)
t.assert(error.message.includes('simulated error'))
nock.removeInterceptor(interceptor)
})

test('should support retries on error', async t => {
const client = new Client({ api_key: 'secret', apihost: 'test_host', proxy: '', retry: { retries: 2 } })
const METHOD = 'GET'
const PATH = '/some/path'

const interceptor = nock('https://test_host')
.get(PATH).times(2).replyWithError('simulated error')
.get(PATH).times(1).reply(200, 'now all good')
const result = await client.request(METHOD, PATH, {})
t.is(result.toString(), 'now all good')
nock.removeInterceptor(interceptor)
})

test('should handle errors even after retries', async t => {
const client = new Client({ api_key: 'secret', apihost: 'test_host', proxy: '', retry: { retries: 2 } })
const METHOD = 'GET'
const PATH = '/some/path'

const interceptor = nock('https://test_host')
.get(PATH).times(3).replyWithError('simulated error')
.get(PATH).times(1).reply(200, 'not enough retries to come here')
const error = await t.throwsAsync(client.request(METHOD, PATH, {}))
t.truthy(error.message)
t.assert(error.message.includes('simulated error'))
nock.removeInterceptor(interceptor)
})

// end client request tests

test('should use default constructor options', t => {
const client = new Client({ api_key: 'aaa', apihost: 'my_host' })
t.false(client.options.ignoreCerts)
Expand Down