Body transfer timeout possibly not enforced correctly

[kelunik]

More details will follow.

[kelunik]

This seems to unsubscribe too early, because it doesn't await the response body. Same here.

[trowski]

I added some tests. The behavior seems to be as expected, since the body re-subscribes to the cancellation token.

I noticed something while writing the tests: We may want to add the ability to have a separate cancellation token for the body. For example, I may want to receive a response from the server within 5 seconds, but maybe I'm downloading a large file that will take much longer than that. Right now there isn't an easy way to do that.

[kelunik]

@trowski Thanks for the tests. I modified one of them slightly and it fails:

public function testTimeoutWhileStreamingBody(): \Generator
{
    $hpack = new HPack;

    [$server, $client] = Socket\createPair();

    $connection = new Http2Connection($client);

    $server->write(self::packFrame('', Http2Parser::SETTINGS, 0, 0));

    yield $connection->initialize();

    $request = new Request('http://localhost/');
    $request->setTransferTimeout(500);

    /** @var Stream $stream */
    $stream = yield $connection->getStream($request);

    asyncCall(static function () use ($server, $hpack) {
        yield delay(100);

        $server->write(self::packFrame($hpack->encode([
            [":status", Status::OK],
            ["content-length", "8"],
            ["date", formatDateHeader()],
        ]), Http2Parser::HEADERS, Http2Parser::END_HEADERS, 1));

        yield delay(100);

        $server->write(self::packFrame('test', Http2Parser::DATA, Http2Parser::NO_FLAG, 1));

        yield delay(1000);

        $server->write(self::packFrame('test', Http2Parser::DATA, Http2Parser::END_STREAM, 1));
    });

    /** @var Response $response */
    $response = yield $stream->request($request, new NullCancellationToken);

    $this->assertSame(200, $response->getStatus());

    try {
        $this->assertSame('test', yield $response->getBody()->buffer());
        $this->fail("The request body should have been cancelled");
    } catch (CancelledException $exception) {
        $buffer = yield $server->read();
        $expected = self::packFrame(\pack("N", Http2Parser::CANCEL), Http2Parser::RST_STREAM, Http2Parser::NO_FLAG, 1);
        $this->assertStringEndsWith($expected, $buffer);
    }
}

We may want to add the ability to have a separate cancellation token for the body. For example, I may want to receive a response from the server within 5 seconds, but maybe I'm downloading a large file that will take much longer than that. Right now there isn't an easy way to do that.

I guess what you're looking for is something like a socket inactivity timeout regardless of whether we're still in the headers or already in the body?

[trowski]

I guess what you're looking for is something like a socket inactivity timeout regardless of whether we're still in the headers or already in the body?

Yes, exactly. Something we can probably add to Request.