search

joepie91 / node-bhttp

A sane HTTP client library for Node.js with Streams2 support.
62 stars 12 forks source link

Error while decoding JSON on empty responses #11

Open brendon-codes opened 9 years ago

brendon-codes commented 9 years ago

If decodeJSON is set to true in a request, and the response has a status code of 200 with empty response, I am getting this error:

Unhandled rejection SyntaxError: Unexpected end of input
    at Object.parse (native)
    at /FOOBAR/node_modules/bhttp/lib/bhttp.js:546:38
    at ConcatStream.<anonymous> (/FOOBAR/node_modules/bhttp/node_modules/concat-stream/index.js:36:43)
    at emitNone (events.js:72:20)
    at ConcatStream.emit (events.js:166:7)
    at finishMaybe (/FOOBAR/node_modules/bhttp/node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:502:14)
    at endWritable (/FOOBAR/node_modules/bhttp/node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:512:3)
    at ConcatStream.Writable.end (/FOOBAR/node_modules/bhttp/node_modules/concat-stream/node_modules/readable-stream/lib/_stream_writable.js:477:5)
    at IncomingMessage.onend (_stream_readable.js:490:10)
    at IncomingMessage.g (events.js:260:16)
    at emitNone (events.js:72:20)
    at IncomingMessage.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:893:12)
    at doNTCallback2 (node.js:429:9)
    at process._tickCallback (node.js:343:17)
joepie91 commented 9 years ago

That is actually correct behaviour; an empty string is not valid JSON (and is not equivalent to eg. null or {}), so it should fail.

decodeJSON is a flag to force bhttp to parse the response as JSON, no matter what the headers say, and is primarily meant for APIs sending incorrect headers (this is a common issue) - if the server sends along the correct Content-Type header, then you won't need to specify it to begin with, as bhttp will auto-detect it.

If the API you're talking to returns an empty response with a Content-Type: text/json header, then the API itself is broken - when specifying that content type, it must return a valid JSON response.

I've had some issues with Tahoe-LAFS' web API in the past, which wouldn't send along a valid Content-Type header, but would respond with JSON on success and with plain text on failure. In such edge cases, the ideal solution is simply to JSON.parse the response yourself based on eg. the status code. Such cases are too unusual to handle automatically, and therefore left up to the user.

brendon-codes commented 9 years ago

In my original post, I actually meant to say that it is failing on a 204 status code. Isn't it reasonable to think that most users would expect that a 204 empty response should not be parsed as JSON?

joepie91 commented 9 years ago

Ah yes, in the case of a 204, it should indeed not try to parse the response, so that's a bug.

Can you provide an example URL, by any chance?

brendon-codes commented 9 years ago

So far, I have been using it with a private cloud service, that I cannot share. I will try to find another public example.