Skip sending empty sessions data payload

CHANGELOG.md

@@ -11,6 +11,10 @@ Changelog
 
 ### Bug fixes
 
+* Only send session data when it exists
+  | [martin308](https://github.com/martin308)
+  | [98](https://github.com/bugsnag/bugsnag-dotnet/pull/98)
+
 * Request context is now available in custom middleware
   | [martin308](https://github.com/martin308)
   | [#91](https://github.com/bugsnag/bugsnag-dotnet/pull/91)

src/Bugsnag/SessionsStore.cs

@@ -2,6 +2,7 @@
 using System;
 using System.Collections.Generic;
 using System.Threading;
+using System.Linq;
 
 namespace Bugsnag
 {
@@ -27,7 +28,8 @@ private void SendSessions(object state)
 
       lock (_lock)
       {
-        foreach (var item in _store)
+        // we only care about entries that have session data
+        foreach (var item in _store.Where(d => d.Value.Any()))
         {
           sessionData[item.Key] = new Dictionary<string, long>(item.Value);
           _store[item.Key].Clear();

tests/Bugsnag.AspNet.Core.Tests/UseExceptionHandlerTests.cs

@@ -24,7 +24,7 @@ public Task DisposeAsync()
 
     public async Task InitializeAsync()
     {
-      var bugsnag = new Bugsnag.Tests.TestServer(1);
+      var bugsnag = new Bugsnag.Tests.TestServer();
       bugsnag.Start();
 
       var builder = new WebHostBuilder()
@@ -47,7 +47,7 @@ public async Task InitializeAsync()
       var client = server.CreateClient();
       var response = await client.SendAsync(new System.Net.Http.HttpRequestMessage() { RequestUri = new Uri("/error", UriKind.Relative) });
 
-      var bugsnags = await bugsnag.Requests();
+      var bugsnags = await bugsnag.Requests(1);
 
       BugsnagPayload = bugsnags.First();
     }

tests/Bugsnag.AspNet.Core.Tests/WebHostTests.cs

@@ -30,7 +30,7 @@ public async void TestWithNoExceptions()
     [Fact]
     public async void TestWithDeveloperExceptionPage()
     {
-      var bugsnag = new Bugsnag.Tests.TestServer(1);
+      var bugsnag = new Bugsnag.Tests.TestServer();
       bugsnag.Start();
 
       var builder = new WebHostBuilder()
@@ -46,7 +46,7 @@ public async void TestWithDeveloperExceptionPage()
       var client = server.CreateClient();
       var response = await client.SendAsync(new System.Net.Http.HttpRequestMessage());
 
-      var bugsnags = await bugsnag.Requests();
+      var bugsnags = await bugsnag.Requests(1);
 
       Assert.NotNull(response);
     }

tests/Bugsnag.AspNet.Tests/ClientTests.cs

@@ -19,7 +19,7 @@ public Task DisposeAsync()
 
     public async Task InitializeAsync()
     {
-      var server = new TestServer(1);
+      var server = new TestServer();
 
       server.Start();
 
@@ -37,7 +37,7 @@ public async Task InitializeAsync()
         });
       }
 
-      var requests = await server.Requests();
+      var requests = await server.Requests(1);
 
       _request = requests.Single();
     }

tests/Bugsnag.AspNet.WebApi.Tests/WebHostTests.cs

@@ -22,7 +22,7 @@ public IHttpActionResult Test()
     [Fact]
     public async void Test()
     {
-      var bugsnagServer = new TestServer(1);
+      var bugsnagServer = new TestServer();
 
       bugsnagServer.Start();
 
@@ -37,7 +37,7 @@ public async void Test()
 
       var response = await client.SendAsync(request);
 
-      var responses = await bugsnagServer.Requests();
+      var responses = await bugsnagServer.Requests(1);
 
       Assert.Single(responses);
       Assert.Contains("Bugsnag is great!", responses.Single());

tests/Bugsnag.Tests.Server/TestServer.cs

@@ -17,10 +17,10 @@ public class TestServer
     private readonly RequestCollection<string> _requests;
     private readonly int _port;
 
-    public TestServer(int expectedNumberOfRequests)
+    public TestServer()
     {
       _port = PortAllocations.Instance.NextFreePort();
-      _requests = new RequestCollection<string>(expectedNumberOfRequests);
+      _requests = new RequestCollection<string>();
       _webHost = new WebHostBuilder()
         .ConfigureServices(services => services.AddSingleton(typeof(RequestCollection<string>), _requests))
         .UseStartup<Startup>()
@@ -37,9 +37,15 @@ public void Start()
       _webHost.Start();
     }
 
-    public async Task<IEnumerable<string>> Requests()
+    public async Task<IEnumerable<string>> Requests(int numberOfRequests)
     {
-      var items = await _requests.Items();
+      var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
+      return await Requests(numberOfRequests, cts.Token);
+    }
+
+    public async Task<IEnumerable<string>> Requests(int numberOfRequests, CancellationToken token)
+    {
+      var items = await _requests.Items(numberOfRequests, token);
       await _webHost.StopAsync();
       return items;
     }
@@ -88,16 +94,14 @@ public void Configure(IApplicationBuilder app)
 
     class RequestCollection<T>
     {
-      private readonly int _expectedNumberOfRequests;
       private readonly List<T> _requests;
       private readonly object _requestsLock;
       private readonly TaskCompletionSource<List<T>> _taskCompletionSource;
 
-      public RequestCollection(int expectedNumberOfRequests)
+      public RequestCollection()
       {
         _taskCompletionSource = new TaskCompletionSource<List<T>>();
-        _expectedNumberOfRequests = expectedNumberOfRequests;
-        _requests = new List<T>(expectedNumberOfRequests);
+        _requests = new List<T>();
         _requestsLock= new object();
       }
 
@@ -106,17 +110,26 @@ public void Add(T item)
         lock (_requestsLock)
         {
           _requests.Add(item);
-
-          if (_requests.Count >= _expectedNumberOfRequests)
-          {
-            _taskCompletionSource.SetResult(_requests);
-          }
         }
       }
 
-      public Task<List<T>> Items()
+      public Task<List<T>> Items(int numberOfRequests, CancellationToken token)
       {
-        return _taskCompletionSource.Task;
+        return Task.Factory.StartNew(async () =>
+        {
+          while (true)
+          {
+            lock (_requestsLock)
+            {
+              if (_requests.Count >= numberOfRequests)
+              {
+                return new List<T>(_requests);
+              }
+            }
+            token.ThrowIfCancellationRequested();
+            await Task.Delay(TimeSpan.FromSeconds(1));
+          }
+        }, token).Unwrap();
       }
     }
   }

tests/Bugsnag.Tests/ClientTests.cs

@@ -12,7 +12,7 @@ public class ClientTests
     [Fact]
     public async void TestThrownException()
     {
-      var server = new TestServer(1);
+      var server = new TestServer();
 
       server.Start();
 
@@ -36,7 +36,7 @@ public async void TestThrownException()
         client.Notify(e);
       }
 
-      var requests = await server.Requests();
+      var requests = await server.Requests(1);
 
       Assert.Single(requests);
     }
@@ -52,7 +52,7 @@ public Task DisposeAsync()
 
       public async Task InitializeAsync()
       {
-        var server = new TestServer(1);
+        var server = new TestServer();
 
         server.Start();
 
@@ -69,7 +69,7 @@ public async Task InitializeAsync()
 
         TestNotifyMethod(client);
 
-        var requests = await server.Requests();
+        var requests = await server.Requests(1);
 
         BugsnagPayload = requests.Single();
       }

tests/Bugsnag.Tests/SessionTrackingTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Threading;
 using Xunit;
 
 namespace Bugsnag.Tests
@@ -16,17 +17,44 @@ public void CurrentSessionIsNullWithoutBeingSet()
     [Fact]
     public async void CurrentSessionCanBeSet()
     {
-      var server = new TestServer(1);
+      var server = new TestServer();
 
       server.Start();
 
-      var sessionTracking = new SessionTracker(new Configuration("123456") { SessionTrackingInterval = TimeSpan.FromSeconds(5), SessionEndpoint = server.Endpoint });
+      var sessionTracking = new SessionTracker(new Configuration("123456") {
+        SessionEndpoint = server.Endpoint
+      });
 
       sessionTracking.CreateSession();
 
-      var requests = await server.Requests();
+      var requests = await server.Requests(1);
 
       Assert.NotNull(sessionTracking.CurrentSession);
     }
+
+    [Fact]
+    public async void EmptySessionsPayloadsAreNotSent()
+    {
+      var server = new TestServer();
+
+      server.Start();
+
+      var sessionTracking = new SessionTracker(new Configuration("123456") {
+        SessionEndpoint = server.Endpoint
+      });
+
+      sessionTracking.CreateSession();
+
+      // set the cancellation token length long enough so that the session
+      // tracker would attempt to send sessions twice
+      var cts = new CancellationTokenSource(TimeSpan.FromSeconds(130));
+
+      // the cancellation token should throw as we are 'expecting' 2 requests
+      // to be received but we only want one to be sent as the second sessions
+      // request will have no sessions data
+      await Assert.ThrowsAsync<OperationCanceledException>(async () => {
+        await server.Requests(2, cts.Token);
+      });
+    }
   }
 }

tests/Bugsnag.Tests/ThreadQueueDeliveryTests.cs

@@ -13,7 +13,7 @@ public async void Test()
     {
       var numberOfRequests = 500;
 
-      var server = new TestServer(numberOfRequests);
+      var server = new TestServer();
 
       server.Start();
 
@@ -23,7 +23,7 @@ public async void Test()
         ThreadQueueDelivery.Instance.Send(payload);
       }
 
-      var requests = await server.Requests();
+      var requests = await server.Requests(numberOfRequests);
 
       Assert.Equal(numberOfRequests, requests.Count());
     }

tests/Bugsnag.Tests/WebRequestTests.cs

@@ -12,7 +12,7 @@ public class WebRequestTests
     public async void Test()
     {
       var numerOfRequests = 1;
-      var server = new TestServer(numerOfRequests);
+      var server = new TestServer();
       server.Start();
 
       var webRequest = new WebRequest();
@@ -23,7 +23,7 @@ public async void Test()
       var response = await Task.Factory.FromAsync((callback, state) => webRequest.BeginSend(server.Endpoint, null, headers, rawPayload, callback, state), webRequest.EndSend, null);
       Assert.Equal(HttpStatusCode.OK, response.HttpStatusCode);
 
-      var requests = await server.Requests();
+      var requests = await server.Requests(numerOfRequests);
 
       Assert.Equal(numerOfRequests, requests.Count());
     }