Bg sync replay fix

package.json

@@ -26,7 +26,7 @@
   },
   "devDependencies": {
     "babel-preset-babili": "0.0.12",
-    "chai": "^3.5.0",
+    "chai": "^4.0.2",
     "chromedriver": "^2.26.1",
     "clear-require": "^2.0.0",
     "cookie-parser": "^1.4.3",

packages/workbox-background-sync/src/lib/request-manager.js

@@ -16,7 +16,6 @@ class RequestManager {
    * attaches event handler
    * @param {Object=} config
    *
-   * @memberOf RequestManager
    * @private
    */
   constructor({callbacks, queue}) {
@@ -29,7 +28,6 @@ class RequestManager {
    * attaches sync handler to replay requests when
    * sync event is fired
    *
-   * @memberOf RequestManager
    * @private
    */
   attachSyncHandler() {
@@ -42,49 +40,67 @@ class RequestManager {
   }
 
   /**
-   * function to start playing requests
-   * in sequence
-   * @return {void}
+   * function to play one single request given its hash
+   *
+   * @param {String} hash
+   *
+   * @return {Promise} Resolves if the request corresponding to the hash is
+   * played successfully, rejects if it fails during the replay
+   *
    *
-   * @memberOf RequestManager
    * @private
    */
-  replayRequests() {
-    return this._queue.queue.reduce((promise, hash) => {
-      return promise
-        .then(async (item) => {
-          const reqData = await this._queue.getRequestFromQueue({hash});
-          if(reqData.response) {
-            // check if request is not played already
-            return;
-          }
-
-          const request = await getFetchableRequest({
-            idbRequestObject: reqData.request,
-          });
-
-          return fetch(request)
-            .then((response)=>{
-              if(!response.ok) {
-                return Promise.resolve();
-              } else {
-                // not blocking on putResponse.
-                putResponse({
-                  hash,
-                  idbObject: reqData,
-                  response: response.clone(),
-                  idbQDb: this._queue.idbQDb,
-                });
-                this._globalCallbacks.onResponse
-                  && this._globalCallbacks.onResponse(hash, response);
-              }
-            })
-            .catch((err)=>{
-              this._globalCallbacks.onRetryFailure
-                && this._globalCallbacks.onRetryFailure(hash, err);
-            });
+  async replayRequest(hash) {
+    try {
+      const reqData = await this._queue.getRequestFromQueue({hash});
+      if(reqData.response) {
+        return;
+      }
+      const request = await getFetchableRequest({
+        idbRequestObject: reqData.request,
+      });
+      const response = await fetch(request);
+      if(!response.ok) {
+        return Promise.reject(response);
+      } else {
+        // not blocking on putResponse.
+        putResponse({
+          hash,
+          idbObject: reqData,
+          response: response.clone(),
+          idbQDb: this._queue.idbQDb,
         });
-    }, Promise.resolve());
+        if (this._globalCallbacks.onResponse)
+          this._globalCallbacks.onResponse(hash, response);
+      }
+    } catch(err) {
+      return Promise.reject(err);
+    }
+  }
+
+  /**
+   * This function is to be called to replay all the requests
+   * in the current queue. It will play all the requests and return a promise
+   * based on the successfull execution of the requests.
+   *
+   * @return {Promise} Resolves if all requests are played successfully,
+   * rejects if any of the request fails during the replay
+   *
+   * @private
+   */
+  async replayRequests() {
+    const failedItems = [];
+    for (let hash of this._queue.queue) {
+      try {
+        await this.replayRequest(hash);
+      } catch (err) {
+        if(this._globalCallbacks.onRetryFailure)
+          this._globalCallbacks.onRetryFailure(hash, err);
+        failedItems.push(err);
+      }
+    }
+    return failedItems.length > 0 ?
+      Promise.reject(failedItems) : Promise.resolve();
   }
 }
 

packages/workbox-background-sync/src/lib/request-queue.js

@@ -140,7 +140,8 @@ class RequestQueue {
     assert.isType({hash}, 'string');
 
     if(this._queue.includes(hash)) {
-      return await this._idbQDb.get(hash);
+      const req = await this._idbQDb.get(hash);
+      return req;
     }
   }
 

packages/workbox-background-sync/test/browser/background-sync-queue-plugin.js

@@ -17,7 +17,6 @@
 'use strict';
 
 describe('background-sync-queue-plugin test', () => {
-  console.log(workbox.backgroundSync.test);
   const backgroundSyncQueue
     = new workbox.backgroundSync.test.BackgroundSyncQueuePlugin({});
 

packages/workbox-background-sync/test/browser/background-sync-queue.js

@@ -13,7 +13,6 @@
 
 /* eslint-env mocha, browser */
 /* global chai, workbox */
-
 'use strict';
 
 function delay(timeout) {
@@ -38,20 +37,33 @@ describe('background sync queue test', () => {
     onRetryFail: onRetryFail,
   };
 
-  const backgroundSyncQueue
-    = new workbox.backgroundSync.test.BackgroundSyncQueue({
+  let backgroundSyncQueue;
+
+  beforeEach(async function() {
+    responseAchieved = 0;
+    backgroundSyncQueue = new workbox.backgroundSync.test.BackgroundSyncQueue({
       maxRetentionTime: MAX_AGE,
-      queueName: QUEUE_NAME,
       callbacks: CALLBACKS,
     });
+  });
+
+  afterEach(async function() {
+    // replay queue so that further tests are not affected
+    try {
+      await backgroundSyncQueue.replayRequests();
+    } catch (e) {
+      // do nothing as this is just a cleanup exercise
+    }
+    await backgroundSyncQueue.cleanupQueue();
+  });
 
   it('check defaults', () => {
     const defaultsBackgroundSyncQueue
       = new workbox.backgroundSync.test.BackgroundSyncQueue({});
     chai.assert.isObject(defaultsBackgroundSyncQueue._queue);
     chai.assert.isObject(defaultsBackgroundSyncQueue._requestManager);
     chai.assert.equal(defaultsBackgroundSyncQueue._queue._queueName,
-      workbox.backgroundSync.test.Constants.defaultQueueName + '_0');
+      workbox.backgroundSync.test.Constants.defaultQueueName + '_1');
     chai.assert.equal(defaultsBackgroundSyncQueue._queue._config.maxAge,
       workbox.backgroundSync.test.Constants.maxAge);
     chai.assert.equal(
@@ -61,6 +73,11 @@ describe('background sync queue test', () => {
   });
 
   it('check parameterised constructor', () =>{
+    backgroundSyncQueue = new workbox.backgroundSync.test.BackgroundSyncQueue({
+      maxRetentionTime: MAX_AGE,
+      queueName: QUEUE_NAME,
+      callbacks: CALLBACKS,
+    });
     chai.assert.isObject(backgroundSyncQueue._queue);
     chai.assert.isObject(backgroundSyncQueue._requestManager);
     chai.assert.equal(backgroundSyncQueue._queue._queueName, QUEUE_NAME);
@@ -69,19 +86,30 @@ describe('background sync queue test', () => {
       CALLBACKS);
   });
 
-  it('check push proxy', async () => {
-    const currentLen = backgroundSyncQueue._queue.queue.length;
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('http://lipsum.com')});
-    chai.assert.equal(backgroundSyncQueue._queue.queue.length, currentLen + 1);
+  it('check push proxy', async function() {
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    chai.assert.equal(backgroundSyncQueue._queue.queue.length, 1);
   });
 
   it('check replay', async function() {
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('https://jsonplaceholder.typicode.com/posts/1')});
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('https://jsonplaceholder.typicode.com/posts/2')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    chai.assert.equal(backgroundSyncQueue._queue.queue.length, 2);
     await backgroundSyncQueue.replayRequests();
     chai.assert.equal(responseAchieved, 2);
   });
 
+  it('check replay failure with rejected promise', async function() {
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__test/404')});
+    try {
+      await backgroundSyncQueue.replayRequests();
+      throw new Error('Replay should have failed because of invalid URL');
+    } catch (err) {
+      chai.assert.equal(404, err[0].status);
+    }
+  });
+
   it('test queue cleanup', async () => {
     /* code for clearing everything from IDB */
     const backgroundSyncQueue
@@ -97,9 +125,9 @@ describe('background sync queue test', () => {
 
     await backgroundSyncQueue.cleanupQueue();
     await backgroundSyncQueue2.cleanupQueue();
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('http://lipsum1.com')});
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('http://lipsum2.com')});
-    await backgroundSyncQueue2.pushIntoQueue({request: new Request('http://lipsum.com')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    await backgroundSyncQueue2.pushIntoQueue({request: new Request('/__echo/counter')});
     const queue1Keys = (await backgroundSyncQueue._queue._idbQDb.getAllKeys());
     const queue2Keys = (await backgroundSyncQueue2._queue._idbQDb.getAllKeys());
     await delay(100);

packages/workbox-background-sync/test/browser/request-manager.js

@@ -15,7 +15,6 @@
 /* global chai, workbox */
 
 'use strict';
-
 describe('request-manager test', () => {
   let responseAchieved = 0;
   const callbacks = {
@@ -27,11 +26,15 @@ describe('request-manager test', () => {
   let queue;
   let reqManager;
 
+  const idbHelper = new workbox.backgroundSync.test.IdbHelper(
+    'bgQueueSyncDB', 1, 'QueueStore');
+
   before( (done) => {
     const QUEUE_NAME = 'QUEUE_NAME';
     const MAX_AGE = 6;
     queue =
       new workbox.backgroundSync.test.RequestQueue({
+        idbQDb: idbHelper,
         config: {maxAge: MAX_AGE},
         queueName: QUEUE_NAME,
       });
@@ -45,7 +48,9 @@ describe('request-manager test', () => {
   it('check constructor', () => {
     chai.assert.isObject(reqManager);
     chai.assert.isFunction(reqManager.attachSyncHandler);
+    chai.assert.isFunction(reqManager.replayRequest);
     chai.assert.isFunction(reqManager.replayRequests);
+
     chai.assert.equal(reqManager._globalCallbacks, callbacks);
     chai.assert.equal(reqManager._queue, queue);
   });
@@ -55,8 +60,8 @@ describe('request-manager test', () => {
       = new workbox.backgroundSync.test.BackgroundSyncQueue({
         callbacks,
       });
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('https://jsonplaceholder.typicode.com/posts/1')});
-    await backgroundSyncQueue.pushIntoQueue({request: new Request('https://jsonplaceholder.typicode.com/posts/2')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
+    await backgroundSyncQueue.pushIntoQueue({request: new Request('/__echo/counter')});
     await backgroundSyncQueue._requestManager.replayRequests();
     chai.assert.equal(responseAchieved, 2);
   });

packages/workbox-background-sync/test/browser/request-queue.js

@@ -19,8 +19,11 @@
 describe('request-queue tests', () => {
   const QUEUE_NAME = 'QUEUE_NAME';
   const MAX_AGE = 6;
+  const idbHelper = new workbox.backgroundSync.test.IdbHelper(
+    'bgQueueSyncDB', 1, 'QueueStore');
   let queue =
     new workbox.backgroundSync.test.RequestQueue({
+      idbQDb: idbHelper,
       config: {maxAge: MAX_AGE},
       queueName: QUEUE_NAME,
     });
@@ -51,8 +54,12 @@ describe('request-queue tests', () => {
   });
 
   it('default config is correct', () => {
-    let tempQueue = new workbox.backgroundSync.test.RequestQueue({});
-    let tempQueue2 = new workbox.backgroundSync.test.RequestQueue({});
+    let tempQueue = new workbox.backgroundSync.test.RequestQueue({
+      idbQDb: idbHelper,
+    });
+    let tempQueue2 = new workbox.backgroundSync.test.RequestQueue({
+      idbQDb: idbHelper,
+    });
     chai.assert.equal(tempQueue._config, undefined);
     chai.assert.equal(tempQueue._queueName,
       workbox.backgroundSync.test.Constants.defaultQueueName + '_0');