Analytics fixes

packages/workbox-background-sync/Queue.mjs

@@ -124,6 +124,10 @@ class Queue {
 
     let storableRequest;
     while (storableRequest = await this._queueStore.getAndRemoveOldestEntry()) {
+      // Make a copy so the unmodified request can be stored]
+      // in the event of a replay failure.
+      const storableRequestClone = storableRequest.clone();
+
       // Ignore requests older than maxRetentionTime.
       const maxRetentionTimeInMs = this._maxRetentionTime * 60 * 1000;
       if (now - storableRequest.timestamp > maxRetentionTimeInMs) {
@@ -139,7 +143,7 @@ class Queue {
         replay.response = await fetch(replay.request.clone());
       } catch (err) {
         replay.error = err;
-        failedRequests.push(storableRequest);
+        failedRequests.push(storableRequestClone);
       }
 
       replayedRequests.push(replay);

packages/workbox-background-sync/models/StorableRequest.mjs

@@ -127,4 +127,25 @@ export default class StorableRequest {
   toRequest() {
     return new Request(this.url, this.requestInit);
   }
+
+  /**
+   * Creates and returns a deep clone of the instance.
+   *
+   * @return {StorableRequest}
+   *
+   * @private
+   */
+  clone() {
+    const requestInit = Object.assign({}, this.requestInit);
+    requestInit.headers = Object.assign({}, this.requestInit.headers);
+    if (this.requestInit.body) {
+      requestInit.body = this.requestInit.body.slice();
+    }
+
+    return new StorableRequest({
+      url: this.url,
+      timestamp: this.timestamp,
+      requestInit,
+    });
+  }
 }

packages/workbox-google-analytics/_default.mjs

@@ -62,21 +62,28 @@ const getTextFromBlob = async (blob) => {
  * @private
  */
 const createRequestWillReplayCallback = (config) => {
-  return async ({url, timestamp, requestInit}) => {
+  return async (storableRequest) => {
+    let {url, requestInit, timestamp} = storableRequest;
     url = new URL(url);
 
     // Measurement protocol requests can set their payload parameters in either
     // the URL query string (for GET requests) or the POST body.
     let params;
     if (requestInit.body) {
-      const payload = await getTextFromBlob(requestInit.body);
+      const payload = requestInit.body instanceof Blob ?
+          await getTextFromBlob(requestInit.body) : requestInit.body;
+
       params = new URLSearchParams(payload);
     } else {
       params = url.searchParams;
     }
 
-    // Set the qt param prior to apply the hitFilter or parameterOverrides.
-    const queueTime = Date.now() - timestamp;
+    // Calculate the qt param, accounting for the fact that an existing
+    // qt param may be present and should be updated rather than replaced.
+    const originalHitTime = timestamp - (params.get('qt') || 0);
+    const queueTime = Date.now() - originalHitTime;
+
+    // Set the qt param prior to applying the hitFilter or parameterOverrides.
     params.set('qt', queueTime);
 
     if (config.parameterOverrides) {
@@ -94,10 +101,10 @@ const createRequestWillReplayCallback = (config) => {
     requestInit.method = 'POST';
     requestInit.mode = 'cors';
     requestInit.credentials = 'omit';
-    requestInit.headers = '[["Content-Type", "text/plain"]]';
+    requestInit.headers = {'Content-Type': 'text/plain'};
 
-    // Ignore URL search params as they're in the post body.
-    requestInit.url = `${url.origin}${url.pathname}`;
+    // Ignore URL search params as they're now in the post body.
+    storableRequest.url = `${url.origin}${url.pathname}`;
   };
 };
 

test/workbox-background-sync/node/lib/test-StorableRequest.mjs

@@ -141,4 +141,32 @@ describe(`[workbox-background-sync] StorableRequest`, function() {
       expect(request.headers.get('x-qux')).to.equal('baz');
     });
   });
+
+  describe(`clone`, function() {
+    it(`creates a new instance with the same values`, async function() {
+      const original = new StorableRequest({
+        timestamp: 123456,
+        url: '/foo',
+        requestInit: {
+          body: new Blob(['it worked!']),
+          method: 'POST',
+          mode: 'no-cors',
+          headers: {
+            'x-foo': 'bar',
+            'x-qux': 'baz',
+          },
+        },
+      });
+      const clone = original.clone();
+
+      expect(original.url).to.equal(clone.url);
+      expect(original.timestamp).to.equal(clone.timestamp);
+      expect(original.requestInit).to.deep.equal(clone.requestInit);
+
+      // Ensure clone was not shallow.
+      expect(original.requestInit.body).to.not.equal(clone.requestInit.body);
+      expect(original.requestInit.headers).to.not.equal(
+          clone.requestInit.headers);
+    });
+  });
 });

test/workbox-background-sync/node/test-Queue.mjs

@@ -18,6 +18,8 @@ import {reset as iDBReset} from 'shelving-mock-indexeddb';
 import sinon from 'sinon';
 import expectError from '../../../infra/testing/expectError';
 import {Queue} from '../../../packages/workbox-background-sync/Queue.mjs';
+import {QueueStore} from
+    '../../../packages/workbox-background-sync/models/QueueStore.mjs';
 import {DB_NAME, OBJECT_STORE_NAME} from
     '../../../packages/workbox-background-sync/utils/constants.mjs';
 import {DBWrapper} from '../../../packages/workbox-core/_private/DBWrapper.mjs';
@@ -426,6 +428,35 @@ describe(`[workbox-background-sync] Queue`, function() {
         url: '/two?q=foo',
       }))).to.be.true;
     });
+
+    it(`should store the original request if a modified request replay fails`,
+        async function() {
+      sandbox.stub(self, 'fetch').rejects();
+      sandbox.spy(QueueStore.prototype, 'addEntry');
+
+      const requestWillReplay = (storableRequest) => {
+        storableRequest.url += '?q=foo';
+        storableRequest.requestInit.headers['x-foo'] = 'bar';
+      };
+
+      const queue = new Queue('foo', {
+        callbacks: {requestWillReplay},
+      });
+
+      await queue.addRequest(new Request('/one'));
+      await queue.addRequest(new Request('/two'));
+
+
+      await expectError(() => {
+        return queue.replayRequests();
+      }, 'queue-replay-failed');
+
+      // Ensure the re-enqueued requests are the same as the originals.
+      expect(QueueStore.prototype.addEntry.getCall(0).args).to.deep.equal(
+          QueueStore.prototype.addEntry.getCall(2).args);
+      expect(QueueStore.prototype.addEntry.getCall(1).args).to.deep.equal(
+          QueueStore.prototype.addEntry.getCall(3).args);
+    });
   });
 
   describe(`_registerSync()`, function() {

test/workbox-google-analytics/node/test-index.mjs

@@ -22,7 +22,6 @@ import {cacheNames} from '../../../packages/workbox-core/_private/cacheNames.mjs
 import {NetworkFirst, NetworkOnly} from '../../../packages/workbox-strategies/index.mjs';
 import * as googleAnalytics from '../../../packages/workbox-google-analytics/index.mjs';
 import {
-  MAX_RETENTION_TIME,
   GOOGLE_ANALYTICS_HOST,
   GTM_HOST,
   ANALYTICS_JS_PATH,
@@ -259,18 +258,22 @@ describe(`[workbox-google-analytics] initialize`, function() {
   it(`should add the qt param to replayed hits`, async function() {
     sandbox.stub(self, 'fetch').rejects();
     sandbox.spy(Queue.prototype, 'addRequest');
+    const clock = sandbox.useFakeTimers({toFake: ['Date']});
 
     googleAnalytics.initialize();
 
     self.dispatchEvent(new FetchEvent('fetch', {
       request: new Request(`https://${GOOGLE_ANALYTICS_HOST}` +
-          `/collect?${PAYLOAD}&`, {
+          `/collect?${PAYLOAD}`, {
         method: 'GET',
       }),
     }));
 
+    await eventsDoneWaiting();
+    clock.tick(100);
+
     self.dispatchEvent(new FetchEvent('fetch', {
-      request: new Request(`https://${GOOGLE_ANALYTICS_HOST}/collect`, {
+      request: new Request(`https://${GOOGLE_ANALYTICS_HOST}/r/collect`, {
         method: 'POST',
         body: PAYLOAD,
       }),
@@ -281,30 +284,77 @@ describe(`[workbox-google-analytics] initialize`, function() {
     self.fetch.restore();
     sandbox.stub(self, 'fetch').resolves(new Response('', {status: 200}));
 
-    const [queuePlugin] = Queue.prototype.addRequest.thisValues;
-    await queuePlugin.replayRequests();
+    const [queue] = Queue.prototype.addRequest.thisValues;
+
+    clock.tick(100);
+
+    await queue.replayRequests();
 
     expect(self.fetch.callCount).to.equal(2);
 
     const replay1 = self.fetch.firstCall.args[0];
     const replay2 = self.fetch.secondCall.args[0];
-
+    expect(replay1.url).to.equal(`https://${GOOGLE_ANALYTICS_HOST}/collect`);
+    expect(replay2.url).to.equal(`https://${GOOGLE_ANALYTICS_HOST}/r/collect`);
 
     const replayParams1 = new URLSearchParams(await replay1.text());
     const replayParams2 = new URLSearchParams(await replay2.text());
     const payloadParams = new URLSearchParams(PAYLOAD);
 
-    expect(parseInt(replayParams1.get('qt'))).to.be.above(0);
-    expect(parseInt(replayParams1.get('qt'))).to.be.below(MAX_RETENTION_TIME);
-    expect(parseInt(replayParams2.get('qt'))).to.be.above(0);
-    expect(parseInt(replayParams2.get('qt'))).to.be.below(MAX_RETENTION_TIME);
+    expect(parseInt(replayParams1.get('qt'))).to.equal(200);
+    expect(parseInt(replayParams2.get('qt'))).to.equal(100);
 
     for (const [key, value] of payloadParams.entries()) {
       expect(replayParams1.get(key)).to.equal(value);
       expect(replayParams2.get(key)).to.equal(value);
     }
   });
 
+  it(`should update an existing qt param`, async function() {
+    sandbox.stub(self, 'fetch').rejects();
+    sandbox.spy(Queue.prototype, 'addRequest');
+    const clock = sandbox.useFakeTimers({toFake: ['Date']});
+
+    googleAnalytics.initialize();
+
+    self.dispatchEvent(new FetchEvent('fetch', {
+      request: new Request(`https://${GOOGLE_ANALYTICS_HOST}` +
+          `/collect?${PAYLOAD}&qt=1000`, {
+        method: 'GET',
+      }),
+    }));
+
+    await eventsDoneWaiting();
+    clock.tick(100);
+
+    self.dispatchEvent(new FetchEvent('fetch', {
+      request: new Request(`https://${GOOGLE_ANALYTICS_HOST}/r/collect`, {
+        method: 'POST',
+        body: `${PAYLOAD}&qt=3000`,
+      }),
+    }));
+
+    await eventsDoneWaiting();
+
+    self.fetch.restore();
+    sandbox.stub(self, 'fetch').resolves(new Response('', {status: 200}));
+
+    const [queue] = Queue.prototype.addRequest.thisValues;
+
+    clock.tick(100);
+    await queue.replayRequests();
+
+    expect(self.fetch.callCount).to.equal(2);
+
+    const replay1 = self.fetch.firstCall.args[0];
+    const replay2 = self.fetch.secondCall.args[0];
+    const replayParams1 = new URLSearchParams(await replay1.text());
+    const replayParams2 = new URLSearchParams(await replay2.text());
+
+    expect(parseInt(replayParams1.get('qt'))).to.equal(1200);
+    expect(parseInt(replayParams2.get('qt'))).to.equal(3100);
+  });
+
   it(`should add parameterOverrides to replayed hits`, async function() {
     sandbox.stub(self, 'fetch').rejects();
     sandbox.spy(Queue.prototype, 'addRequest');