Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EOF exceptions on empty responses w/ :as :clojure [1.1.0] #257

Closed
hlship opened this issue Mar 31, 2015 · 10 comments
Closed

EOF exceptions on empty responses w/ :as :clojure [1.1.0] #257

hlship opened this issue Mar 31, 2015 · 10 comments
Labels

Comments

@hlship
Copy link

@hlship hlship commented Mar 31, 2015

I have hit an issue where, when I supply the :as :clojure option, clj-http will always attempt to parse the response, even when blank.

This is not ideal for me, and is generally problematic for any HTTP request that may return a body (to be parsed as JSON or EDN, etc.) or may respond with a status code and empty body.

This has only just come up because of ring-middleware-format:0.5.0, which now allows responses that are truly empty (in the past, a nil body would be converted to the string "nil" or "null").

Here's the exception I'm hitting:

                      users-spec/eval21796/fn/fn/fn    users_spec.clj:  178
                       users-spec/eval21796/fn/show    users_spec.clj:   59
                                                ...                        
                                clj-http.client/get        client.clj:  739
               clj-http.client/wrap-unknown-host/fn        client.clj:  636
                       clj-http.links/wrap-links/fn         links.clj:   50
                   clj-http.cookies/wrap-cookies/fn       cookies.clj:  121
                     clj-http.client/wrap-method/fn        client.clj:  576
              clj-http.client/wrap-nested-params/fn        client.clj:  617
                clj-http.client/wrap-form-params/fn        client.clj:  593
               clj-http.client/wrap-content-type/fn        client.clj:  477
            clj-http.client/wrap-accept-encoding/fn        client.clj:  502
                     clj-http.client/wrap-accept/fn        client.clj:  487
                 clj-http.client/wrap-exceptions/fn        client.clj:  149
            clj-http.client/wrap-output-coercion/fn        client.clj:  366
                                                ...                        
                       clj-http.client/eval15959/fn        client.clj:  348
                clj-http.client/coerce-clojure-body        client.clj:  310
                                                ...                        
                          clj-http.client/parse-edn        client.clj:   46
                                 clojure.core/apply          core.clj:  628
                                                ...                        
               clojure.tools.reader.edn/read-string           edn.clj:  406
                      clojure.tools.reader.edn/read           edn.clj:  357
                      clojure.tools.reader.edn/read           edn.clj:  364
                                                ...                        
     clojure.tools.reader.reader-types/reader-error  reader_types.clj:  337
                               clojure.core/ex-info          core.clj: 4577
     clojure.lang.ExceptionInfo: EOF
         data: {:type :reader-exception}

My suggestion is that the :as functionality return nil when the body is empty.

I'm attempting a workaround where I do not use :as, but do supply a custom middleware to do the same thing ... with the "empty" check.

@hlship

This comment has been minimized.

Copy link
Author

@hlship hlship commented Mar 31, 2015

Hm. This code makes it look like it's doing the right thing:

(defn wrap-output-coercion
  "Middleware converting a response body from a byte-array to a different
  object. Defaults to a String if no :as key is specified, the
  `coerce-response-body` multimethod may be extended to add
  additional coercions."
  [client]
  (fn [req]
    (let [{:keys [body] :as resp} (client req)]
      (if body
        (coerce-response-body req resp)
        resp))))

Let me check to see exactly what's in my response that's causing this kind of problem.

@hlship

This comment has been minimized.

Copy link
Author

@hlship hlship commented Mar 31, 2015

The issue is that even with an empty body, there's still an non-nil :body, a FilterStream.

clj-http_client_-fan-suite-___workspaces_fexco_fan-suite

so the conversion code is called, even when the FilterStream is empty.

At the very least, a quick check of the content-length header would be an improvement.

After that it gets more complicated. If mark is supported, you can attempt to read a single byte, then reset.

Ideally, lower layers of clj-http would ensure that :body was nil if the actual response is empty.

@hlship

This comment has been minimized.

Copy link
Author

@hlship hlship commented Mar 31, 2015

filterinputstream_java_-fan-suite-___workspaces_fexco_fan-suite

Looks like the lower levels do know that there is no input stream, no real body.

@hlship hlship changed the title EOF exceptions on empty responses w/ :as :clojure EOF exceptions on empty responses w/ :as :clojure [0.9.2] Mar 31, 2015
@hlship hlship changed the title EOF exceptions on empty responses w/ :as :clojure [0.9.2] EOF exceptions on empty responses w/ :as :clojure [1.1.0] Mar 31, 2015
@hlship

This comment has been minimized.

Copy link
Author

@hlship hlship commented Mar 31, 2015

Verified this occurs in 1.1.0.

@dakrone

This comment has been minimized.

Copy link
Owner

@dakrone dakrone commented Mar 31, 2015

Yes, I think this is a byproduct of the switch from a String body to a stream, in the past it the body could be nil as you said, but now it is always a stream (even if an empty one).

I will work on fixing this.

@hlship

This comment has been minimized.

Copy link
Author

@hlship hlship commented Mar 31, 2015

Ok, thanks. I think I can do a workaround by defining a custom coerce-response-body method that eats the EOF exception.

dakrone added a commit that referenced this issue Apr 3, 2015
Currently commented out (since it fails)
@bilus

This comment has been minimized.

Copy link

@bilus bilus commented Jul 20, 2015

For anyone who may come across this problem, the quick & dirty workaround below may save a few minutes:

(ns my-ns
  (:require [clj-http.client :as http]))

(defmethod http/coerce-response-body :clojure [req resp]
  (if (> (Integer/parseInt (or (get-in resp [:headers "Content-Length"]) "0")) 0)
    (http/coerce-clojure-body req resp)
    (assoc resp :body nil)))

It is specific to :clojure but should be pretty easy to adapt to any format. Not the best example of sound engineering practices but I expect it to work till the problem is fixed.

@kamituel

This comment has been minimized.

Copy link

@kamituel kamituel commented Dec 9, 2015

I just encountered that issue too. For now, @bilus's workaround seems to be working (thanks).

CodingAgainstChaos added a commit to CodingAgainstChaos/clj-http that referenced this issue Apr 16, 2016
This commit checks if the body is empty during clojure coercion.
If it's empty, the body is set to nil. Previously, the test assumed
the body would return as an empty string but the expected behavior
per the convo in the issue is that nil should be returned.
CodingAgainstChaos added a commit to CodingAgainstChaos/clj-http that referenced this issue Apr 16, 2016
This commit checks if the body is empty during clojure coercion.
If it's empty, the body is set to nil. Previously, the test assumed
the body would return as an empty string but the expected behavior
per the convo in the issue is that nil should be returned.
CodingAgainstChaos added a commit to CodingAgainstChaos/clj-http that referenced this issue Apr 16, 2016
This commit checks if the body is empty during clojure coercion.
If it's empty, the body is set to nil. Previously, the test assumed
the body would return as an empty string but the expected behavior
per the convo in the issue is that nil should be returned.
CodingAgainstChaos added a commit to CodingAgainstChaos/clj-http that referenced this issue Apr 16, 2016
This commit checks if the body is empty during clojure coercion.
If it's empty, the body is set to nil. Previously, the test assumed
the body would return as an empty string but the expected behavior
per the convo in the issue is that nil should be returned.
dakrone added a commit that referenced this issue Apr 18, 2016
@smee

This comment has been minimized.

Copy link

@smee smee commented Jul 8, 2016

The same problem occurs if a server sends an empty body with "content-encoding: gzip".

(defmethod decompress-body "gzip"
tries to unzip the empty body, which fails. According to rabbitmq/rabbitmq-management#154 (comment) this behaviour looks like a client error.

@dakrone dakrone closed this in 6574e0a Feb 28, 2018
dakrone added a commit that referenced this issue Feb 28, 2018
When a request returns a response that is empty in addition to the
"content-encoding: gzip" header, clj-http would try to decode the response,
causing an EOFException.

This resolves this by attempting to read a single byte, and returning an empty
byte array if there is no body. This also fixes the case where the body was
attempted to be coerced and failed due to the `EOFException`

Resolves #257
@dakrone

This comment has been minimized.

Copy link
Owner

@dakrone dakrone commented Feb 28, 2018

I finally pushed a commit for this, thanks for your patience!

@dakrone dakrone added bug 3.x 4.x labels Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.