igrigorik / http-2

Pure Ruby implementation of HTTP/2 protocol
https://httpwg.github.io/specs/rfc7540.html
MIT License
894 stars 63 forks source link

How to deal with idempotent requests which fail? #128

Closed ioquatix closed 6 years ago

ioquatix commented 6 years ago

I'm implementing an h2 proxy.

I'm following the rough outline here: https://tools.ietf.org/html/rfc7230#section-6.3.1

What's interesting to me is how to handle persistent connections which have been closed.

With HTTP/1.x, if you try to write the request line, but it fails, you generally know that the persistent connection was closed - it's not an error, and it's safe to open a new connection and resend the request (even non-idempotent ones).

It would be interesting to know how to handle this with this gem.

At what point can we assume that the request has been received by the remote end?

I assume that after calling:

stream.headers(headers, end_stream: true)
io.flush

We have to assume at this point the remote end has received the request and we can no longer retry non-idempotent requests.

Thoughts?

igrigorik commented 6 years ago

With HTTP/1.x, if you try to write the request line, but it fails, you generally know that the persistent connection was closed

What does "fails" means in this scenario?

FWIW, H2 introduced GOAWAY to address this case: https://httpwg.org/specs/rfc7540.html#rfc.section.6.8

Endpoints SHOULD always send a GOAWAY frame before closing a connection so that the remote peer can know whether a stream has been partially processed or not. For example, if an HTTP client sends a POST at the same time that a server closes a connection, the client cannot know if the server started to process that POST request if the server does not send a GOAWAY frame to indicate what streams it might have acted on.

ioquatix commented 6 years ago

That makes sense.

I've been trying out goaway.

Sometimes the endpoint would crash or experience other kinds of failure.

Connection might be closed without goaway, so we still need to handle those cases.

igrigorik commented 6 years ago

If the other end closes the underling TCP connection properly you'll get a TCP RST, which can then signal the teardown up the stack. On the other hand, if the connection drops without a RST there isn't much you can do there short of listening for the TCP timeout.. Of course, none of this is h2 specific.

ioquatix commented 6 years ago

Yes that makes sense.

HTTP/2 is actually better in this regard because I have a single async task reading from the socket and the event dispatch happens from there. If the read failed, we get EOF, and we can gracefully close the connection.

HTTP/1 is a bit harder, since you don't know the connection has failed until you try to read from it and get EOF. In the end I added a second check https://github.com/socketry/async-http/blob/b1168cd95dd3ef9879e0e005a53bd4666a7d1e4b/lib/async/http/protocol/http11.rb#L58-L61 which invokes https://github.com/socketry/async-io/blob/ebca237cd3d14d2c08c693ec5ea4142298b5ec76/lib/async/io/socket.rb#L29-L37

The main question is: If the socket is disconnected, can we reconnect and send the request again?

If the request is idempotent, we can try again without worrying about the nature of the previous failure.

If the request is not idempotent, it's bit more tricky. If you think you wrote a complete request to the network, at that point you need to fail. However, if you actually didn't complete writing the full request, there is a reasonable expectation that the request was never fully received on the remote end and it should be okay to retry.

The logic for the client is here https://github.com/socketry/async-http/blob/b1168cd95dd3ef9879e0e005a53bd4666a7d1e4b/lib/async/http/client.rb#L67-L99

It has two failure paths - a general one which fails if the request was not idempotent, and a specific one which retries the request. That special path should okay be taken if the request actually failed in a way where it would have been impossible for the upstream server to execute it.

ioquatix commented 6 years ago

With h2, the only place where I could insert this is here:

https://github.com/socketry/async-http/blob/b1168cd95dd3ef9879e0e005a53bd4666a7d1e4b/lib/async/http/protocol/http2.rb#L256-L260

Fortunately, most post requests have bodies.

igrigorik commented 6 years ago

@ioquatix fyi, might be of interest: https://tools.ietf.org/html/draft-nottingham-httpbis-retry-00

ioquatix commented 6 years ago

Thanks that's really interesting.

I've got something that works pretty well right now. I may revisit for the future and that guide will surely be helpful.

ioquatix commented 6 years ago

You might appreciate https://news.ycombinator.com/item?id=16964907

igrigorik commented 6 years ago

^ that's a good one 😆