[GitHub] [openwhisk-client-js] selfxp commented on a change in pull request #227: Support client retries

[GitBox <gi...@apache.org>]

selfxp commented on a change in pull request #227:
URL: https://github.com/apache/openwhisk-client-js/pull/227#discussion_r728395861



##########
File path: test/unit/client.test.js
##########
@@ -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)
+})
+

Review comment:
       Should we add a test to validate that indeed no retry is made in case no retry configuration exists? Something like: 
   ```
   test('should not retry when no retry config available', 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).replyWithError('simulated error')
       .get(PATH).times(1).reply(200, 'now all good')
     const error = await t.throwsAsync(client.request(METHOD, PATH, {}))
     t.truthy(error.message)
     t.assert(error.message.includes('simulated error'))
     nock.removeInterceptor(interceptor)
   })
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org