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

Network errors on HTTP/2 should propagate across all streams. #440

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

tomchristie
Copy link
Member

Network errors on HTTP/2 need to be handled a bit differently to HTTP/1.1, because there will often be multiple streams being handled on the same connection. Rather than simply raising the exception we should...

  • Save the exception.
  • Mark the connection as errored, and return False for any future is_available calls to disallow further requests being queued on the connection.
  • Immediately re-raise the exception on any other network calls, so that it applies across all streams.

This closes encode/httpx#1414 - where the observed behaviour was a disconnect, followed by TooManyStreams exceptions. Instead of continuing to queue up further requests behind the one request that resulted in a disconnect, we instead want to see the disconnect apply to all existing requests on that connection, as well as preventing any future requests from being queued up on it.

Testing this out...

import anyio
import httpcore
import time


async def download(http, year):
    url = f"https://en.wikipedia.org/wiki/{year}"
    extensions = {"timeout": {"read": 5.0}}
    try:
        response = await http.request("GET", url, extensions=extensions)
    except Exception as exc:
        print(year, type(exc))
    else:
        print(year, response)


async def main():
    async with httpcore.AsyncConnectionPool(http2=True) as http:
        async with anyio.create_task_group() as task_group:
            for year in range(1900, 2020):
                task_group.start_soon(download, http, year)


anyio.run(main)
  • Start a number of concurrent HTTP/2 requests, using the above, and allow it to start.
  • Quickly disconnect the Wi-Fi / network connection, before all requests have completed.
  • We ought to get graceful exception behaviour on the remaining requests. (No TooManyStreams exceptions. No gradually one-by-one trickling ReadTimeout exceptions.)

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

Successfully merging this pull request may close these issues.

Max outbound streams is 100, 100 open
1 participant