bug 1437120 - Ensure pings sent after net shutdown are persisted to disk r=Dexter
authorChris H-C <chutten@mozilla.com>
Tue, 13 Feb 2018 15:52:28 -0500
changeset 403772 a0a032ce94f89b2a738037aa4bd71cf9277948e9
parent 403771 0bb9c745a9dfc887c40118923e3818bfb502d133
child 403773 ab50bf89a3ac2937572aa2fc176acbf5cc52682e
push id33444
push userapavel@mozilla.com
push dateThu, 15 Feb 2018 09:59:25 +0000
treeherdermozilla-central@026401920e32 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersDexter
bugs1437120, 1397293
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1437120 - Ensure pings sent after net shutdown are persisted to disk r=Dexter bug 1397293 introduced a mechanism by which we would bail out early in trying to send a ping if we were trying to send it after the network had been torn down. Unfortunately, it did so indistinguishably from the case where we weren't allowed to send pings, so we neglected to save the ping (as "pending") and just archived it. This change cleanly rejects the ping, and correctly tests that the rejected ping is persisted, not just ephemerally hanging in memory for but a few moments longer. MozReview-Commit-ID: 2g8cpeBEzSE
toolkit/components/telemetry/TelemetrySend.jsm
toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
--- a/toolkit/components/telemetry/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/TelemetrySend.jsm
@@ -779,21 +779,23 @@ var TelemetrySendImpl = {
   reset() {
     this._log.trace("reset");
 
     this._shutdown = false;
     this._currentPings = new Map();
     this._overduePingCount = 0;
     this._tooLateToSend = false;
     this._isOSShutdown = false;
+    this._sendingEnabled = true;
 
     const histograms = [
       "TELEMETRY_SUCCESS",
       "TELEMETRY_SEND_SUCCESS",
       "TELEMETRY_SEND_FAILURE",
+      "TELEMETRY_SEND_FAILURE_TYPE",
     ];
 
     histograms.forEach(h => Telemetry.getHistogramById(h).clear());
 
     return SendScheduler.reset();
   },
 
   /**
@@ -1095,19 +1097,20 @@ var TelemetrySendImpl = {
   _doPing(ping, id, isPersisted) {
     if (!this.sendingEnabled(ping)) {
       // We can't send the pings to the server, so don't try to.
       this._log.trace("_doPing - Can't send ping " + ping.id);
       return Promise.resolve();
     }
 
     if (this._tooLateToSend) {
+      // Too late to send now. Reject so we pend the ping to send it next time.
       this._log.trace("_doPing - Too late to send ping " + ping.id);
       Telemetry.getHistogramById("TELEMETRY_SEND_FAILURE_TYPE").add("eTooLate");
-      return Promise.resolve();
+      return Promise.reject();
     }
 
     this._log.trace("_doPing - server: " + this._server + ", persisted: " + isPersisted +
                     ", id: " + id);
 
     const url = this._buildSubmissionURL(ping);
 
     let request = new ServiceRequest();
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
@@ -436,21 +436,29 @@ add_task(async function test_tooLateToSe
   await TelemetrySend.reset();
   PingServer.start();
   PingServer.registerPingHandler(() => Assert.ok(false, "Should not have received any pings now"));
 
   Assert.equal(TelemetrySend.pendingPingCount, 0, "Should have no pending pings yet");
 
   TelemetrySend.testTooLateToSend(true);
 
-  TelemetryController.submitExternalPing(TEST_TYPE, {});
-  Assert.equal(TelemetrySend.pendingPingCount, 1, "Should not send the ping, should pend delivery");
+  const id = await TelemetryController.submitExternalPing(TEST_TYPE, {});
+
+  // Triggering a shutdown should persist the pings
+  await TelemetrySend.shutdown();
+  const pendingPings = TelemetryStorage.getPendingPingList();
+  Assert.equal(pendingPings.length, 1, "Should have a pending ping in storage");
+  Assert.equal(pendingPings[0].id, id, "Should have pended our test's ping");
 
   Assert.equal(Telemetry.getHistogramById("TELEMETRY_SEND_FAILURE_TYPE").snapshot().counts[7], 1,
     "Should have registered the failed attempt to send");
+
+  await TelemetryStorage.reset();
+  Assert.equal(TelemetrySend.pendingPingCount, 0, "Should clean up after yourself");
 });
 
 // Test that the current, non-persisted pending pings are properly saved on shutdown.
 add_task(async function test_persistCurrentPingsOnShutdown() {
   const TEST_TYPE = "test-persistCurrentPingsOnShutdown";
   const PING_COUNT = 5;
   await TelemetrySend.reset();
   PingServer.stop();