v3 bgSync Queue not queueing failed requests

[DavidScales]

Library Affected:
v3 workbox-sw

Browser & Platform:
Google Chrome v64.0.3

I'm trying to implement background sync on an API route, such that failed requests to the route are stored in a bgSync queue (IndexedDB), and resent later when online.

I originally tried using workbox.backgroundSync.Plugin, based on the v3 guide for Using Workbox Background Sync with other Workbox Packages. I had done the same thing previously in a v2 app, based on the v2 QueuePlugin documentation.

However, I learned in #1248 that v3's Plugin does not support replayRequest, which I would like to use**. So I am trying to use the Queue class, which does support replayRequest.

My Queue use looks like the following:

const bgSyncPlugin = new workbox.backgroundSync.Queue(
  'dashboardr-queue',
  {
    callbacks: {
      queueDidReplay: showNotification
    }
  }
);

const networkWithBackgroundSync = new workbox.strategies.NetworkOnly({
  plugins: [bgSyncPlugin],
});

const addEventRoute = new workbox.routing.Route(
  ({url}) => {
    console.log('api hit');
    return url.pathname === '/api.txt'
  },
  networkWithBackgroundSync,
);

workbox.routing.registerRoute(addEventRoute);

The the full app code here & hosted sample here.

Currently failed requests are not being queued, and Workbox is not throwing any errors, so I'm having trouble debugging. Any ideas?

** @gauntface I originally wanted replayRequests in Workbox codelabs because sometimes bgSync can be slow to occur, and I wanted a way for students to be able to test quickly. But I learned in #1249 that toggling the wifi might be sufficient to force bgSync, so I'm less confident in my particular use case, but I assume there are still other similar use cases.

[prateekbh]

So workbox.backgroundSync.Queue and workbox.backgroundSync.QueuePlugin are two different classes. workbox.backgroundSync.QueuePlugin has a method fetchDidFail which enque the request when it fails on network.

The best bet is to extend https://github.com/GoogleChrome/workbox/blob/v3/packages/workbox-background-sync/Plugin.mjs and add the custom replayRequests inside that.

Edit: You will have access to this._queue.replayRequests inside that.

[gauntface]

@prateekbh that isn't the approach we should be encouraging here.

@DavidScales I get your usecase, but IMO, this is actually more an issue with DevTools / setting things up and I don't want to add an API just because it can't be tested.

If you want - you can dispatch a fake sync event - forcing a replay of the requests OR you can use the Queue class directly, code off the top of my head would be something like:

const queue = new workbox.backgroundSync.Queue('myQueueName');

workbox.routing.registerRoute(
  ({url}) => {
    console.log('api hit');
    return url.pathname === '/api.txt'
  },
  () => {
    try {
      await fetch(request);
    } catch (err) {
      queue.addRequest(request);
    }
  }
);

// In message listener or something:
queue.replayReqiests();

The docs for background sync and pretty sketchy re-reading them, so I'll have another attempt at that.

[prateekbh]

makes sense, this one is pretty minimal and cleaner

[gauntface]

@DavidScales your code above is a mash up of the plugin and the lower level code, but the lower level code isn't a drop in replacement.

I.e. only the Plugin class can be used as a plugin, the Queue is designed so that you can you it on it's own (i.e. without all the other workbox modules).

I've updated the doc some more for plugin / advanced usage (Plugin / Queue) and I've added testing to doc as well so you should be able to use one of them in your codelab to do what you are looking for.

https://developers.google.com/web/tools/workbox/modules/workbox-background-sync

I would love any more feedback on all - and please say if it's still garbage and you want more from the docs

[DavidScales]

@gauntface thanks for all your help, I appreciate it and apologies if I'm dragging this out too much.

I think I have a better understanding of the differences between the Plugin & the Queue. The reason I was so keen to stick with Plugin, is because it really makes the most sense to me in this context to use Plugin, since I am trying to use bgSync "with other Workbox modules".

Going forward I think I am going to drop replayRequests (and stick with Plugin) and try to use a DevTools approach as you're suggesting.

To feel good about closing this issue though, ideally I'd like to know that its possible to use Queue to implement my original desired behavior.

I'm currently trying to use Queue like you've suggested:

const bgSyncQueue = new workbox.backgroundSync.Queue(
  'dashboardr-queue',
  {
    callbacks: {
      queueDidReplay: showNotification
    }
  }
);

const addEventRoute = new workbox.routing.Route(
  ({url}) => url.pathname === '/api/add',
  args => {
    fetch(args.url.href)
      .catch(err => {
        bgSyncQueue.addRequest(args.url.href);
      });
  },
  'POST'
);

But getting an error:

Uncaught (in promise) TypeError: request.clone is not a function
    at workbox-background-sync.dev.js:322
    at Generator.next (<anonymous>)
    at step (workbox-core.dev.js:14)
    at workbox-core.dev.js:32
    at new Promise (<anonymous>)
    at workbox-core.dev.js:11
    at Queue.addRequest (workbox-background-sync.dev.js:328)
    at fetch.catch.err (sw.js:105)
    at <anonymous>
(anonymous) | @ | workbox-background-sync.dev.js:322
  | step | @ | workbox-core.dev.js:14
  | (anonymous) | @ | workbox-core.dev.js:32
  | (anonymous) | @ | workbox-core.dev.js:11
  | addRequest | @ | workbox-background-sync.dev.js:328
  | fetch.catch.err | @ | sw.js:105
  | Promise.catch (async) |   |  
  | workbox.routing.Route.args | @ | sw.js:104
  | handleRequest | @ | workbox-routing.dev.js:365
  | workbox.routing.self.addEventListener.event | @ | workbox-routing.dev.js:817

Any ideas?

The docs are good, thanks for updating them! 😁 The testing section will be very helpful for codelabs.

[gauntface]

The error you are getting is because addRequest doesn't expect a URL string, expects a Request instance. Maybe args.event.request instead of args.url.href.

@prateekbh do you have any bandwidth to add typechecking to addRequest() to ensure it's a Request type and show an error message otherwise?

[prateekbh]

yep will do that

[DavidScales]

Ah ok. For posterity, changing to args.event.request throws:

Uncaught (in promise) TypeError: Failed to execute 'clone' on 'Request': Request body is already used
at workbox-background-sync.dev.js:322
    at Generator.next (<anonymous>)
    at step (workbox-core.dev.js:14)
    at workbox-core.dev.js:32
    at new Promise (<anonymous>)
    at workbox-core.dev.js:11
    at Queue.addRequest (workbox-background-sync.dev.js:328)
    at fetch.catch.err (sw.js:105)
    at <anonymous>

Because I forgot to clone the request:

const addEventRoute = new workbox.routing.Route(
  ({url}) => url.pathname === '/api/add',
  args => {
    console.log(args);
    fetch(args.event.request.clone())
      .catch(err => {
        bgSyncQueue.addRequest(args.event.request);
      });
  },
  'POST'
)

But yeah it works like magic. Thanks again guys! :D

This issue is good to close by me, but I'll leave it open if @prateekbh wants to use it to track his typechecking fix.

[gauntface]

Can we keep this open for the following:

• Change docs so we show off fetch event
• Add an assertion to input of addRequest()

[prateekbh]

Change demo so we show off fetch event

Do you want the demo to not use a fetch instead of a strategy?

[gauntface]

@prateekbh sorry - went auto-pilot - I meant change the docs to show off a fetch.