Merge pull request #164 from kanongil/boom-errors-fix

Fix boom response error handling
hapijs · Mar 23, 2017 · 1544728 · 1544728
2 parents 901d521 + b661859
commit 1544728
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 28 deletions.
diff --git a/README.md b/README.md
@@ -119,7 +119,7 @@ Initiate an HTTP request.
       to force SSL version 3. The possible values depend on your installation of OpenSSL. Read the official OpenSSL docs
       for possible [SSL_METHODS](http://www.openssl.org/docs/ssl/ssl.html#DEALING_WITH_PROTOCOL_METHODS).
 - `callback` - The optional callback function using the signature `function (err, response)` where:
-    - `err` - Any error that may have occurred during the handling of the request or a Boom error object if the response has an error status code.
+    - `err` - Any error that may have occurred during the handling of the request.
     - `response` - The [HTTP Incoming Message](http://nodejs.org/api/http.html#http_http_incomingmessage)
        object, which is also a readable stream.
 

diff --git a/lib/index.js b/lib/index.js
@@ -219,10 +219,6 @@ internals.Client.prototype.request = function (method, url, options, callback, _
             req.abort();
         }
 
-        if (!err && res.statusCode >= 400) {
-            err = Boom.create(res.statusCode, res.statusMessage);
-        }
-
         req.removeListener('response', onResponse);
         req.removeListener('error', onError);
         req.on('error', Hoek.ignore);
@@ -524,6 +520,14 @@ internals.Client.prototype._shortcut = function (method, uri, options, callback)
 
         this.read(res, options, (err, payload) => {
 
+            if (!err && res.statusCode >= 400) {
+                return callback(Boom.create(res.statusCode, 'Response Error: ' + res.statusMessage, {
+                    isResponseError: true,
+                    headers: res.headers,
+                    payload
+                }));
+            }
+
             return callback(err, res, payload);
         });
     });

diff --git a/test/index.js b/test/index.js
@@ -1062,27 +1062,6 @@ describe('request()', () => {
         server.listen(0);
     });
 
-    it('handles error responses with a boom error object', (done) => {
-
-        const server = Http.createServer((req, res) => {
-
-            res.writeHead(400);
-            res.end();
-        });
-
-        server.once('listening', () => {
-
-            Wreck.request('get', 'http://127.0.0.1:' + server.address().port, { payload: '' }, (err) => {
-
-                expect(err.isBoom).to.be.true();
-                expect(err.message).to.equal('Bad Request');
-                done();
-            });
-        });
-
-        server.listen(0);
-    });
-
     it('handles request errors with a boom response when payload is being sent', (done) => {
 
         const server = Http.createServer((req, res) => {
@@ -1428,8 +1407,7 @@ describe('request()', () => {
 
             Wreck.request('post', 'http://localhost:' + server.address().port, { headers: { connection: 'close' }, payload: null }, (err, res) => {
 
-                expect(err).exist();
-                expect(err.isBoom).to.be.true();
+                expect(err).to.not.exist();
                 Wreck.read(res, null, (err, body) => {
 
                     expect(err).to.not.exist();
@@ -2070,6 +2048,33 @@ describe('Shortcut', () => {
             });
         });
     });
+
+    it('handles error responses with a boom error object', (done) => {
+
+        const server = Http.createServer((req, res) => {
+
+            res.setHeader('content-type', 'application/json');
+            res.setHeader('x-custom', 'yes');
+            res.writeHead(400);
+            res.end(JSON.stringify({ details: 'failed' }));
+        });
+
+        server.once('listening', () => {
+
+            Wreck.get('http://127.0.0.1:' + server.address().port, { json: true }, (err) => {
+
+                expect(err.isBoom).to.be.true();
+                expect(err.message).to.equal('Response Error: Bad Request');
+                expect(err.data.isResponseError).to.be.true();
+                expect(err.data.headers).to.include({ 'x-custom': 'yes' });
+                expect(err.data.payload).to.equal({ details: 'failed' });
+                done();
+            });
+        });
+
+        server.listen(0);
+    });
+
 });
 
 describe('json', () => {