switch from request-promise to needle (#78)
Switch from request-promise to needle, for an initialization performance boost. Fixes #77
Also adds test coverage for 404 and 409 cases. Fixes #79
* switch from request-promise to needle
* remove needless clause
* handle errors for needle
* improve comments of rp facade code
* improve error handling test coverage
* fix for 409 test
diff --git a/lib/client.js b/lib/client.js
index b6666e8..73dfe4a 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -5,10 +5,56 @@
const messages = require('./messages')
const OpenWhiskError = require('./openwhisk_error')
-const rp = require('request-promise')
+const needle = require('needle')
const url = require('url')
const http = require('http')
+/**
+ * This implements a request-promise-like facade over the needle
+ * library. There are two gaps between needle and rp that need to be
+ * bridged: 1) convert `qs` into a query string; and 2) convert
+ * needle's non-excepting >=400 statusCode responses into exceptions
+ *
+ */
+const rp = opts => {
+ 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}`
+ }
+ }
+ }
+
+ // 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,
+ opts.body || opts.params,
+ opts)
+ .then(resp => {
+ if (resp.statusCode >= 400) {
+ // we turn >=400 statusCode responses into exceptions
+ const error = new Error(resp.body.error || resp.statusMessage)
+ error.statusCode = resp.statusCode // the http status code
+ error.options = opts // the code below requires access to the input opts
+ error.error = resp.body // the error body
+ throw error
+ } else {
+ // otherwise, the response body is the expected return value
+ return resp.body
+ }
+ })
+}
+
class Client {
constructor (options) {
this.options = this.parse_options(options || {})
diff --git a/package.json b/package.json
index 105f154..e856296 100644
--- a/package.json
+++ b/package.json
@@ -42,7 +42,7 @@
"proxyquire": "1.7.4"
},
"dependencies": {
- "request-promise": "^2.0.1",
+ "needle": "^2.0.1",
"@types/node": "^8.0.26",
"@types/swagger-schema-official": "^2.0.6"
},
diff --git a/test/integration/actions.test.js b/test/integration/actions.test.js
index b1e98e2..d61bd94 100644
--- a/test/integration/actions.test.js
+++ b/test/integration/actions.test.js
@@ -50,6 +50,29 @@
})
})
+test('get a non-existing action, expecting 404', async t => {
+ const actions = new Actions(new Client(options))
+ await actions.get({name: 'glorfindel'}).catch(err => {
+ t.is(err.statusCode, 404)
+ })
+})
+
+test('delete a non-existing action, expecting 404', async t => {
+ const actions = new Actions(new Client(options))
+ await actions.delete({name: 'glorfindel'}).catch(err => {
+ t.is(err.statusCode, 404)
+ })
+})
+
+test('create with an existing action, expecting 409', async t => {
+ const actions = new Actions(new Client(options))
+ await actions.create({name: 'glorfindel2', action: 'x=>x'})
+ .then(() => actions.create({name: 'glorfindel2', action: 'x=>x'}))
+ .catch(err => {
+ t.is(err.statusCode, 409)
+ })
+})
+
test('create, get and delete an action', t => {
const errors = err => {
console.log(err)
diff --git a/test/integration/packages.test.js b/test/integration/packages.test.js
index 7bb7ffe..d0a66e0 100644
--- a/test/integration/packages.test.js
+++ b/test/integration/packages.test.js
@@ -49,6 +49,13 @@
})
})
+test('get a non-existing package, expecting 404', async t => {
+ const packages = new Packages(new Client(options))
+ await packages.get({name: 'glorfindel'}).catch(err => {
+ t.is(err.statusCode, 404)
+ })
+})
+
test('create, get and delete an package', t => {
const errors = err => {
console.log(err)
diff --git a/test/integration/rules.test.js b/test/integration/rules.test.js
index b1e5f84..434bab9 100644
--- a/test/integration/rules.test.js
+++ b/test/integration/rules.test.js
@@ -51,6 +51,13 @@
})
})
+test('get a non-existing rule, expecting 404', async t => {
+ const rules = new Rules(new Client(options))
+ await rules.get({name: 'glorfindel'}).catch(err => {
+ t.is(err.statusCode, 404)
+ })
+})
+
// Running update tests conconcurrently leads to resource conflict errors.
test.serial('create, get and delete a rule', t => {
const errors = err => {
diff --git a/test/integration/triggers.test.js b/test/integration/triggers.test.js
index d49f08c..4b68e2b 100644
--- a/test/integration/triggers.test.js
+++ b/test/integration/triggers.test.js
@@ -49,6 +49,13 @@
})
})
+test('get a non-existing trigger, expecting 404', async t => {
+ const triggers = new Triggers(new Client(options))
+ await triggers.get({name: 'glorfindel'}).catch(err => {
+ t.is(err.statusCode, 404)
+ })
+})
+
test('create, get and delete an trigger', t => {
const errors = err => {
console.log(err)
