Bug 973579 - TelemetryFile should fail silently when not overwriting an existing file. r=Yoric

toolkit/components/telemetry/TelemetryFile.jsm

@@ -64,14 +64,20 @@ this.TelemetryFile = {
    *
    * @param {object} ping The content of the ping to save.
    * @param {string} file The destination file.
-   * @param {bool} overwrite If |true|, the file will be overwritten
-   * if it exists.
+   * @param {bool} overwrite If |true|, the file will be overwritten if it exists,
+   * if |false| the file will not be overwritten and no error will be reported if
+   * the file exists.
    * @returns {promise}
    */
   savePingToFile: function(ping, file, overwrite) {
-    let pingString = JSON.stringify(ping);
-    return OS.File.writeAtomic(file, pingString, {tmpPath: file + ".tmp",
-                                noOverwrite: !overwrite});
+    return Task.spawn(function*() {
+      try {
+        let pingString = JSON.stringify(ping);
+        yield OS.File.writeAtomic(file, pingString, {tmpPath: file + ".tmp",
+                                  noOverwrite: !overwrite});
+      } catch(e if e.becauseExists) {
+      }
+    })
   },
 
   /**
@@ -86,7 +92,7 @@ this.TelemetryFile = {
     return Task.spawn(function*() {
       yield getPingDirectory();
       let file = pingFilePath(ping);
-      return this.savePingToFile(ping, file, overwrite);
+      yield this.savePingToFile(ping, file, overwrite);
     }.bind(this));
   },
 
@@ -98,6 +104,8 @@ this.TelemetryFile = {
    */
   savePendingPings: function(sessionPing) {
     let p = pendingPings.reduce((p, ping) => {
+      // Restore the files with the previous pings if for some reason they have
+      // been deleted, don't overwrite them otherwise.
       p.push(this.savePing(ping, false));
       return p;}, [this.savePing(sessionPing, true)]);
 

toolkit/components/telemetry/TelemetryPing.jsm

@@ -789,13 +789,13 @@ let Impl = {
 
     function handler(success) {
       return function(event) {
-        this.finishPingRequest(success, startTime, ping);
-
-        if (success) {
-          deferred.resolve();
-        } else {
-          deferred.reject(event);
-        }
+        this.finishPingRequest(success, startTime, ping).then(() => {
+          if (success) {
+            deferred.resolve();
+          } else {
+            deferred.reject(event);
+          }
+        });
       };
     }
     request.addEventListener("error", handler(false).bind(this), false);

toolkit/components/telemetry/tests/unit/test_TelemetryPing.js

@@ -18,6 +18,7 @@ Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/LightweightThemeManager.jsm", this);
 Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
 Cu.import("resource://gre/modules/TelemetryPing.jsm", this);
+Cu.import("resource://gre/modules/TelemetryFile.jsm", this);
 Cu.import("resource://gre/modules/Task.jsm", this);
 Cu.import("resource://gre/modules/Promise.jsm", this);
 
@@ -399,6 +400,14 @@ function actualTest() {
   run_next_test();
 }
 
+// Ensure that not overwriting an existing file fails silently
+add_task(function* test_overwritePing() {
+  let ping = {slug: "foo"}
+  yield TelemetryFile.savePing(ping, true);
+  yield TelemetryFile.savePing(ping, false);
+  yield TelemetryFile.cleanupPingFile(ping);
+});
+
 // Ensures that expired histograms are not part of the payload.
 add_task(function* test_expiredHistogram() {
   let histogram_id = "FOOBAR";