`IAsyncEnumerator<T>.Dispose` should not rethrow exception that `MoveNextAsync` already threw

[lostmsu]

All the exceptions should be thrown in MoveNextAsync.

The current behavior is surprising to say the least.

[AArnott]

Please share the exception you're seeing with type, message and callstack. Also please describe the scenario you're in when you see it throwing.

Looking at the code below, it appears it would only throw if you called it out of order with MoveNextAsync, which seems reasonable to me.

https://github.com/microsoft/vs-streamjsonrpc/blob/c6a47d46c75929d303ca4cf3c5de6812b9a4d1bd/src/StreamJsonRpc/ProxyGeneration.cs#L1085-L1092

[lostmsu]

OK, you are probably right because the line number points to the .Current invocation, but in the trace I see DisposeAsync and not .Current being called. So probably caused by raising the same exception twice:

Stream failed: StreamJsonRpc.RemoteInvocationException: Unsupported mime type audio/wav
  at StreamJsonRpc.JsonRpc.InvokeCoreAsync[TResult](RequestId id, String targetName, IReadOnlyList`1 arguments, IReadOnlyList`1 positionalArgumentDeclaredTypes, IReadOnlyDictionary`2 namedArgumentDeclaredTypes, CancellationToken cancellationToken, Boolean isParameterObject)
  at StreamJsonRpc.ProxyGeneration.AsyncEnumerableProxy`1.AsyncEnumeratorProxy.<>c__DisplayClass1_0.<<-ctor>b__0>d.MoveNext()
   --- End of stack trace from previous location ---
  at Microsoft.VisualStudio.Threading.AwaitExtensions.ExecuteContinuationSynchronouslyAwaiter`1.GetResult()
  at Microsoft.VisualStudio.Threading.AsyncLazy`1.<>c__DisplayClass20_0.<<GetValueAsync>b__0>d.MoveNext()
   --- End of stack trace from previous location ---
  at StreamJsonRpc.ProxyGeneration.AsyncEnumerableProxy`1.AsyncEnumeratorProxy.DisposeAsync()

Looking at the code my guess is enumeratorLazy.GetValueAsync() throws this exception, it gets handled in my MoveNextAsync() exception handler, then DisposeAsync is called by await using due to leaving the scope, it calls enumeratorLazy.GetValueAsync() again and gets the stored exception, which creates a double fault.

[AArnott]

Does the fault itself make sense? Does your original async enumerable method throw the original mime type exception?

And (more importantly) do you have what you need to resolve the failure on your own at this point?

[lostmsu]

@AArnott I can work around it with some ugly code (e.g. replace await using var enumerator = CallRemote().GetAsyncEnumerator() with 2 nested try ... catch ... blocks), but I believe the issue is still with the library. The behavior right now is the equivalent of file.Read failing followed by file.Dispose failing with exact same exception.

The error itself is correct. The remote method starts with

if (mimeType != "audio/mpeg" && mimeType != "audio/mp4" && mimeType != "audio/ogg")
    throw new NotSupportedException(UnsupportedMimeTypeMessage(mimeType));

Looks like I already have a workaround for a similar https://github.com/microsoft/vs-streamjsonrpc/issues/1174 with enumerator wrapper.

I think in this particular case if the exception has already surfaces in MoveNextAsync, it should not be surfaced again in DisposeAsync.

[AArnott]

if the exception has already surfaces in MoveNextAsync, it should not be surfaced again in DisposeAsync.

Ok, I buy that argument, especially if it leads to odd exception behaviors on the receiver side. Let's keep this issue tracking that. I'm not sure when we'll get to fixing it. And if you happen to be motivated to author a test (and possibly a fix), I'd welcome that.