microsoft / microsoft/vs-streamjsonrpc

Surprising behavior with IAsyncEnumerable: ConnectionLostException on disposing iterator

Open
#1,174 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
937
Forks
178
Avg merge
3d 1h
Merged PRs (30d)
28

Description

Repro

[Fact]
public async Task ConnectionLost_MoveNext()
{
    await foreach (int number in this.clientProxy.Value.GetNumbersAsync(this.TimeoutToken))
    {
    }

    // Simulate a connection loss by disposing the server.
    this.serverRpc.Dispose();
    await using var enumerator = this.clientProxy.Value.GetNumbersAsync(this.TimeoutToken)
                                                       .GetAsyncEnumerator(this.TimeoutToken);
    try
    {
        await enumerator.MoveNextAsync();
    } catch (ConnectionLostException e)
    {
        return;
    }
}

Expected

Test passes

Actual

Test fails with

  StreamJsonRpc.ConnectionLostException : The JSON-RPC connection with the remote party was lost before the request could complete.
  ---- System.OperationCanceledException : The operation was canceled.

Stack Trace: 
  JsonRpc.InvokeCoreAsync(JsonRpcRequest request, Type expectedResultType, CancellationToken cancellationToken) line 1982
  JsonRpc.InvokeCoreAsync[TResult](RequestId id, String targetName, IReadOnlyList`1 arguments, IReadOnlyList`1 positionalArgumentDeclaredTypes, IReadOnlyDictionary`2 namedArgumentDeclaredTypes, CancellationToken cancellationToken, Boolean isParameterObject) line 1564
  <<-ctor>b__0>d.MoveNext() line 1076
  --- End of stack trace from previous location ---
  ExecuteContinuationSynchronouslyAwaiter`1.GetResult()
  <<GetValueAsync>b__0>d.MoveNext()
  --- End of stack trace from previous location ---
  AsyncEnumeratorProxy.DisposeAsync() line 1102
  AsyncEnumerableTests.ConnectionLost_MoveNext() line 469
  --- End of stack trace from previous location ---
  ----- Inner Stack Trace -----
  CancellationToken.ThrowOperationCanceledException()
  CancellationToken.ThrowIfCancellationRequested()
  MessageHandlerBase.WriteAsync(JsonRpcMessage content, CancellationToken cancellationToken) line 218
  JsonRpc.SendAsync(JsonRpcMessage message, CancellationToken cancellationToken) line 1720
  JsonRpc.InvokeCoreAsync(JsonRpcRequest request, Type expectedResultType, CancellationToken cancellationToken) line 1949

Remarks

This happens because DisposeAsync throws when connection is already gone.

It breaks ergonomics on using async iterators as you can no longer rely on await using var iterator = ...GetAsyncEnumerator to correctly dispose the iterator on failure - you have to wrap the whole thing in try ... catch. Normally you only have to do it for MoveNextAsync.

It also violates CA1065

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 AsyncEnumerableTests.ConnectionLost_MoveNext at line 469 and reproduce the failure after disposing the server RPC. Then inspect AsyncEnumeratorProxy.DisposeAsync at line 1102 and the related JsonRpc invocation path shown in the stack trace. Done means the existing scenario passes with the connection-loss exception handled by MoveNextAsync rather than unexpectedly escaping during iterator disposal.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.