Reject promises on 4XX and 5XX responses (#1)

Author: rlewan

.gitignore

@@ -3,3 +3,5 @@ npm-debug.log
 .env
 coverage
 .vscode
+.idea
+

helpers/isErrorResponse.js

@@ -0,0 +1,11 @@
+/**
+ * Checks if the returned response is an error response.
+ *
+ * @param response a Node's http.IncomingMessage object
+ * @return boolean true if the response status is 4XX or 5XX
+ */
+function isErrorResponse(response) {
+  return response.statusCode >= 400;
+}
+
+module.exports = isErrorResponse;

index.js

@@ -11,6 +11,7 @@ var extend = require('extend');
 var when = require('when');
 var request = require('request');
 var RetryStrategies = require('./strategies');
+var isErrorResponse = require('./helpers/isErrorResponse')
 var _ = require('lodash');
 
 var DEFAULTS = {
@@ -109,6 +110,10 @@ function Request(url, options, f, retryConfig) {
       return this._reject(err);
     }
 
+    if (isErrorResponse(response)) {
+      return this._reject(this.fullResponse ? response : body);
+    }
+
     // resolve with the full response or just the body
     response = this.fullResponse ? response : body;
     this._resolve(response);

package-lock.json

@@ -1,7 +1,7 @@
 {
-  "name": "requestretry",
+  "name": "@hmcts/requestretry",
   "description": "request-retry wrap nodejs request to retry http(s) requests in case of error",
-  "version": "1.13.0",
+  "version": "1.0.0",
   "author": {
     "name": "Francois-Guillaume Ribreau",
     "email": "npm@fgribreau.com",
@@ -18,7 +18,8 @@
     }
   ],
   "repository": {
-    "url": "https://github.com/FGRibreau/node-request-retry"
+    "type": "git",
+    "url": "https://github.com/hmcts/node-request-retry"
   },
   "main": "index.js",
   "scripts": {
@@ -55,6 +56,7 @@
     "coveralls": "^2.11.12",
     "kew": "~0.7.0",
     "mocha": "^3.0.2",
+    "nock": "^9.2.3",
     "nyc": "^10.0.0",
     "q": "~1.4.1",
     "rsvp": "^3.2.1",

package.json

@@ -0,0 +1,18 @@
+'use strict';
+
+const isErrorResponse = require('../../helpers/isErrorResponse');
+const expect = require('chai').expect;
+
+describe('isErrorResponse', function () {
+  [200, 201, 302].forEach(statusCode => {
+    it(`should return false for ${statusCode} statusCode`, function () {
+      expect(isErrorResponse({ statusCode: statusCode })).to.equal(false);
+    });
+  });
+
+  [400, 401, 403, 422, 500, 502].forEach(statusCode => {
+    it(`should return true for ${statusCode} statusCode`, function () {
+      expect(isErrorResponse({ statusCode: statusCode })).to.equal(true);
+    });
+  });
+});

test/helpers/isErrorResponse.test.js

@@ -2,6 +2,7 @@
 
 var request = require('../');
 var t = require('chai').assert;
+var nock = require('nock')
 
 describe('Promises support', function () {
 
@@ -84,6 +85,38 @@ describe('Promises support', function () {
     });
   });
 
+  it('should reject the promise on 400 response', function (done) {
+    nock('http://some-test-host')
+      .get('/return-400')
+      .reply(400, 'Client Error');
+
+    request({
+      url: 'http://some-test-host/return-400',
+      maxAttempts: 1,
+      retryStrategy: request.RetryStrategies.HTTPOrNetworkError
+    })
+      .catch(function (err) {
+        t.strictEqual(err.body, 'Client Error');
+        done();
+      });
+  });
+
+  it('should reject the promise on 500 response', function (done) {
+    nock('http://some-test-host')
+      .get('/return-500')
+      .reply(500, 'Server Error');
+
+    request({
+      url: 'http://some-test-host/return-500',
+      maxAttempts: 1,
+      retryStrategy: request.RetryStrategies.HTTPOrNetworkError
+    })
+      .catch(function (err) {
+        t.strictEqual(err.body, 'Server Error');
+        done();
+      });
+  });
+
   it('should still work with callbacks', function (done) {
     request({url: 'http://www.filltext.com/?rows=1'}, function requestCallback(err, response, body) {
       t.strictEqual(response.statusCode, 200);
@@ -108,10 +141,6 @@ describe('Promises support', function () {
         promiseFactory: promiseFactoryFn // custom promise factory function
       })
       .then(function (body) {
-        if (throwError) {
-          throw body; // To simulate an error in the request
-        }
-
         t.isString(body);
         var data = JSON.parse(body);
         t.isArray(data);