From 6f507460564379aa76bd605ddf37dc22c4194836 Mon Sep 17 00:00:00 2001
From: Andrew Arnott <andrewarnott@gmail.com>
Date: Tue, 9 Jun 2020 12:45:53 -0600
Subject: [PATCH] Trace message transmission failures

We didn't trace these before, and while `NotifyAsync` and other methods will propagate the exception to the caller, if the NotifyAsync call was made by a dynamically generated proxy in response to raising an event, the exception would be swallowed without a trace.

This also fixes invocation of the virtual `JsonRpc.OnRequestTransmissionAborted` method so that we only invoke it on actual transmission failures and not on all errors in receiving a response too.
---
 src/StreamJsonRpc.Tests/JsonRpcTests.cs          | 12 ++++++++++++
 src/StreamJsonRpc/JsonRpc.cs                     | 16 +++++++++++-----
 .../netcoreapp2.1/PublicAPI.Unshipped.txt        |  1 +
 .../netstandard2.0/PublicAPI.Unshipped.txt       |  1 +
 4 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/StreamJsonRpc.Tests/JsonRpcTests.cs b/src/StreamJsonRpc.Tests/JsonRpcTests.cs
index 2031abe7..5390a682 100644
--- a/src/StreamJsonRpc.Tests/JsonRpcTests.cs
+++ b/src/StreamJsonRpc.Tests/JsonRpcTests.cs
@@ -955,6 +955,18 @@ public async Task ReportProgressWithUnserializableData_LeavesTraceEvidence()
         }
     }
 
+    [Fact]
+    public async Task NotifyAsync_LeavesTraceEvidenceOnFailure()
+    {
+        var exception = await Assert.ThrowsAnyAsync<Exception>(() => this.clientRpc.NotifyAsync("DoesNotMatter", new TypeThrowsWhenSerialized()));
+
+        // Verify that the trace explains what went wrong with the original exception message.
+        while (!this.clientTraces.Messages.Any(m => m.Contains("Can't touch this")))
+        {
+            await this.clientTraces.MessageReceived.WaitAsync(this.TimeoutToken);
+        }
+    }
+
     [Fact]
     public async Task InvokeWithParameterObject_ProgressParameterAndFields()
     {
diff --git a/src/StreamJsonRpc/JsonRpc.cs b/src/StreamJsonRpc/JsonRpc.cs
index cda4ace7..da91c64a 100644
--- a/src/StreamJsonRpc/JsonRpc.cs
+++ b/src/StreamJsonRpc/JsonRpc.cs
@@ -381,6 +381,11 @@ public enum TraceEvents
             /// Thus a failure recorded in this event may be followed by a successful deserialization to another parameter type and invocation of a different overload.
             /// </remarks>
             MethodArgumentDeserializationFailure,
+
+            /// <summary>
+            /// An outgoing RPC message was not sent due to an exception, possibly a serialization failure.
+            /// </summary>
+            TransmissionFailed,
         }
 
         /// <summary>
@@ -1721,11 +1726,6 @@ private RemoteRpcException CreateExceptionFromRpcError(JsonRpcError response, st
             {
                 throw new ConnectionLostException(Resources.ConnectionDropped, ex);
             }
-            catch (Exception)
-            {
-                this.OnRequestTransmissionAborted(request);
-                throw;
-            }
         }
 
         private JsonRpcError CreateError(JsonRpcRequest request, Exception exception)
@@ -2592,6 +2592,12 @@ private async ValueTask TransmitAsync(JsonRpcMessage message, CancellationToken
                     this.OnJsonRpcDisconnected(e);
                 }
 
+                this.TraceSource.TraceEvent(TraceEventType.Error, (int)TraceEvents.TransmissionFailed, "Exception thrown while transmitting message: {0}", exception);
+                if (message is JsonRpcRequest request)
+                {
+                    this.OnRequestTransmissionAborted(request);
+                }
+
                 throw;
             }
         }
diff --git a/src/StreamJsonRpc/netcoreapp2.1/PublicAPI.Unshipped.txt b/src/StreamJsonRpc/netcoreapp2.1/PublicAPI.Unshipped.txt
index b2408592..ea6b0f4b 100644
--- a/src/StreamJsonRpc/netcoreapp2.1/PublicAPI.Unshipped.txt
+++ b/src/StreamJsonRpc/netcoreapp2.1/PublicAPI.Unshipped.txt
@@ -1,5 +1,6 @@
 StreamJsonRpc.DisconnectedReason.RemoteProtocolViolation = 6 -> StreamJsonRpc.DisconnectedReason
 StreamJsonRpc.JsonRpc.DispatchCompletion.get -> System.Threading.Tasks.Task!
+StreamJsonRpc.JsonRpc.TraceEvents.TransmissionFailed = 18 -> StreamJsonRpc.JsonRpc.TraceEvents
 StreamJsonRpc.JsonRpcEnumerableSettings.Prefetch.get -> int
 StreamJsonRpc.JsonRpcEnumerableSettings.Prefetch.set -> void
 StreamJsonRpc.JsonRpcTargetOptions.UseSingleObjectParameterDeserialization.get -> bool
diff --git a/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt b/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt
index b2408592..ea6b0f4b 100644
--- a/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt
+++ b/src/StreamJsonRpc/netstandard2.0/PublicAPI.Unshipped.txt
@@ -1,5 +1,6 @@
 StreamJsonRpc.DisconnectedReason.RemoteProtocolViolation = 6 -> StreamJsonRpc.DisconnectedReason
 StreamJsonRpc.JsonRpc.DispatchCompletion.get -> System.Threading.Tasks.Task!
+StreamJsonRpc.JsonRpc.TraceEvents.TransmissionFailed = 18 -> StreamJsonRpc.JsonRpc.TraceEvents
 StreamJsonRpc.JsonRpcEnumerableSettings.Prefetch.get -> int
 StreamJsonRpc.JsonRpcEnumerableSettings.Prefetch.set -> void
 StreamJsonRpc.JsonRpcTargetOptions.UseSingleObjectParameterDeserialization.get -> bool