vs-streamjsonrpc
vs-streamjsonrpc copied to clipboard
Confusingly named ConnectionLostException thrown on client when client RPC instance is disposed prematurely
Repro:
- Create JsonRpc instance, e.g.: perhaps via brokered service, servicehub service.
- Invoke long running request
- Dispose RPC instance
Expected: Clear indication of what the client is doing wrong, such as ObjectDisposedException or message indicating that the client RPC instance was disposed.
Actual: ConnectionLostException is thrown on the invocation Task. While this matches the documented behavior, it's initially unclear that the client is doing something wrong and it is not the server crashing or forcibly ending the session. As a result I spent a lot of time digging through irrelevant server logs trying to figure out the source of the crash.
Recommendations: It seems like there would be benefit to having a way to differentiate between client-severed-connection and server-severed-connection, especially in fault telemetry.
While this matches the documented behavior
Really? I'm surprised to not find in this doc that this exception might be thrown. Where did you find this behavior documented?
I need to refamiliarize myself with the documented behavior to evaluate risk when changing that behavior. FWIW though, the docs do claim that InvokeAsync can throw ObjectDisposedException so what you're asking for seems like a reasonable idea.
I was thinking specifically of this: https://github.com/microsoft/vs-streamjsonrpc/blob/716e9179a9895cd617b0c07046797578f450cc81/doc/disconnecting.md
[When] the JsonRpc.Dispose method is invoked ... [JsonRpc] faults all outstanding Task objects that represent outbound requests with a ConnectionLostException.
Ah, well that's pretty prescriptive there. Roslyn is pretty particular about the exception types we throw too, so changing this given it's documented there may have to be done with some coordination with them. The docs.microsoft.com web site is wrong either way and should be updated.