dakrone / clj-http

An idiomatic clojure http client wrapping the apache client. Officially supported version.
http://clojars.org/clj-http
MIT License
1.78k stars 408 forks source link

Body is lost if response middleware encounters an error #430

Open quoll opened 6 years ago

quoll commented 6 years ago

If an exception occurs while a coercion function is processing a response, then any code that attempts to catch the exception is unable to see the body, since this has been drained out of the InputStream and into a byte array, which then goes out of scope.

Every coercion function starts by getting the body via util/force-byte-array. Based on this, I'm wondering if it would be acceptable to update the response object before processing with something like: (update resp :body util/force-byte-array)

I'm cautious about this suggestion. The fact that every function starts with its own call to util/force-byte-array, instead of doing it in a central location, makes me think that it's been left intentionally open for content-types where this is not desired. In particular, I'm left wondering if there is intent to support streaming parsers (such as StAX). If that were the case, then preemptively drawing the body in as a byte array would not work. However, for the current cases, it would work just fine.

Is there any design philosophy for cli-http which specifically considers this? Thanks.

dakrone commented 6 years ago

@quoll I think what I mentioned in #429 would be the best path forward, let me know what your thoughts are

wmatson commented 5 years ago

@dakrone I think the boolean for parse only on success (instead of always attempting parsing) would be a reasonable path forward. I find that I often lose important information on 4xx and 5xx responses.

Another possible option would be to attempt parsing only on matching content-type headers.