HTTP2: Allow frame of type RstStreamFrame when in state HalfClosedLocalWaitingForPeerStream

[jtjeferreira]

As described in apache/pekko-connectors#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

[raboof]

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

[jrudolph]

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.

[jtjeferreira]

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]

should I handle rst: RstStreamFrame is other states? should I change other places that handle RstStreamFrame to return the fake response?