bug 1437120 - Ensure pings sent after net shutdown are persisted to disk r=Dexter a=jcristau
authorChris H-C <chutten@mozilla.com>
Tue, 13 Feb 2018 15:52:28 -0500
changeset 454925 4a1f0de7b492ddc9973944e002ce34006ccce0c0
parent 454924 bbb6b07c92f9a8f35976e05b5a3d55259830fe01
child 454926 daeb05a6586b5c3ec5b7c4ba1a980f50e3620937
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersDexter, jcristau
bugs1437120, 1397293
milestone59.0
bug 1437120 - Ensure pings sent after net shutdown are persisted to disk r=Dexter a=jcristau 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
@@ -776,21 +776,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();
   },
 
   /**
@@ -1092,19 +1094,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
@@ -420,21 +420,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();