Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid input ConnectionInputs.RECV_HEADERS in state ConnectionState.CLOSED #46

Closed
victoraugustolls opened this issue Mar 11, 2020 · 7 comments
Labels

Comments

@victoraugustolls
Copy link
Contributor

This seems to be an error with h2, just raising here to confirm it, as before using httpx with httpcore interface, I never saw this issue, at least not with this frequency.

Error: Invalid input ConnectionInputs.RECV_HEADERS in state ConnectionState.CLOSED

@tomchristie
Copy link
Member

Any idea how to reproduce this?

@victoraugustolls
Copy link
Contributor Author

I had the traceback, but not anymore. Will try to get a new one.

@victoraugustolls
Copy link
Contributor Author

@tomchristie not seeing this error in a long time now, I think we can close this!

@mm-matthias
Copy link

@tomchristie @victoraugustolls Can you please reopen this issue?

I encountered the problems with the latest httpcore version from master (ad7a7e3) and also with older versions.

This is what provokes the error:

  • Start many parallel requests to a server, using

    AsyncClient(
            limits=Limits(
                max_connections=1,
                max_keepalive_connections=1,
                keepalive_expiry=60,
            ),
    )

    makes it easier to trigger the problem.

  • This causes e.g. many SEND_HEADERS with new stream IDs, e.g. up to stream ID 2059

  • At one point the server will send a GOAWAY frame. In my case with nginx this always happens after exactly 2000 streams have been created (always last_stream_id=1999). C.f. this post.

  • After the server sent the GOAWAY frame it still finishes the processing of any of the outstanding streams up to stream 1999, e.g. we will get a RECV_HEADERS for stream 1997 after we received the GOAWAY. This is valid behaviour according to https://datatracker.ietf.org/doc/html/rfc7540#section-6.8:

    GOAWAY allows an endpoint to gracefully stop accepting new streams while still finishing processing of previously established streams.

  • The behaviour of h2 seems to be to set ConnectionState.CLOSED immediately after it received the GOAWAY.

  • Thus the error Invalid input ConnectionInputs.RECV_HEADERS in state ConnectionState.CLOSED is incorrectly triggered.

  • There are additional problems: h2 already sent data for streams with IDs 2000-2059. According to my understanding of the RFC these streams will not be processed by the server.

    Once sent, the sender will ignore frames sent on streams initiated by the receiver if the stream has an identifier higher than the included last stream identifier.

    If the receiver of the GOAWAY has sent data on streams with a higher stream identifier than what is indicated in the GOAWAY frame, those streams are not or will not be processed. The receiver of the GOAWAY frame can treat the streams as though they had never been created at all, thereby allowing those streams to be retried later on a new connection.

  • There is much more wording in the RFC regarding the treatment of GOAWAY after the excerpts that I quoted.

What's the stance h2 takes here? Is the goal to implement the behavior asked for the RFC or is this too much work? If it cannot be implemented for now, maybe it's a good idea to make the error message clearer, e.g. more like "Not implemented error, refer to XYZ for more information".

@zanieb
Copy link
Contributor

zanieb commented May 12, 2023

See the fixes in #679 and #683 as well as discussion at encode/httpx#2112

There's an h2 issue at python-hyper/h2#1181 that tracks implementing in accordance with the RFC but the issue doesn't seem to be going anywhere and nobody responded to my request for guidance.

@mm-matthias
Copy link

@madkinsz Thanks for the updates and links, python-hyper/h2#1181 probably hits closest for me. As you said, this seems to be a lot of work and I also cannot provide any real guidance here as I'm new myself and barely have time.
Guess I gotta live with HTTP/1.1 for now :(

@zanieb
Copy link
Contributor

zanieb commented May 13, 2023

I think following #683 you'll get pretty good behavior with HTTP/2, httpcore will just retry the requests when a GOAWAY is received — it sounds like many clients do not properly implement the GOAWAY behavior described in the RFC anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants