clean up and document HttpError logic

README.md

@@ -83,6 +83,18 @@ Send a DELETE request to `url` and return the callback with an error or JSON des
 
 Otherwise works the same as GET.
 
+## Error handling and bad JSON responses
+
+Server errors (i.e. response codes >= 300) are handled as standard responses. You can get the status code from the response object which is the third argument to the standard callback if you need to handle error responses in a different way.
+
+However, if any type of response returns data that is not JSON format, an error will be generated and passed as the first argument on the callback, with the following customisations:
+
+* If the status code from the server is >= 300, you will receive an error of type `jsonist.HttpError`, otherwise it will be of type `SyntaxError` indicating a bad JSON parse on a normal response.
+* The error will come with the following additional properties attached:
+  - `data`: a `Buffer` containing the full response from the server
+  - `response`: the full HTTP response object
+  - `statusCode`: the status code received from the server (a short-cut to `response.statusCode`)
+
 ## License
 
 **jsonist** is Copyright (c) 2014 Rod Vagg [@rvagg](https://github.com/rvagg) and licensed under the MIT licence. All rights not explicitly granted in the MIT license are reserved. See the included LICENSE file for more details.

jsonist.js

@@ -5,14 +5,12 @@ var url         = require('url')
   , xtend       = require('xtend')
 
 
-function HttpError (status, response) {
-    Error.call(this)
-    this.status = status
-    this.response = response
+function HttpError (message) {
+    SyntaxError.call(this, message)
     Error.captureStackTrace(this, arguments.callee)
 }
 
-HttpError.prototype = Object.create(Error.prototype)
+HttpError.prototype = Object.create(SyntaxError.prototype)
 HttpError.prototype.constructor = HttpError
 
 
@@ -45,20 +43,17 @@ function collector (uri, options, callback) {
       if (!data.length)
         return callback(null, null, request.response)
 
-      var ret
+      var ret, msg
 
       try {
         ret = JSON.parse(data.toString())
       } catch (e) {
-        if (request.response.statusCode >= 300) {
-          err = new HttpError(request.response.statusCode, request.response)
-        } else {
-          err = new SyntaxError('JSON parse error: ' + e.message, e)
-          err.statusCode = request.response.statusCode
-          err.data = data
-          err.response = request.response
-          return callback(err2)
-        }
+        msg = 'JSON parse error: ' + e.message
+        err = request.response.statusCode >= 300 ? new HttpError(msg) : new SyntaxError(msg)
+
+        err.statusCode = request.response.statusCode
+        err.data = data
+        err.response = request.response
 
         return callback(err);
       }
@@ -123,8 +118,8 @@ function isRedirect (request, response) {
 }
 
 
-module.exports.get    = makeMethod('GET'    , false)
-module.exports.post   = makeMethod('POST'   , true)
-module.exports.put    = makeMethod('PUT'    , true)
-module.exports.delete = makeMethod('DELETE' , false)
+module.exports.get       = makeMethod('GET'    , false)
+module.exports.post      = makeMethod('POST'   , true)
+module.exports.put       = makeMethod('PUT'    , true)
+module.exports.delete    = makeMethod('DELETE' , false)
 module.exports.HttpError = HttpError

test.js

@@ -9,7 +9,7 @@ const test      = require('tape')
     , after     = require('after')
 
 
-function testServer (serverResponse) {
+function testServer (serverResponse, statusCode) {
   var ee     = new EE()
     , server = http.createServer(handler)
 
@@ -23,7 +23,10 @@ function testServer (serverResponse) {
       setTimeout(server.close.bind(server), 20)
     }))
 
-    res.writeHead(200, { 'content-type': 'application/json' })
+    res.writeHead(
+        typeof statusCode == 'number' ? statusCode : 200
+      , { 'content-type': 'application/json' }
+    )
     res.end(serverResponse || '')
   }
 
@@ -312,3 +315,51 @@ test('follow redirect limit', function (t) {
     })
   })
 })
+
+
+test('server error, non-JSON', function (t) {
+  t.plan(7)
+
+  var responseText = 'there was an error'
+
+  testServer(responseText, 501)
+    .on('ready', function (url) {
+      jsonist.get(url, function (err, data, response) {
+        t.ok(err)
+        t.ok(err instanceof jsonist.HttpError)
+        t.equal(err.data.toString(), responseText, 'got correct response')
+        t.equal(err.statusCode, 501, 'got correct statusCode')
+      })
+    })
+    .on('request', function (req, data) {
+      t.equal(req.method, 'GET', 'got GET request')
+      t.equal(req.headers['accept'], 'application/json', 'got correct accept header')
+    })
+    .on('close', t.ok.bind(t, true, 'ended'))
+})
+
+
+test('server error, with-JSON', function (t) {
+  t.plan(8)
+
+  var responseDoc = 'there was an error'
+
+  testServer(stringify(responseDoc), 501)
+    .on('ready', function (url) {
+      jsonist.get(url, function (err, data, response) {
+        t.notOk(err, 'no error')
+        t.deepEqual(data, responseDoc, 'got correct doc')
+        t.ok(response, 'got response object')
+        t.equal(response.statusCode, 501, 'got correct status code')
+        t.equal(
+            response && response.headers && response.headers['content-type']
+          , 'application/json', 'verified response object by content-type header'
+        )
+      })
+    })
+    .on('request', function (req, data) {
+      t.equal(req.method, 'GET', 'got GET request')
+      t.equal(req.headers['accept'], 'application/json', 'got correct accept header')
+    })
+    .on('close', t.ok.bind(t, true, 'ended'))
+})