Bug 1878140 - Make sendBeacon fallback the default uploader.

CHANGELOG.md

@@ -2,7 +2,8 @@
 
 [Full changelog](https://github.com/mozilla/glean.js/compare/v4.0.0...main)
 
-* [#1866](https://github.com/mozilla/glean.js/pull/1866): Added a new uploader that falls back to `fetch` if `navigator.sendBeacon` fails. This is not used by default and needs to be enabled manually.
+* [#1866](https://github.com/mozilla/glean.js/pull/1866): Added a new uploader that falls back to `fetch` if `navigator.sendBeacon` fails. 
+* [#1876](https://github.com/mozilla/glean.js/pull/1876): **BREAKING CHANGE**: `navigator.sendBeacon` with fallback to `fetch` (see #1866) is now the default uploader. This can be changed manually.
 
 # v4.0.0 (2024-01-24)
 

glean/src/core/upload/worker.ts

@@ -70,22 +70,9 @@ class PingUploadWorker {
     try {
       const finalPing = this.buildPingRequest(ping);
 
-      let safeUploader = this.uploader;
-      if (!this.uploader.supportsCustomHeaders()) {
-        // Some options require us to submit custom headers. Unfortunately not all the
-        // uploaders support them (e.g. `sendBeacon`). In case headers are required, switch
-        // back to the default uploader that, for now, supports headers.
-        const needsHeaders = !(
-          (Context.config.sourceTags === undefined) && (Context.config.debugViewTag === undefined)
-        );
-        if (needsHeaders) {
-          safeUploader = Context.platform.uploader;
-        }
-      }
-
       // The POST call has to be asynchronous. Once the API call is triggered,
       // we rely on the browser's "keepalive" header.
-      return safeUploader.post(
+      return this.uploader.post(
         `${this.serverEndpoint}${ping.path}`,
         finalPing
       );
@@ -117,23 +104,23 @@ class PingUploadWorker {
       try {
         const task = getUploadTask();
         switch (task.type) {
-        case UploadTaskTypes.Upload: {
-          if (this.isBlocking) {
-            return;
+          case UploadTaskTypes.Upload: {
+            if (this.isBlocking) {
+              return;
+            }
+
+            this.attemptPingUpload(task.ping)
+              .then((result) => {
+                processUploadResponse(task.ping, result);
+              })
+              .catch((error) => {
+                console.log(error);
+              });
+            continue;
           }
 
-          this.attemptPingUpload(task.ping)
-            .then((result) => {
-              processUploadResponse(task.ping, result);
-            })
-            .catch((error) => {
-              console.log(error);
-            });
-          continue;
-        }
-
-        case UploadTaskTypes.Done:
-          return;
+          case UploadTaskTypes.Done:
+            return;
         }
       } catch (error) {
         log(

glean/src/platform/browser/sendbeacon_fallback_uploader.ts

@@ -10,6 +10,7 @@ import BrowserFetchUploader from "./fetch_uploader.js";
 import BrowserSendBeaconUploader from "./sendbeacon_uploader.js";
 import { UploadResultStatus } from "../../core/upload/uploader.js";
 import type { UploadResult } from "../../core/upload/uploader.js";
+import { Context } from "../../core/context.js";
 
 const LOG_TAG = "platform.browser.SendBeaconFallbackUploader";
 
@@ -23,13 +24,23 @@ class BrowserSendBeaconFallbackUploader extends Uploader {
     pingRequest: PingRequest<string | Uint8Array>
   ): Promise<UploadResult> {
 
-    // Try `sendBeacon` first,
-    // fall back to `fetch` if `sendBeacon` reports an error.
-    const beaconStatus = await this.sendBeaconUploader.post(url, pingRequest, false);
-    if (beaconStatus.result == UploadResultStatus.Success) {
-      return beaconStatus;
+    // Some options require us to submit custom headers. Unfortunately not all the
+    // uploaders support them (e.g. `sendBeacon`). In case headers are required, switch
+    // back to the `fetch` uploader that supports headers.
+    // Then try `sendBeacon` first,
+    // fall back to `fetch` if `sendBeacon` reports an error or `sendBeacon` is
+    // not defined.
+    const hasNoCustomHeaders = !Context.config?.sourceTags && !Context.config?.debugViewTag;
+
+    if (hasNoCustomHeaders && !!navigator && !!navigator.sendBeacon) {
+      const beaconStatus = await this.sendBeaconUploader.post(url, pingRequest, false);
+      if (beaconStatus.result == UploadResultStatus.Success) {
+        return beaconStatus;
+      }
+      log(LOG_TAG, "The `sendBeacon` call was not serviced by the browser. Falling back to the `fetch` uploader.", LoggingLevel.Warn);
+    } else {
+      log(LOG_TAG, "`sendBeacon` is not available. Falling back to the `fetch` uploader.", LoggingLevel.Warn);
     }
-    log(LOG_TAG, "The `sendBeacon` call was not serviced by the browser. Falling back to the fetch uploader.", LoggingLevel.Warn);
 
     return this.fetchUploader.post(url, pingRequest);
   }

glean/src/platform/browser/web/index.ts

@@ -4,7 +4,7 @@
 
 import type Platform from "../../index.js";
 
-import uploader from "../fetch_uploader.js";
+import uploader from "../sendbeacon_fallback_uploader.js";
 import info from "./platform_info.js";
 import Storage from "./storage.js";
 

glean/src/platform/browser/web/platform_info.ts

@@ -84,7 +84,12 @@ const BrowserPlatformInfo: PlatformInfo = {
   },
 
   locale(): string {
-    return navigator.language || "und";
+    if (!!navigator && !!navigator.language) {
+      return navigator.language;
+    }
+    else {
+      return "und";
+    }
   }
 };
 

glean/tests/unit/platform/browser/sendbeacon_fallback_uploader.spec.ts

@@ -33,17 +33,17 @@ function setGlobalSendBeacon() {
 // @ts-ignore
 global.fetch = fetch;
 
-describe("Uploader/BrowserSendBeaconFallback", function () {
+describe("Uploader/BrowserSendBeaconFallback", function() {
   beforeEach(function() {
     setGlobalSendBeacon();
   });
 
-  afterEach(function () {
+  afterEach(function() {
     sandbox.restore();
   });
 
-  it("returns the correct status for successful requests", async function () {
-    const TEST_PING_CONTENT = {"my-test-value": 40721};
+  it("returns the correct status for successful requests", async function() {
+    const TEST_PING_CONTENT = { "my-test-value": 40721 };
     for (const status of [200, 400, 500]) {
       nock(MOCK_ENDPOINT).post(/./i, body => {
         return JSON.stringify(body) == JSON.stringify(TEST_PING_CONTENT);
@@ -60,8 +60,8 @@ describe("Uploader/BrowserSendBeaconFallback", function () {
     }
   });
 
-  it("returns the correct status after fallback", async function () {
-    const TEST_PING_CONTENT = {"my-test-value": 40721};
+  it("returns the correct status after fallback", async function() {
+    const TEST_PING_CONTENT = { "my-test-value": 40721 };
     nock(MOCK_ENDPOINT).post(/./i).reply(200);
 
     // Reset `fetch` to a known state.
@@ -80,7 +80,7 @@ describe("Uploader/BrowserSendBeaconFallback", function () {
     );
   });
 
-  it("returns the correct status when both uploads fail", async function () {
+  it("returns the correct status when both uploads fail", async function() {
     nock(MOCK_ENDPOINT).post(/./i).replyWithError({
       message: "something awful happened",
       code: "AWFUL_ERROR",
@@ -101,4 +101,21 @@ describe("Uploader/BrowserSendBeaconFallback", function () {
       expectedResponse
     );
   });
+
+  it("still sends even when navigator has been unset", async function() {
+    const TEST_PING_CONTENT = { "my-test-value": 40721 };
+    nock(MOCK_ENDPOINT).post(/./i).reply(200);
+
+    // Unset Navigator, to emulate add-on behavior.
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+    // @ts-ignore
+    global.navigator.sendBeacon = undefined;
+
+    const response = BrowserSendBeaconFallbackUploader.post(MOCK_ENDPOINT, new PingRequest("abc", {}, JSON.stringify(TEST_PING_CONTENT), 1024));
+    const expectedResponse = new UploadResult(UploadResultStatus.Success, 200);
+    assert.deepStrictEqual(
+      await response,
+      expectedResponse
+    );
+  });
 });