You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by st...@apache.org on 2021/12/20 22:03:14 UTC

[openwhisk-client-js] branch master updated: fix: Do not modify input params. This closes #230 (#231)

This is an automated email from the ASF dual-hosted git repository.

stanciu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openwhisk-client-js.git


The following commit(s) were added to refs/heads/master by this push:
     new 3701d0e  fix: Do not modify input params. This closes #230 (#231)
3701d0e is described below

commit 3701d0ed1e5ba12600d56db2d195d6b52ea17bd5
Author: Jesse MacFadyen <pu...@gmail.com>
AuthorDate: Mon Dec 20 14:00:12 2021 -0800

    fix: Do not modify input params. This closes #230 (#231)
    
    * fix: Do not modify input params. This closes #230
    
    * added test
---
 lib/client.js            | 18 +++++-------------
 test/unit/client.test.js | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/lib/client.js b/lib/client.js
index ee92620..3f1c510 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -32,27 +32,19 @@ const retry = require('async-retry')
  *
  */
 const rp = opts => {
+  let url = opts.url
   if (opts.qs) {
-    // we turn the qs struct into a query string over the url
-    let first = true
-    for (let key in opts.qs) {
-      const str = `${encodeURIComponent(key)}=${encodeURIComponent(opts.qs[key])}`
-      if (first) {
-        opts.url += `?${str}`
-        first = false
-      } else {
-        opts.url += `&${str}`
-      }
-    }
+    url += '?' + Object.keys(opts.qs)
+      .map(key => `${encodeURIComponent(key)}=${encodeURIComponent(opts.qs[key])}`)
+      .join('&')
   }
-
   // it appears that certain call paths from our code do not set the
   // opts.json field to true; rp is apparently more resilient to
   // this situation than needle
   opts.json = true
 
   return needle(opts.method.toLowerCase(), // needle takes e.g. 'put' not 'PUT'
-    opts.url,
+    url,
     opts.body || opts.params,
     opts)
     .then(resp => {
diff --git a/test/unit/client.test.js b/test/unit/client.test.js
index 7e93c60..370d9b2 100644
--- a/test/unit/client.test.js
+++ b/test/unit/client.test.js
@@ -108,6 +108,34 @@ test('should handle errors even after retries', async t => {
   mock.interceptors.forEach(nock.removeInterceptor)
 })
 
+test('should retry with same url + querystring', async t => {
+  const calledUrls = []
+  const retrySpy = sinon.spy((error) => {
+    if (error && error.options) {
+      calledUrls.push(error.options.url)
+    }
+  })
+  const client = new Client({ api_key: 'secret', apihost: 'test_host', proxy: '', retry: { retries: 3, onRetry: retrySpy } })
+  const METHOD = 'GET'
+  const PATH = '/dont/add/to/querystring'
+  const options = { qs: { bob: 'your uncle' } }
+  const mock = nock('https://test_host')
+    .get(PATH)
+    .query({ bob: 'your uncle' })
+    .times(4)
+    .replyWithError('simulated error')
+    .get(PATH).times(1).reply(200, 'not enough retries to come here')
+
+  const error = await t.throwsAsync(client.request(METHOD, PATH, options))
+  t.truthy(error.message)
+  t.assert(error.message.includes('simulated error'))
+  t.assert(calledUrls.length === 3)
+  t.assert(calledUrls[0] === calledUrls[1])
+  t.assert(calledUrls[0] === calledUrls[2])
+
+  mock.interceptors.forEach(nock.removeInterceptor)
+})
+
 // end client request tests
 
 test('should use default constructor options', t => {