Add support for DELETE to RequestLib

app/lib/request.lib.js

@@ -68,6 +68,20 @@ function defaultOptions () {
   }
 }
 
+/**
+ * Make a DELETE request to the specified URL
+ *
+ * > Note: This function has been called `deleteRequest` here rather than `delete` as `delete` is a reserved word.
+ *
+ * @param {string} url - The full URL that you wish to connect to
+ * @param {Object} additionalOptions - Append to or replace the options passed to Got when making the request
+ *
+ * @returns {Promise<Object>} The result of the request; whether it succeeded and the response or error returned
+ */
+async function deleteRequest (url, additionalOptions = {}) {
+  return _sendRequest('delete', url, additionalOptions)
+}
+
 /**
  * Make a GET request to the specified URL
  *
@@ -84,8 +98,8 @@ function defaultOptions () {
  * service responds, `response:` will be the full response Got returns. In the event of a network error `response:` will
  * be a Got error instance.
  *
- * @param {string} url The full URL that you wish to connect to
- * @param {Object} additionalOptions Append to or replace the options passed to Got when making the request
+ * @param {string} url - The full URL that you wish to connect to
+ * @param {Object} additionalOptions - Append to or replace the options passed to Got when making the request
  *
  * @returns {Promise<Object>} The result of the request; whether it succeeded and the response or error returned
  */
@@ -125,10 +139,10 @@ async function _importGot () {
  * Errbit instance. We also output the full response to the log as it will be the Got error containing all the info
  * we need to diagnose the problem.
  *
- * @param {string} method the type of request made, for example, 'GET', 'POST, or 'PATCH'
- * @param {Object} result the result object we generate
- * @param {*} url the requested url
- * @param {*} additionalOptions any additional options that were passed to Got by the calling service
+ * @param {string} method - the type of request made, for example, 'GET', 'POST, or 'PATCH'
+ * @param {Object} result - the result object we generate
+ * @param {string} url - the requested url
+ * @param {Object} additionalOptions - any additional options that were passed to Got by the calling service
  */
 function _logFailure (method, result, url, additionalOptions) {
   const data = {
@@ -160,7 +174,7 @@ function _logFailure (method, result, url, additionalOptions) {
  *
  * Those that use this module can add to, extend or replace the options we pass to Got when a request is made.
  *
- * @param {Object} additionalOptions Object of custom options
+ * @param {Object} additionalOptions - Object of custom options
  */
 function _requestOptions (additionalOptions) {
   return { ...defaultOptions(), ...additionalOptions }
@@ -194,6 +208,7 @@ async function _sendRequest (method, url, additionalOptions) {
 }
 
 module.exports = {
+  delete: deleteRequest,
   get,
   patch,
   post,

test/lib/request.lib.test.js

@@ -49,6 +49,274 @@ describe('RequestLib', () => {
     delete global.GlobalNotifier
   })
 
+  describe('#delete()', () => {
+    describe('When a request succeeds', () => {
+      beforeEach(() => {
+        Nock(testDomain).delete('/').reply(200, { data: 'hello world' })
+      })
+
+      describe('the result it returns', () => {
+        it("has a 'succeeded' property marked as true", async () => {
+          const result = await RequestLib.delete(testDomain)
+
+          expect(result.succeeded).to.be.true()
+        })
+
+        it("has a 'response' property containing the web response", async () => {
+          const result = await RequestLib.delete(testDomain)
+
+          expect(result.response).to.exist()
+          expect(result.response.statusCode).to.equal(200)
+          expect(result.response.body).to.equal('{"data":"hello world"}')
+        })
+      })
+    })
+
+    describe('When a request fails', () => {
+      describe('because the response was a 500', () => {
+        beforeEach(() => {
+          Nock(testDomain).delete('/').reply(500, { data: 'hello world' })
+        })
+
+        it('logs the failure', async () => {
+          await RequestLib.delete(testDomain)
+
+          const logDataArg = notifierStub.omg.args[0][1]
+
+          expect(notifierStub.omg.calledWith('DELETE request failed')).to.be.true()
+          expect(logDataArg.method).to.equal('DELETE')
+          expect(logDataArg.url).to.equal('http://example.com')
+          expect(logDataArg.additionalOptions).to.equal({})
+          expect(logDataArg.result.succeeded).to.be.false()
+          expect(logDataArg.result.response).to.equal({
+            statusCode: 500,
+            body: '{"data":"hello world"}'
+          })
+        })
+
+        describe('the result it returns', () => {
+          it("has a 'succeeded' property marked as false", async () => {
+            const result = await RequestLib.delete(testDomain)
+
+            expect(result.succeeded).to.be.false()
+          })
+
+          it("has a 'response' property containing the web response", async () => {
+            const result = await RequestLib.delete(testDomain)
+
+            expect(result.response).to.exist()
+            expect(result.response.statusCode).to.equal(500)
+            expect(result.response.body).to.equal('{"data":"hello world"}')
+          })
+        })
+      })
+
+      describe('because there was a network issue', () => {
+        describe('and all retries fail', () => {
+          beforeEach(async () => {
+            Nock(testDomain)
+              .delete(() => true)
+              .replyWithError({ code: 'ECONNRESET' })
+              .persist()
+          })
+
+          it('logs when a retry has happened', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            expect(notifierStub.omg.callCount).to.equal(2)
+            expect(notifierStub.omg.alwaysCalledWith('Retrying HTTP request')).to.be.true()
+          })
+
+          it('logs and records the error', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            const logDataArg = notifierStub.omfg.args[0][1]
+
+            expect(notifierStub.omfg.calledWith('DELETE request errored')).to.be.true()
+            expect(logDataArg.method).to.equal('DELETE')
+            expect(logDataArg.url).to.equal('http://example.com')
+            expect(logDataArg.additionalOptions).to.exist()
+            expect(logDataArg.result.succeeded).to.be.false()
+            expect(logDataArg.result.response).to.be.an.error()
+          })
+
+          describe('the result it returns', () => {
+            it("has a 'succeeded' property marked as false", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.succeeded).to.be.false()
+            })
+
+            it("has a 'response' property containing the error", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.response).to.exist()
+              expect(result.response).to.be.an.error()
+              expect(result.response.code).to.equal('ECONNRESET')
+            })
+          })
+        })
+
+        describe('and a retry succeeds', () => {
+          beforeEach(async () => {
+            // The first response will error, the second response will return OK
+            Nock(testDomain)
+              .delete(() => true)
+              .replyWithError({ code: 'ECONNRESET' })
+              .delete(() => true)
+              .reply(200, { data: 'econnreset hello world' })
+          })
+
+          it('logs when a retry has happened', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            expect(notifierStub.omg.callCount).to.equal(1)
+            expect(notifierStub.omg.alwaysCalledWith('Retrying HTTP request')).to.be.true()
+          })
+
+          describe('the result it returns', () => {
+            it("has a 'succeeded' property marked as true", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.succeeded).to.be.true()
+            })
+
+            it("has a 'response' property containing the web response", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.response).to.exist()
+              expect(result.response.statusCode).to.equal(200)
+              expect(result.response.body).to.equal('{"data":"econnreset hello world"}')
+            })
+          })
+        })
+      })
+
+      describe('because request timed out', () => {
+        beforeEach(async () => {
+          // Set the timeout value to 50ms for these tests
+          Sinon.replace(requestConfig, 'timeout', 50)
+        })
+
+        // Because of the fake delay in this test, Lab will timeout (by default tests have 2 seconds to finish). So, we
+        // have to override the timeout for this specific test to all it to complete
+        describe('and all retries fail', { timeout: 5000 }, () => {
+          beforeEach(async () => {
+            Nock(testDomain)
+              .delete(() => true)
+              .delay(100)
+              .reply(200, { data: 'hello world' })
+              .persist()
+          })
+
+          it('logs when a retry has happened', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            expect(notifierStub.omg.callCount).to.equal(2)
+            expect(notifierStub.omg.alwaysCalledWith('Retrying HTTP request')).to.be.true()
+          })
+
+          it('logs and records the error', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            const logDataArg = notifierStub.omfg.args[0][1]
+
+            expect(notifierStub.omfg.calledWith('DELETE request errored')).to.be.true()
+            expect(logDataArg.method).to.equal('DELETE')
+            expect(logDataArg.url).to.equal('http://example.com')
+            expect(logDataArg.additionalOptions).to.exist()
+            expect(logDataArg.result.succeeded).to.be.false()
+            expect(logDataArg.result.response).to.be.an.error()
+          })
+
+          describe('the result it returns', () => {
+            it("has a 'succeeded' property marked as false", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.succeeded).to.be.false()
+            })
+
+            it("has a 'response' property containing the error", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.response).to.exist()
+              expect(result.response).to.be.an.error()
+              expect(result.response.code).to.equal('ETIMEDOUT')
+            })
+          })
+        })
+
+        describe('and a retry succeeds', () => {
+          beforeEach(async () => {
+            // The first response will time out, the second response will return OK
+            Nock(testDomain)
+              .delete(() => true)
+              .delay(100)
+              .reply(200, { data: 'hello world' })
+              .delete(() => true)
+              .reply(200, { data: 'delayed hello world' })
+          })
+
+          it('logs when a retry has happened', async () => {
+            await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+            expect(notifierStub.omg.callCount).to.equal(1)
+            expect(notifierStub.omg.alwaysCalledWith('Retrying HTTP request')).to.be.true()
+          })
+
+          describe('the result it returns', () => {
+            it("has a 'succeeded' property marked as true", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.succeeded).to.be.true()
+            })
+
+            it("has a 'response' property containing the web response", async () => {
+              const result = await RequestLib.delete(testDomain, { retry: shortBackoffLimitRetryOptions })
+
+              expect(result.response).to.exist()
+              expect(result.response.statusCode).to.equal(200)
+              expect(result.response.body).to.equal('{"data":"delayed hello world"}')
+            })
+          })
+        })
+      })
+    })
+
+    describe('When I provide additional options', () => {
+      describe('and they expand the default options', () => {
+        beforeEach(() => {
+          Nock(testDomain).delete('/').reply(200, { data: 'hello world' })
+        })
+
+        it('adds them to the options used when making the request', async () => {
+          // We tell Got to return the response as json rather than text. We can confirm the option has been applied by
+          // checking the result.response.body below.
+          const options = { responseType: 'json' }
+          const result = await RequestLib.delete(testDomain, options)
+
+          expect(result.succeeded).to.be.true()
+          expect(result.response.body).to.equal({ data: 'hello world' })
+        })
+      })
+
+      describe('and they replace a default option', () => {
+        beforeEach(() => {
+          Nock(testDomain).delete('/').reply(500)
+        })
+
+        it('uses them in the options used when making the request', async () => {
+          // We tell Got throw an error if the response is not 2xx or 3xx
+          const options = { throwHttpErrors: true }
+          const result = await RequestLib.delete(testDomain, options)
+
+          expect(result.succeeded).to.be.false()
+          expect(result.response).to.be.an.error()
+        })
+      })
+    })
+  })
+
   describe('#get()', () => {
     describe('When a request succeeds', () => {
       beforeEach(() => {