OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

Adding a test to ensure data streams respect TcpClientReceiveResponseTransmissionAfterInitialReadTimeout #551

Closed sburmanoctopus closed 12 months ago

sburmanoctopus commented 12 months ago

[sc-42848]

Background

We wanted to make sure data streams respected the new TcpClientReceiveResponseTransmissionAfterInitialReadTimeout. Turns out they did. So let's write a test to prove it.

The data streams work because MessageExchangeStream.ReceiveResponseAsync calls ReceiveAsync using the new timeouts, and ReceiveAsync calls ReadStreamsAsync. So the timeouts are in place already.

public async Task<ResponseMessage> ReceiveResponseAsync(CancellationToken cancellationToken)
{
    // Wait for data to become available using existing timeouts, then once we have data streaming in, use the smaller timeout (so we do not wait as long if an error happens here).
    await stream.WithReadTimeout(
        halibutTimeoutsAndLimits.TcpClientReceiveResponseTimeout,
        async () => await stream.WaitForDataToBeAvailableAsync(cancellationToken));

    return await stream.WithReadTimeout(
        halibutTimeoutsAndLimits.TcpClientReceiveResponseTransmissionAfterInitialReadTimeout,
        async () => await ReceiveAsync<ResponseMessage>(cancellationToken));
}

async Task<T> ReceiveAsync<T>(CancellationToken cancellationToken)
{
    var (result, dataStreams) = await serializer.ReadMessageAsync<T>(stream, cancellationToken);
    await ReadStreamsAsync(dataStreams, cancellationToken);
    log.Write(EventType.Diagnostic, "Received: {0}", result);
    return result;
}

How to review this PR

Quality :heavy_check_mark:

Pre-requisites

sburmanoctopus commented 12 months ago

@LukeButters , I also ninja'd in a namespace change, as the version in the project name and namespace did not align 😁

shortcut-integration[bot] commented 12 months ago

This pull request has been linked to Shortcut Story #42848: Significantly reduce the current TCP Timeout of 10 minutes (not control messages).