make any non-successful HTTP response result in an error argument

Any completed response (non network error) also results in the error
argument being provided to the end callback. This make it easier to
handle successful responses versus failed responses. Along with the
error argument, the response argument is also supplied.

This behavior does not currently extend to `pipe()`, only to buffered
responses that are parsed in full.

see this issue for context:
#283

lib/client.js

@@ -298,6 +298,7 @@ function Response(req, options) {
   this.text = (this.req.method !='HEAD' && (this.xhr.responseType === '' || this.xhr.responseType === 'text'))
      ? this.xhr.responseText
      : null;
+  this.statusText = this.req.xhr.statusText;
   this.setStatusProperties(this.xhr.status);
   this.header = this.headers = parseHeader(this.xhr.getAllResponseHeaders());
   // getAllResponseHeaders sometimes falsely returns "" for CORS requests, but
@@ -468,7 +469,18 @@ function Request(method, url) {
       self.emit('response', res);
     }
 
-    self.callback(err, res);
+    if (res && res.status >= 200 && res.status < 300) {
+      return self.callback(err, res);
+    }
+
+    var msg = 'Unsuccessful HTTP response';
+    if (res) {
+      msg = res.statusText || msg;
+    }
+    var new_err = new Error(msg);
+    new_err.original = err;
+
+    self.callback(err || new_err, res);
   });
 }
 

lib/node/index.js

@@ -767,7 +767,19 @@ Request.prototype.callback = function(err, res){
   // only emit error event if there is a listener
   // otherwise we assume the callback to `.end()` will get the error
   if (err && this.listeners('error').length > 0) this.emit('error', err);
-  fn(err, res);
+
+  if (res && res.status >= 200 && res.status < 300) {
+    return fn(err, res);
+  }
+
+  var msg = 'Unsuccessful HTTP response';
+  if (res) {
+    msg = http.STATUS_CODES[res.status] || msg;
+  }
+  var new_err = new Error(msg);
+  new_err.original = err;
+
+  fn(err || new_err, res);
 };
 
 /**

test/basic.js

@@ -51,6 +51,8 @@ describe('request', function(){
           res.error.message.should.equal('cannot GET ' + uri + '/error (500)');
         }
         assert(res.error.status === 500);
+        assert(err, 'should have an error for 500');
+        assert.equal(err.message, 'Internal Server Error');
         done();
       });
     })
@@ -83,6 +85,7 @@ describe('request', function(){
       request
       .get(uri + '/login')
       .end(function(err, res){
+        assert(!err, 'should not have an error for success responses');
         assert(200 == res.status);
         assert(2 == res.statusType);
         done();

test/client/request.js

@@ -30,6 +30,7 @@ it('request() simple HEAD', function(next){
 
 it('request() error object', function(next) {
   request('GET', '/error').end(function(err, res) {
+    assert(err);
     assert(res.error, 'response should be an error');
     assert(res.error.message == 'cannot GET /error (500)');
     assert(res.error.status == 500);
@@ -41,6 +42,8 @@ it('request() error object', function(next) {
 
 it('request() GET 5xx', function(next){
   request('GET', '/error').end(function(err, res){
+    assert(err);
+    assert(err.message == 'Internal Server Error');
     assert(!res.ok, 'response should not be ok');
     assert(res.error, 'response should be an error');
     assert(!res.clientError, 'response should not be a client error');
@@ -51,6 +54,8 @@ it('request() GET 5xx', function(next){
 
 it('request() GET 4xx', function(next){
   request('GET', '/notfound').end(function(err, res){
+    assert(err);
+    assert.equal(err.message, 'Not Found');
     assert(!res.ok, 'response should not be ok');
     assert(res.error, 'response should be an error');
     assert(res.clientError, 'response should be a client error');
@@ -61,27 +66,31 @@ it('request() GET 4xx', function(next){
 
 it('request() GET 404 Not Found', function(next){
   request('GET', '/notfound').end(function(err, res){
+    assert(err);
     assert(res.notFound, 'response should be .notFound');
     next();
   });
 });
 
 it('request() GET 400 Bad Request', function(next){
   request('GET', '/bad-request').end(function(err, res){
+    assert(err);
     assert(res.badRequest, 'response should be .badRequest');
     next();
   });
 });
 
 it('request() GET 401 Bad Request', function(next){
   request('GET', '/unauthorized').end(function(err, res){
+    assert(err);
     assert(res.unauthorized, 'response should be .unauthorized');
     next();
   });
 });
 
 it('request() GET 406 Not Acceptable', function(next){
   request('GET', '/not-acceptable').end(function(err, res){
+    assert(err);
     assert(res.notAcceptable, 'response should be .notAcceptable');
     next();
   });
@@ -96,6 +105,7 @@ it('request() GET 204 No Content', function(next){
 
 it('request() header parsing', function(next){
   request('GET', '/notfound').end(function(err, res){
+    assert(err);
     assert('text/html; charset=utf-8' == res.header['content-type']);
     assert('Express' == res.header['x-powered-by']);
     next();
@@ -104,6 +114,7 @@ it('request() header parsing', function(next){
 
 it('request() .status', function(next){
   request('GET', '/notfound').end(function(err, res){
+    assert(err);
     assert(404 == res.status, 'response .status');
     assert(4 == res.statusType, 'response .statusType');
     next();
@@ -112,6 +123,7 @@ it('request() .status', function(next){
 
 it('get()', function(next){
   request.get('/notfound').end(function(err, res){
+    assert(err);
     assert(404 == res.status, 'response .status');
     assert(4 == res.statusType, 'response .statusType');
     next();

test/node/agency.js

@@ -66,7 +66,7 @@ describe('request', function() {
       agent1
         .get('http://localhost:4000/dashboard')
         .end(function(err, res) {
-          should.not.exist(err);
+          should.exist(err);
           res.should.have.status(401);
           should.exist(res.headers['set-cookie']);
           done();
@@ -99,7 +99,7 @@ describe('request', function() {
       agent2
         .get('http://localhost:4000/dashboard')
         .end(function(err, res) {
-          should.not.exist(err);
+          should.exist(err);
           res.should.have.status(401);
           done();
         });
@@ -144,7 +144,7 @@ describe('request', function() {
         .get('http://localhost:4000/')
         .redirects(0)
         .end(function(err, res) {
-          should.not.exist(err);
+          should.exist(err);
           res.should.have.status(302);
           res.redirects.should.eql([]);
           res.header.location.should.equal('/dashboard');
@@ -167,7 +167,7 @@ describe('request', function() {
       agent1
         .get('http://localhost:4000/dashboard')
         .end(function(err, res) {
-          should.not.exist(err);
+          should.exist(err);
           res.should.have.status(401);
           should.not.exist(res.headers['set-cookie']);
           done();

test/node/flags.js

@@ -38,6 +38,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/notfound')
       .end(function(err, res){
+        assert(err);
         assert(!res.ok, 'response should not be ok');
         assert(res.error, 'response should be an error');
         assert(res.clientError, 'response should be a client error');
@@ -52,6 +53,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/error')
       .end(function(err, res){
+        assert(err);
         assert(!res.ok, 'response should not be ok');
         assert(!res.notFound, 'response should not be notFound');
         assert(res.error, 'response should be an error');
@@ -67,6 +69,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/notfound')
       .end(function(err, res){
+        assert(err);
         assert(res.notFound, 'response should be .notFound');
         done();
       });
@@ -78,6 +81,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/bad-request')
       .end(function(err, res){
+        assert(err);
         assert(res.badRequest, 'response should be .badRequest');
         done();
       });
@@ -89,6 +93,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/unauthorized')
       .end(function(err, res){
+        assert(err);
         assert(res.unauthorized, 'response should be .unauthorized');
         done();
       });
@@ -100,6 +105,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/not-acceptable')
       .end(function(err, res){
+        assert(err);
         assert(res.notAcceptable, 'response should be .notAcceptable');
         done();
       });
@@ -111,6 +117,7 @@ describe('flags', function(){
       request
       .get('http://localhost:3004/no-content')
       .end(function(err, res){
+        assert(!err);
         assert(res.noContent, 'response should be .noContent');
         done();
       });