nervous-systems / kvlt

Multi-target Clojure/script HTTP client
The Unlicense
69 stars 8 forks source link

JSON parser throws exception on bad HTTP response when using {:as :json} #6

Closed ingesolvoll closed 8 years ago

ingesolvoll commented 8 years ago

Obviously, a 404 response won't contain valid JSON. If I skip the {:as :json} part and use a (p/catch) for the request, I get a nice promise containing info about 404. When using p/catch with :as :json, the info about 404 is lost.

Unhandled rejection SyntaxError: Unexpected end of JSON input at Object.parse (native) at kvlt$platform$util$parse_json (http://localhost:3449/js/compiled/kvlt/platform/util.js:15:83) at http://localhost:3449/js/compiled/kvlt/middleware.js:103:123 at cljs.core.MultiFn.call.G108392 (http://localhost:3449/js/compiled/cljs/core.js:33327:106) at cljs.core.MultiFn.call.G10839 as call at http://localhost:3449/js/compiled/kvlt/middleware.js:222:42 at cljs.core.MultiFn.cljs$core$IFn$_invoke$arity$1 (http://localhost:3449/js/compiled/cljs/core.js:33693:106) at cljs.core.comp.cljs$core$IFn$_invoke$arity$2.G91121 (http://localhost:3449/js/compiled/cljs/core.js:14017:52) at cljs.core.comp.cljs$core$IFn$_invoke$arity$2.G__9112 (http://localhost:3449/js/compiled/cljs/core.js:14061:19) at http://localhost:3449/js/compiled/cats/core.js:213:14 at http://localhost:3449/js/compiled/promesa/core.js:474:11 From previous event: at Promise.promesa.core.Promise.promesa$core$IPromise$_bind$arity$2 (http://localhost:3449/js/compiled/promesa/core.js:472:15) at promesa$core$_bind (http://localhost:3449/js/compiled/promesa/core.js:284:10) at promesa.monad.promise_context.promesa.monad.t_promesa$monad42506.cats$protocols$Monad$_mbind$arity$3 (http://localhost:3449/js/compiled/promesa/monad.js:88:27) at cats$protocols$_mbind (http://localhost:3449/js/compiled/cats/protocols.js:405:10) at cats$core$bind (http://localhost:3449/js/compiled/cats/core.js:202:30) at http://localhost:3449/js/compiled/cljs/core.js:7624:96 at Function.cljs.core.seq_reduce.cljs$core$IFn$_invoke$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:7625:3) at cljs.core.Cons.cljs$core$IReduce$_reduce$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:10490:29) at Function.cljs.core.reduce.cljs$core$IFn$_invoke$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:7718:13) at cljs$core$reduce (http://localhost:3449/js/compiled/cljs/core.js:7686:25) at Function.cats.core._GTGTEQ_.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/cats/core.js:1246:25) at cats$core$GTGTEQ (http://localhost:3449/js/compiled/cats/core.js:1236:31) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:31) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic (http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53)

moea commented 8 years ago

Thanks, i'll take care of this ASAP.

On Mon, May 30, 2016 at 12:27 PM, ingesolvoll notifications@github.com wrote:

Obviously, a 404 response won't contain valid JSON. If I skip the {:as :json} part and use a (p/catch) for the request, I get a nice promise containing info about 404. When using p/catch with :as :json, the info about 404 is lost.

Unhandled rejection SyntaxError: Unexpected end of JSON input at Object.parse (native) at kvlt$platform$util$parse_json ( http://localhost:3449/js/compiled/kvlt/platform/util.js:15:83) at http://localhost:3449/js/compiled/kvlt/middleware.js:103:123 at cljs.core.MultiFn.call.G108392 ( http://localhost:3449/js/compiled/cljs/core.js:33327:106) at cljs.core.MultiFn.call.G__10839 as call http://localhost:3449/js/compiled/cljs/core.js:33594:20 at http://localhost:3449/js/compiled/kvlt/middleware.js:222:42 at cljs.core.MultiFn.cljs$core$IFn$

_invoke$arity$1 (http://localhost:3449/js/compiled/cljs/core.js:33693:106 http://localhost:3449/js/compiled/cljs/core.js:33693:106) at cljs.core.comp.cljs$core$IFn$_invoke$arity$2.G91121 (http://localhost:3449/js/compiled/cljs/core.js:14017:52 http://localhost:3449/js/compiled/cljs/core.js:14017:52) at cljs.core.comp.cljs$core$IFn$_invoke$arity$2.G9112 (http://localhost:3449/js/compiled/cljs/core.js:14061:19 http://localhost:3449/js/compiled/cljs/core.js:14061:19) at http://localhost:3449/js/compiled/cats/core.js:213:14 http://localhost:3449/js/compiled/cats/core.js:213:14 at http://localhost:3449/js/compiled/promesa/core.js:474:11 http://localhost:3449/js/compiled/promesa/core.js:474:11 From previous event: at Promise.promesa.core.Promise.promesa$core$IPromise$_bind$arity$2 (http://localhost:3449/js/compiled/promesa/core.js:472:15 http://localhost:3449/js/compiled/promesa/core.js:472:15) at promesa$core$_bind (http://localhost:3449/js/compiled/promesa/core.js:284:10 http://localhost:3449/js/compiled/promesa/core.js:284:10) at promesa.monad.promise_context.promesa.monad.t_promesa$monad42506.cats$protocols$Monad$_mbind$arity$3 (http://localhost:3449/js/compiled/promesa/monad.js:88:27 http://localhost:3449/js/compiled/promesa/monad.js:88:27) at cats$protocols$_mbind (http://localhost:3449/js/compiled/cats/protocols.js:405:10 http://localhost:3449/js/compiled/cats/protocols.js:405:10) at cats$core$bind (http://localhost:3449/js/compiled/cats/core.js:202:30 http://localhost:3449/js/compiled/cats/core.js:202:30) at http://localhost:3449/js/compiled/cljs/core.js:7624:96 http://localhost:3449/js/compiled/cljs/core.js:7624:96 at Function.cljs.core.seq_reduce.cljs$core$IFn$_invoke$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:7625:3 http://localhost:3449/js/compiled/cljs/core.js:7625:3) at cljs.core.Cons.cljs$core$IReduce$_reduce$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:10490:29 http://localhost:3449/js/compiled/cljs/core.js:10490:29) at Function.cljs.core.reduce.cljs$core$IFn$_invoke$arity$3 (http://localhost:3449/js/compiled/cljs/core.js:7718:13 http://localhost:3449/js/compiled/cljs/core.js:7718:13) at cljs$core$reduce (http://localhost:3449/js/compiled/cljs/core.js:7686:25 http://localhost:3449/js/compiled/cljs/core.js:7686:25) at Function.cats.core.GTGTEQ.cljs$core$IFn$ _invoke$arity$variadic (http://localhost:3449/js/compiled/cats/core.js:1246:25 http://localhost:3449/js/compiled/cats/core.js:1246:25) at cats$core$_GTGT_EQ ( http://localhost:3449/js/compiled/cats/core.js:1236:31) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:31) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53) at kvlt.middleware.util.__GT_mw.cljs$core$IFn$_invoke$arity$variadic ( http://localhost:3449/js/compiled/kvlt/middleware/util.js:159:53)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nervous-systems/kvlt/issues/6, or mute the thread https://github.com/notifications/unsubscribe/ABYuKxGH6SQWdcwHQsFKzYKxX8xL6_cOks5qGsmngaJpZM4Ipt8j .

moea commented 8 years ago

Thinking about this a bit more: what do you think is sensible behaviour here? I've certainly issued 404 responses with a body containing valid JSON - though I've also set the content-type, so :as :auto could rather be used. Options would be:

ingesolvoll commented 8 years ago

Tough question!

For my app (most apps probably), I assume the errors most likely to happen would be server down (404) or internal error 50x. At least for the server down case, a valid JSON response is unlikely :).

I see your point about error responses containing valid JSON, so you probably shouldn't block for that.

In my case, the last 2 options solves my problem, in general I would go for not overwriting existing error info.

ingesolvoll commented 8 years ago

To elaborate, my case for error handling right now is exceptional "hostile" errors like network lost etc. REST errors like resource not found, missing/malformed query parameters etc would probably benefit from having response interpreted as JSON?

moea commented 8 years ago

Let me know if you have any luck with 0.1.2-SNAPSHOT. If there's no underlying error, the request will fail with :middleware-error. If there's some other error condition (404, say), you ought to receive the error as before.

ingesolvoll commented 8 years ago

Seems to work fine!