diff --git a/CHANGELOG.md b/CHANGELOG.md index f69c96677..d4a0aac5e 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/glean/src/core/upload/worker.ts b/glean/src/core/upload/worker.ts index fd8779fb1..970d74512 100644 --- a/glean/src/core/upload/worker.ts +++ b/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( diff --git a/glean/src/platform/browser/sendbeacon_fallback_uploader.ts b/glean/src/platform/browser/sendbeacon_fallback_uploader.ts index 5b2abd4fa..9599ae680 100644 --- a/glean/src/platform/browser/sendbeacon_fallback_uploader.ts +++ b/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 ): Promise { - // 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); } diff --git a/glean/src/platform/browser/web/index.ts b/glean/src/platform/browser/web/index.ts index b4fd424a9..a5a182fe5 100644 --- a/glean/src/platform/browser/web/index.ts +++ b/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"; diff --git a/glean/src/platform/browser/web/platform_info.ts b/glean/src/platform/browser/web/platform_info.ts index 4ce21c9af..25a24a26b 100644 --- a/glean/src/platform/browser/web/platform_info.ts +++ b/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"; + } } }; diff --git a/glean/tests/unit/platform/browser/sendbeacon_fallback_uploader.spec.ts b/glean/tests/unit/platform/browser/sendbeacon_fallback_uploader.spec.ts index c8926b4e8..e96b18c1c 100644 --- a/glean/tests/unit/platform/browser/sendbeacon_fallback_uploader.spec.ts +++ b/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 + ); + }); });