swift-server / swift-server/async-http-client

HTTP2 go away at the protocol level lets the request hang

Open
#660 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

kind/bug
Dominant language
Swift
Stars
1.1k
Forks
156
PR merge metrics
No merged PRs in 30d

Description

We currently ignore the error code of the go away. I think if it is a protocol error we will never receive any further HTTP2Frame on the connection or any stream and need manually cancel all active streams. Maybe this is better handled at the nio http2 level but I'm not sure yet.

Reproducer:

    func testGoAwayFromServerDoesntHangRequest() throws {
        //try XCTSkipIf(true, "this currently hangs")
        let bin = HTTPBin(.http2())
        defer { XCTAssertNoThrow(try bin.shutdown()) }
        
        let loggerFactor = StreamLogHandler.standardOutput(label:)
        var bgLogger = Logger(label: "BG", factory: loggerFactor)
        bgLogger.logLevel = .trace
        
        let client = HTTPClient(
            eventLoopGroupProvider: .createNew,
            configuration: .init(certificateVerification: .none),
            backgroundActivityLogger: bgLogger
        )
        
        defer { XCTAssertNoThrow(try client.syncShutdown()) }
        
        var request = try HTTPClient.Request(url: bin.baseURL, method: .POST)
        // add ~64 KB header
        let headerValue = String(repeating: "0", count: 1024)
        for headerID in 0..<64 {
            request.headers.replaceOrAdd(name: "larg-header-\(headerID)", value: headerValue)
        }

        // non empty body is important to trigger this bug as we otherwise finish the request in a single flush
        request.body = .byteBuffer(ByteBuffer(bytes: [0]))

        var rqLogger = Logger(label: "RQ", factory: loggerFactor)
        rqLogger.logLevel = .trace
        
        XCTAssertThrows(try client.execute(request: request, deadline: .distantFuture, logger: rqLogger).wait())
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2IdleHandler.swift, especially the handling of the HTTP/2 GOAWAY error code. Run the reproducer testGoAwayFromServerDoesntHangRequest, which uses a non-empty request body and a distant-future deadline. Done means the request no longer hangs and the test observes the expected thrown error.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.