bug 1437120 - Ensure pings sent after net shutdown are persisted to disk r?Dexter draft
authorChris H-C <chutten@mozilla.com>
Tue, 13 Feb 2018 15:52:28 -0500
changeset 754887 7168db407271f5557494056515b93ac917515ee6
parent 754399 38b3c1d03a594664c6b32c35533734283c258f43
push id99031
push userbmo:chutten@mozilla.com
push dateWed, 14 Feb 2018 14:04:33 +0000
reviewersDexter
bugs1437120, 1397293
milestone60.0a1
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();