Add Monitoring Connections for Error Status Messages #6283
Conversation
As per https://tools.ietf.org/html/rfc2616#section-8.2.2 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. Currently, okhttp implements a send request, then read response if/when request was sent successfully. This hides early responses such as 413 Payload Too Large errors from clients. This commit fixes that by adding a response read in case of an exception happening while the request is being sent. This closes square#1001 .
| import java.util.concurrent.TimeUnit; | ||
| import javax.annotation.Nullable; | ||
| import javax.net.ServerSocketFactory; | ||
| import javax.net.SocketFactory; | ||
|
|
||
| import com.sun.net.httpserver.HttpExchange; |
|
....in particular, we need tests to confirm the connection is properly released and not re-used. Also, we use MockWebServer in this project! |
|
Hi @swankjesse , Thank you for the quick response! Can you please elaborate on
? Is there something fundamentally off with it or is it more of a "let's use MockWebServer and please add 1-2 more test cases"? |
|
I think the fix needs to move from |
|
@swankjesse Devil's advocate, since it doesn't introduce new API, just improves behaviour in a readable way for HTTP/1.1 should we adopt this short term until a proper full fix covering HTTP/2 also? |
|
I've run into an HTTP/2 variant of this problem and I'm going to attempt to fix that there. The thing I'm least comfortable with is lifecycle of the connection after we recover. It's broken for writing but not reading, and that isn't something we've modeled. |
|
Thanks for driving the work on this. I’ve created a PR that includes some ideas from this. Please review! |
As per https://tools.ietf.org/html/rfc2616#section-8.2.2 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.
Currently, okhttp implements a send request, then read response
if/when request was sent successfully.
This hides early responses such as 413 Payload Too Large
errors from clients.
This commit fixes that by adding a response read in case of an
exception happening while the request is being sent.
This closes #1001 .
Related PR: #6121