pekko-http icon indicating copy to clipboard operation
pekko-http copied to clipboard

HTTP2: Allow frame of type RstStreamFrame when in state HalfClosedLocalWaitingForPeerStream

Open jtjeferreira opened this issue 1 year ago • 3 comments

As described in https://github.com/apache/pekko-connectors/pull/830, I got an "Received unexpected frame of type RstStreamFrame for stream 1 in state HalfClosedLocalWaitingForPeerStream" which I think is a bug in the http2 client side support.

In this PR I took the approach of returning an HttpResponse with status 429, and with an failed source with PeerClosedStreamException as entity. I though of several alternatives but I could not implement them:

  • transparently retry the request when the error code is REFUSED_STREAM, since the spec clearly says the request can be retried.
  • return/throw PeerClosedStreamException, without having to "wrap" it in a HttpResponse

jtjeferreira avatar Oct 01 '24 10:10 jtjeferreira

Thanks for looking into this!

transparently retry the request when the error code is REFUSED_STREAM, since the spec clearly says the request can be retried.

That would indeed be nice but could get pretty complex, seems reasonable to postpone that.

return/throw PeerClosedStreamException, without having to "wrap" it in a HttpResponse

Hmm, not having to create a 'fake' HttpResponse would indeed be nice, I wonder if we can avoid that somehow

raboof avatar Oct 02 '24 11:10 raboof

return/throw PeerClosedStreamException, without having to "wrap" it in a HttpResponse

Hmm, not having to create a 'fake' HttpResponse would indeed be nice, I wonder if we can avoid that somehow

Indeed! In the API it's not even easy to support failed requests because the HTTP/2 API is ultimately flow based, so we would have to change the flow type to support failed requests that somehow support the request association.

Maybe it's indeed better to create a well-document fake response that works with the existing API types (i.e. HttpResponse).

In some way, the server misses the opportunity to offer a better response code by rejecting a request with a RST frame instead of returning a (potentially canned) response giving more information about the reason.

jrudolph avatar Oct 02 '24 12:10 jrudolph

Indeed! In the API it's not even easy to support failed requests because the HTTP/2 API is ultimately flow based, so we would have to change the flow type to support failed requests that somehow support the request association.

I guess that would mean changing the HTTP/2 flow API to be similar to the HTTP/1 flow api: Flow[(HttpRequest, T), (Try[HttpResponse], T), HostConnectionPool], that uses Try[HttpResponse] ? But again thats a bigger change for another PR...

Meanwhile feel free to review the PR

jtjeferreira avatar Oct 02 '24 14:10 jtjeferreira