Code review feedback

SignalR · Jul 3, 2014 · dc618f2 · dc618f2
1 parent f702f76
commit dc618f2
Show file tree

Hide file tree

Showing 13 changed files with 44 additions and 43 deletions.
diff --git a/.../Microsoft.AspNet.SignalR.LoadTestHarness/Microsoft.AspNet.SignalR.LoadTestHarness.csproj b/.../Microsoft.AspNet.SignalR.LoadTestHarness/Microsoft.AspNet.SignalR.LoadTestHarness.csproj
@@ -33,7 +33,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\</OutputPath>
-    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <CodeAnalysisRuleSet>AllRules.ruleset</CodeAnalysisRuleSet>
@@ -43,7 +43,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\</OutputPath>
-    <DefineConstants>TRACE</DefineConstants>
+    <DefineConstants>TRACE;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <CodeAnalysisRuleSet>AllRules.ruleset</CodeAnalysisRuleSet>

diff --git a/src/Microsoft.AspNet.SignalR.Client/Connection.cs b/src/Microsoft.AspNet.SignalR.Client/Connection.cs
@@ -608,7 +608,7 @@ public void Stop(TimeSpan timeout)
                 {
                     // Close the receive queue so currently running receive callback finishes and no more are run.
                     // We can't wait on the result of the drain because this method may be on the stack of the task returned (aka deadlock).
-                    _receiveQueue.Drain().Catch((format, args) => Trace(TraceLevels.Messages, format, args));
+                    _receiveQueue.Drain().Catch(this);
                 }
 
                 // This is racy since it's outside the _stateLock, but we are trying to avoid 30s deadlocks when calling _transport.Abort()

diff --git a/src/Microsoft.AspNet.SignalR.Client/Infrastructure/TransportAbortHandler.cs b/src/Microsoft.AspNet.SignalR.Client/Infrastructure/TransportAbortHandler.cs
@@ -71,7 +71,7 @@ public virtual void Abort(IConnection connection, TimeSpan timeout, string conne
                                        ((TransportAbortHandler)state).CompleteAbort();
                                    },
                                    this,
-                                   (format, args) => connection.Trace(TraceLevels.Messages, format, args)
+                                   connection
                                 );
 
                     if (!_abortResetEvent.WaitOne(timeout))

diff --git a/src/Microsoft.AspNet.SignalR.Client/Infrastructure/TransportInitializationHandler.cs b/src/Microsoft.AspNet.SignalR.Client/Infrastructure/TransportInitializationHandler.cs
@@ -98,9 +98,7 @@ private void Start()
                                     CompleteFail(new StartException(Resources.Error_StartFailed));
                                 }
                             })
-                            .Catch(ex => CompleteFail(new StartException(Resources.Error_StartFailed, ex)),
-                                   (format, args) => _connection.Trace(TraceLevels.Messages, format, args)
-                            );
+                            .Catch(ex => CompleteFail(new StartException(Resources.Error_StartFailed, ex)), _connection);
         }
 
         private void CompleteStart()

diff --git a/src/Microsoft.AspNet.SignalR.Client/Transports/HttpBasedTransport.cs b/src/Microsoft.AspNet.SignalR.Client/Transports/HttpBasedTransport.cs
@@ -60,7 +60,7 @@ public override Task Send(IConnection connection, string data, string connection
                         connection.OnReceived(connection.JsonDeserializeObject<JObject>(raw));
                     }
                 })
-                .Catch(connection.OnError, (format, args) => connection.Trace(TraceLevels.Messages, format, args));
+                .Catch(connection.OnError, connection);
         }
     }
 }
diff --git a/src/Microsoft.AspNet.SignalR.Core/Hubs/HubInvocationProgress.cs b/src/Microsoft.AspNet.SignalR.Core/Hubs/HubInvocationProgress.cs
@@ -22,7 +22,7 @@ protected HubInvocationProgress(Func<object, Task> sendProgressFunc)
             _sendProgressFunc = sendProgressFunc;
         }
 
-        protected TraceSource Trace { get; private set; }
+        private TraceSource Trace { get; set; }
 
         public static HubInvocationProgress Create(Type progressGenericType, Func<object, Task> sendProgressFunc, TraceSource traceSource)
         {

diff --git a/src/Microsoft.AspNet.SignalR.Core/Microsoft.AspNet.SignalR.Core.csproj b/src/Microsoft.AspNet.SignalR.Core/Microsoft.AspNet.SignalR.Core.csproj
@@ -22,7 +22,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\Debug\</OutputPath>
-    <DefineConstants>TRACE;DEBUG;PERFCOUNTERS</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;PERFCOUNTERS,SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Debug\Microsoft.AspNet.SignalR.Core.XML</DocumentationFile>
@@ -32,7 +32,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\Release\</OutputPath>
-    <DefineConstants>TRACE;PERFCOUNTERS</DefineConstants>
+    <DefineConstants>TRACE;PERFCOUNTERS,SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Release\Microsoft.AspNet.SignalR.Core.XML</DocumentationFile>

diff --git a/src/Microsoft.AspNet.SignalR.Core/TaskAsyncHelper.cs b/src/Microsoft.AspNet.SignalR.Core/TaskAsyncHelper.cs
@@ -11,6 +11,9 @@
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
+#if !SERVER
+using Microsoft.AspNet.SignalR.Client;
+#endif
 using Microsoft.AspNet.SignalR.Infrastructure;
 
 namespace Microsoft.AspNet.SignalR
@@ -94,15 +97,15 @@ public static Task<T> FromAsync<T>(Func<AsyncCallback, object, IAsyncResult> beg
         }
 
         [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "This is a shared file")]
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
         public static TTask Catch<TTask>(this TTask task, TraceSource traceSource = null) where TTask : Task
         {
             return Catch(task, ex => { }, traceSource);
         }
 #else
-        public static TTask Catch<TTask>(this TTask task, Action<string, object[]> traceMethod = null) where TTask : Task
+        public static TTask Catch<TTask>(this TTask task, IConnection connection = null) where TTask : Task
         {
-            return Catch(task, ex => { }, traceMethod);
+            return Catch(task, ex => { }, connection);
         }
 #endif
 
@@ -125,28 +128,28 @@ public static TTask Catch<TTask>(this TTask task, TraceSource traceSource, param
 #endif
 
         [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "This is a shared file")]
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
         public static TTask Catch<TTask>(this TTask task, Action<AggregateException, object> handler, object state, TraceSource traceSource = null) where TTask : Task
 #else
-        public static TTask Catch<TTask>(this TTask task, Action<AggregateException, object> handler, object state, Action<string, object[]> traceMethod = null) where TTask : Task
+        public static TTask Catch<TTask>(this TTask task, Action<AggregateException, object> handler, object state, IConnection connection = null) where TTask : Task
 #endif
         {
             if (task != null && task.Status != TaskStatus.RanToCompletion)
             {
                 if (task.Status == TaskStatus.Faulted)
                 {
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
                     ExecuteOnFaulted(handler, state, task.Exception, traceSource);
 #else
-                    ExecuteOnFaulted(handler, state, task.Exception, traceMethod);
+                    ExecuteOnFaulted(handler, state, task.Exception, connection);
 #endif
                 }
                 else
                 {
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
                     AttachFaultedContinuation<TTask>(task, handler, state, traceSource);
 #else
-                    AttachFaultedContinuation<TTask>(task, handler, state, traceMethod);
+                    AttachFaultedContinuation<TTask>(task, handler, state, connection);
 #endif
                 }
             }
@@ -155,58 +158,58 @@ public static TTask Catch<TTask>(this TTask task, Action<AggregateException, obj
         }
 
         [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "This is a shared file")]
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
         private static void AttachFaultedContinuation<TTask>(TTask task, Action<AggregateException, object> handler, object state, TraceSource traceSource) where TTask : Task
 #else      
-        private static void AttachFaultedContinuation<TTask>(TTask task, Action<AggregateException, object> handler, object state, Action<string, object[]> traceMethod) where TTask : Task
+        private static void AttachFaultedContinuation<TTask>(TTask task, Action<AggregateException, object> handler, object state, IConnection connection) where TTask : Task
 #endif
         {
             task.ContinueWithPreservedCulture(innerTask =>
             {
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
                 ExecuteOnFaulted(handler, state, innerTask.Exception, traceSource);
 #else
-                ExecuteOnFaulted(handler, state, innerTask.Exception, traceMethod);
+                ExecuteOnFaulted(handler, state, innerTask.Exception, connection);
 #endif
             },
             TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously);
         }
 
         [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "This is a shared file")]
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
         private static void ExecuteOnFaulted(Action<AggregateException, object> handler, object state, AggregateException exception, TraceSource traceSource)
 #else
-        private static void ExecuteOnFaulted(Action<AggregateException, object> handler, object state, AggregateException exception, Action<string, object[]> traceMethod)
+        private static void ExecuteOnFaulted(Action<AggregateException, object> handler, object state, AggregateException exception, IConnection connection)
 #endif
         {
             // Observe Exception
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
             if (traceSource != null)
             {
                 traceSource.TraceEvent(TraceEventType.Warning, 0, "Exception thrown by Task: {0}", exception);
             }
 #else
-            if (traceMethod != null)
+            if (connection != null)
             {
-                traceMethod("Exception thrown by Task: {0}", new [] { exception });
+                connection.Trace(TraceLevels.Messages, "Exception thrown by Task: {0}", new [] { exception });
             }
 #endif
             handler(exception, state);
         }
 
         [SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode", Justification = "This is a shared file")]
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
         public static TTask Catch<TTask>(this TTask task, Action<AggregateException> handler, TraceSource traceSource = null) where TTask : Task
 #else
-        public static TTask Catch<TTask>(this TTask task, Action<AggregateException> handler, Action<string, object[]> traceMethod = null) where TTask : Task
+        public static TTask Catch<TTask>(this TTask task, Action<AggregateException> handler, IConnection connection = null) where TTask : Task
 #endif
         {
             return task.Catch((ex, state) => ((Action<AggregateException>)state).Invoke(ex),
                               handler,
-#if !PORTABLE && !NETFX_CORE && !__ANDROID__ && !IOS && !CLIENT_NET4 && !CLIENT_NET45
+#if SERVER
                               traceSource
 #else
- traceMethod
+                              connection
 #endif
                               );
         }

diff --git a/src/Microsoft.AspNet.SignalR.Core/Transports/TransportDisconnectBase.cs b/src/Microsoft.AspNet.SignalR.Core/Transports/TransportDisconnectBase.cs
@@ -306,7 +306,7 @@ private Task Abort(bool clean)
             var disconnectTask = Disconnected != null ? Disconnected(clean) : TaskAsyncHelper.Empty;
 
             // Ensure delegate continues to use the C# Compiler static delegate caching optimization.
-            return disconnectTask.Catch((ex, state) => OnDisconnectError(ex, state), Trace, Trace)
+            return disconnectTask.Catch((ex, state) => OnDisconnectError(ex, state), state: Trace, traceSource: Trace)
                                  .Then(counters => counters.ConnectionsDisconnected.Increment(), _counters);
         }
 

diff --git a/src/Microsoft.AspNet.SignalR.Redis/Microsoft.AspNet.SignalR.Redis.csproj b/src/Microsoft.AspNet.SignalR.Redis/Microsoft.AspNet.SignalR.Redis.csproj
@@ -20,7 +20,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\Debug\</OutputPath>
-    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Debug\Microsoft.AspNet.SignalR.Redis.XML</DocumentationFile>
@@ -30,7 +30,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\Release\</OutputPath>
-    <DefineConstants>TRACE</DefineConstants>
+    <DefineConstants>TRACE;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Release\Microsoft.AspNet.SignalR.Redis.XML</DocumentationFile>

diff --git a/src/Microsoft.AspNet.SignalR.ServiceBus/Microsoft.AspNet.SignalR.ServiceBus.csproj b/src/Microsoft.AspNet.SignalR.ServiceBus/Microsoft.AspNet.SignalR.ServiceBus.csproj
@@ -20,7 +20,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\Debug\</OutputPath>
-    <DefineConstants>TRACE;DEBUG</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -31,7 +31,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\Release\</OutputPath>
-    <DefineConstants>TRACE</DefineConstants>
+    <DefineConstants>TRACE;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>

diff --git a/src/Microsoft.AspNet.SignalR.SqlServer/Microsoft.AspNet.SignalR.SqlServer.csproj b/src/Microsoft.AspNet.SignalR.SqlServer/Microsoft.AspNet.SignalR.SqlServer.csproj
@@ -20,7 +20,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\Debug\</OutputPath>
-    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Debug\Microsoft.AspNet.SignalR.SqlServer.XML</DocumentationFile>
@@ -30,7 +30,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\Release\</OutputPath>
-    <DefineConstants>TRACE</DefineConstants>
+    <DefineConstants>TRACE;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <DocumentationFile>bin\Release\Microsoft.AspNet.SignalR.SqlServer.XML</DocumentationFile>

diff --git a/src/Microsoft.AspNet.SignalR.Stress/Microsoft.AspNet.SignalR.Stress.csproj b/src/Microsoft.AspNet.SignalR.Stress/Microsoft.AspNet.SignalR.Stress.csproj
@@ -23,7 +23,7 @@
     <DebugType>full</DebugType>
     <Optimize>false</Optimize>
     <OutputPath>bin\Debug\</OutputPath>
-    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <DefineConstants>TRACE;DEBUG;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <Prefer32Bit>false</Prefer32Bit>
@@ -33,7 +33,7 @@
     <DebugType>pdbonly</DebugType>
     <Optimize>true</Optimize>
     <OutputPath>bin\Release\</OutputPath>
-    <DefineConstants>TRACE</DefineConstants>
+    <DefineConstants>TRACE;SERVER</DefineConstants>
     <ErrorReport>prompt</ErrorReport>
     <WarningLevel>4</WarningLevel>
     <Prefer32Bit>false</Prefer32Bit>
@@ -42,7 +42,7 @@
   <PropertyGroup Condition="'$(PerfRun)'=='True'">
     <DefineConstants>$(DefineConstants);PERFRUN</DefineConstants>
   </PropertyGroup>
-  <Choose>   
+  <Choose>
     <When Condition="'$(PerfRun)'=='True'">
       <ItemGroup>
         <Reference Include="Microsoft.VisualStudio.Diagnostics.Measurement">