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

"java.net.SocketException: Broken pipe" when server sends response before request is fully sent #277

Open wyegelwel opened 9 years ago

wyegelwel commented 9 years ago

I'm writing an application in which the server can quickly rule out a request based on it's headers. When this happens the server will send back a 4xx response. If the client is sending a large body in a POST, it will cause a "java.net.ScoketException: Broken pipe" on the client.

The issue appears to be that client expects to fully send it's request before accepting a response.

Here is a simple test that exhibits the problem:

(ns http-test.core
  (:require [clj-http.client :as client]
            [org.httpkit.server :refer [run-server]]))

;; Server 

(defn app  [req]
  {:status  200
   :headers  {"Content-Type" "text/html"}
   :body    "hello HTTP!"})

(run-server app  {:port 8080})

;; Client 

(def s (slurp "big-file.txt"))

(client/post "http://localhost:9091/run" {:body s})
(defproject http-test "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.6.0"]
                 [clj-http "2.0.0"]
                 [http-kit "2.1.18"]
                 ])

big-file.txt is 12 mb. A 112kb file doesn't cause cause the problem.

dakrone commented 9 years ago

wyegelwel writes:

I'm writing an application in which the server can quickly rule out a request based on it's headers. When this happens the server will send back a 4xx response. If the client is sending a large body in a POST, it will cause a "java.net.ScoketException: Broken pipe" on the client.

Do you have the entire stacktrace for this? I'd like to see whether this is an Apache problem, or something clj-http is doing.

wyegelwel commented 9 years ago
Caused by: java.net.SocketException: Broken pipe
        at java.net.SocketOutputStream.socketWrite0(Native Method)
        at java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:113)
        at java.net.SocketOutputStream.write(SocketOutputStream.java:159)
        at org.apache.http.impl.io.AbstractSessionOutputBuffer.write(AbstractSessionOutputBuffer.java:181)
        at org.apache.http.impl.io.ContentLengthOutputStream.write(ContentLengthOutputStream.java:115)
        at org.apache.http.impl.io.ContentLengthOutputStream.write(ContentLengthOutputStream.java:122)
        at org.apache.http.entity.StringEntity.writeTo(StringEntity.java:169)
        at org.apache.http.entity.HttpEntityWrapper.writeTo(HttpEntityWrapper.java:96)
        at org.apache.http.impl.client.EntityEnclosingRequestWrapper$EntityWrapper.writeTo(EntityEnclosingRequestWrapper.java:112)
        at org.apache.http.impl.entity.EntitySerializer.serialize(EntitySerializer.java:117)
        at org.apache.http.impl.AbstractHttpClientConnection.sendRequestEntity(AbstractHttpClientConnection.java:265)
        at org.apache.http.impl.conn.ManagedClientConnectionImpl.sendRequestEntity(ManagedClientConnectionImpl.java:203)
        at org.apache.http.protocol.HttpRequestExecutor.doSendRequest(HttpRequestExecutor.java:237)
        at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:122)
        at org.apache.http.impl.client.DefaultRequestDirector.tryExecute(DefaultRequestDirector.java:685)
        at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:487)
        at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:882)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:82)
        at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:107)
        at clj_http.core$request.invoke(core.clj:298)
        at clojure.lang.Var.invoke(Var.java:379)
        at clj_http.client$wrap_request_timing$fn__10067.invoke(client.clj:796)
        at clj_http.headers$wrap_header_map$fn__9361.invoke(headers.clj:143)
        at clj_http.client$wrap_query_params$fn__9970.invoke(client.clj:624)
        at clj_http.client$wrap_basic_auth$fn__9977.invoke(client.clj:640)
        at clj_http.client$wrap_oauth$fn__9981.invoke(client.clj:650)
        at clj_http.client$wrap_user_info$fn__9986.invoke(client.clj:663)
        at clj_http.client$wrap_url$fn__10053.invoke(client.clj:762)
        at clj_http.client$wrap_redirects$fn__9754.invoke(client.clj:237)
        at clj_http.client$wrap_decompression$fn__9779.invoke(client.clj:309)
        at clj_http.client$wrap_input_coercion$fn__9909.invoke(client.clj:463)
        at clj_http.client$wrap_additional_header_parsing$fn__9930.invoke(client.clj:522)
        at clj_http.client$wrap_output_coercion$fn__9900.invoke(client.clj:438)
        at clj_http.client$wrap_exceptions$fn__9740.invoke(client.clj:189)
        at clj_http.client$wrap_accept$fn__9944.invoke(client.clj:565)
        at clj_http.client$wrap_accept_encoding$fn__9950.invoke(client.clj:579)
        at clj_http.client$wrap_content_type$fn__9939.invoke(client.clj:555)
        at clj_http.client$wrap_form_params$fn__10031.invoke(client.clj:726)
        at clj_http.client$wrap_nested_params$fn__10048.invoke(client.clj:751)
        at clj_http.client$wrap_method$fn__9991.invoke(client.clj:670)
        at clj_http.cookies$wrap_cookies$fn__8314.invoke(cookies.clj:124)
        at clj_http.links$wrap_links$fn__9559.invoke(links.clj:51)
        at clj_http.client$wrap_unknown_host$fn__10057.invoke(client.clj:771)
        at clj_http.client$post.doInvoke(client.clj:886)
        at clojure.lang.RestFn.invoke(RestFn.java:423)
        at http_test.core$eval10315.invoke(core.clj:18)
        at clojure.lang.Compiler.eval(Compiler.java:6703)
        at clojure.lang.Compiler.load(Compiler.java:7130)
dakrone commented 9 years ago

Okay, thanks for the reproduction, I'll take a look!

wyegelwel commented 9 years ago

Thank you!

mzhang-code commented 7 years ago

hi @wyegelwel . I found it's due to the setting :max-body of httpkit.server implementation. The default is 8MB.

When you do (run-server app {:port 23456 :max-body (* 100 1024 1024)}), It'd be good.

Best!

dakrone commented 7 years ago

Closing this, it sounds like this is a problem on the server side cutting a transmission short.

bladealslayer commented 7 years ago

@dakrone this is not a problem with the server. A server may legitimately return error response early. I.e. precisely when enforcing a limit on upload size. The client should monitor what the server is sending and terminate transmission if it receives an error. (This is, btw, also in RFC 2616, section 8.2.2.)

dakrone commented 7 years ago

Hmm... I'm not sure if there's something that could be done to be better about handling the broken pipe, but we can re-open this to look at trying to handle the exception in a better/clearer way.

dakrone commented 6 years ago

Revisiting this, I'm not sure clj-http should attempt to do anything in this case, I think it's a legitimate exception that is thrown and should be caught by the user of clj-http.

I'm going to close this (again) for now, but if someone has a reason why clj-http should do something in this case, let me know and I'm happy to revisit this.

bladealslayer commented 6 years ago

I think ideally, clj-http would not throw, but return the server's response (which would probably have status code 413 or something similar).

dakrone commented 6 years ago

Okay, I'm reopening this to look into it more. For reference, here's the RFC mentioning this:

   An HTTP/1.1 (or later) client sending a message-body SHOULD monitor
   the network connection for an error status while it is transmitting
   the request. If the client sees an error status, it SHOULD
   immediately cease transmitting the body. If the body is being sent
   using a "chunked" encoding (section 3.6), a zero length chunk and
   empty trailer MAY be used to prematurely mark the end of the message.
   If the body was preceded by a Content-Length header, the client MUST
   close the connection.

The difficult thing here (and not disagreeing with you necessarily about the behavior) is that the server sends the error status (okay so far), so the client stops sending data, but on the connection side, it appears that it's http-kit that is actually closing the connection, not clj-http, in which case clj-http is responding to the socket being closed. I don't see in the RFC what the server should do, should it read and discard what has already been sent and then nicely close the socket?

dakrone commented 6 years ago

Also, I was able to reproduce this on a server other than http-kit, so I'm glad it's not dependent on that.

dakrone commented 6 years ago

@bladealslayer after some more research, I found a couple of things:

It's known that Apache does this, it's unfortunately a lose-lose situation:

I also read that Apache's async client does not have this issue, so I tried it and got:

(post "http://localhost:9090" {:body (String. (byte-array (* 10 1024 1024))) :async true} (fn [resp] (println :got resp)) (fn [err] (println :fail err)))
#object[org.apache.http.impl.nio.client.FutureWrapper 0xe6ee480 "org.apache.http.concurrent.BasicFuture@55c949cf"]
:fail #error {
 :cause clj-http: status 413
 :data {:request-time 33, :repeatable? false, :protocol-version {:name HTTP, :major 1, :minor 1}, :streaming? true, :http-client #object[org.apache.http.impl.nio.client.InternalHttpAsyncClient 0x10913785 org.apache.http.impl.nio.client.InternalHttpAsyncClient@10913785], :chunked? false, :type :clj-http.client/unexceptional-status, :reason-phrase Request Entity Too Large, :headers {Content-Length 47, Server http-kit, Date Wed, 21 Mar 2018 04:06:52 GMT}, :orig-content-encoding nil, :status 413, :length 47, :body request body 10485760; max request body 8388608, :trace-redirects []}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message clj-http: status 413
   :data {:request-time 33, :repeatable? false, :protocol-version {:name HTTP, :major 1, :minor 1}, :streaming? true, :http-client #object[org.apache.http.impl.nio.client.InternalHttpAsyncClient 0x10913785 org.apache.http.impl.nio.client.InternalHttpAsyncClient@10913785], :chunked? false, :type :clj-http.client/unexceptional-status
, :reason-phrase Request Entity Too Large, :headers {Content-Length 47, Server http-kit, Date Wed, 21 Mar 2018 04:06:52 GMT}, :orig-content-encoding nil, :status 413, :length 47, :body request body 10485760; max request body 8388608, :trace-redirects []}
   :at [clojure.core$ex_info invokeStatic core.clj 4739]}]
 :trace
 [[clojure.core$ex_info invokeStatic core.clj 4739]
  [clojure.core$ex_info invoke core.clj 4739]
  [clj_http.client$exceptions_response invokeStatic client.clj 241]
  [clj_http.client$exceptions_response invoke client.clj 232]
  [clj_http.client$wrap_exceptions$fn__9231$fn__9232 invoke client.clj 254]
  [clj_http.client$wrap_output_coercion$fn__9401$fn__9402 invoke client.clj 472]
  [clj_http.client$wrap_additional_header_parsing$fn__9439$fn__9440 invoke client.clj 583]
  [clj_http.client$wrap_decompression$fn__9261$fn__9262 invoke client.clj 312]
  [clj_http.client$wrap_request_timing$fn__9610$fn__9611 invoke client.clj 976]
  [clj_http.core$request$reify__6992 completed core.clj 529]
  [org.apache.http.concurrent.BasicFuture
 completed BasicFuture.java 123]
  [org.apache.http.impl.nio.client.DefaultClientExchangeHandlerImpl responseCompleted DefaultClientExchangeHandlerImpl.java 181]
  [org.apache.http.nio.protocol.HttpAsyncRequestExecutor processResponse HttpAsyncRequestExecutor.java 437]
  [org.apache.http.nio.protocol.HttpAsyncRequestExecutor inputReady HttpAsyncRequestExecutor.java 327]
  [org.apache.http.impl.nio.DefaultNHttpClientConnection consumeInput DefaultNHttpClientConnection.java 265]
  [org.apache.http.impl.nio.client.InternalIODispatch onInputReady InternalIODispatch.java 81]
  [org.apache.http.impl.nio.client.InternalIODispatch onInputReady InternalIODispatch.java 39]
  [org.apache.http.impl.nio.reactor.AbstractIODispatch inputReady AbstractIODispatch.java 114]
  [org.apache.http.impl.nio.reactor.BaseIOReactor readable BaseIOReactor.java 162]
  [org.apache.http.impl.nio.reactor.AbstractIOReactor processEvent AbstractIOReactor.java 337]
  [org.apache.http.impl.nio.reactor.AbstractIOReactor processEvents AbstractIOReactor.java
 315]
  [org.apache.http.impl.nio.reactor.AbstractIOReactor execute AbstractIOReactor.java 276]
  [org.apache.http.impl.nio.reactor.BaseIOReactor execute BaseIOReactor.java 104]
  [org.apache.http.impl.nio.reactor.AbstractMultiworkerIOReactor$Worker run AbstractMultiworkerIOReactor.java 588]
  [java.lang.Thread run Thread.java 844]]}

Which seems to be exactly what you expect, the failure handler is invoked with the 413 response rather than getting a socket exception.

At this point, I think this is an Apache issue and not something clj-http can really work around. What do you think?

I'll leave this open as a reminder to retry this with the 5.0 http client when it's finally released, to see if anything has changed. Hopefully in the meantime using the async client will work for you.

[0]: almost all, though there might be changes for this in the future