Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3 - trouble getting Background Sync to work #1222

Closed
DavidScales opened this issue Jan 23, 2018 · 8 comments
Closed

v3 - trouble getting Background Sync to work #1222

DavidScales opened this issue Jan 23, 2018 · 8 comments

Comments

@DavidScales
Copy link

Library Affected:
workbox-sw, workbox-backgroundSync

Issue or Feature Request Description:
I'm having some trouble setting up background sync with v3.

I started with the alpha 5 build & the code from the background sync guide:

// sw.js

importScripts('https://storage.googleapis.com/workbox-cdn/releases/3.0.0-alpha.5/workbox-sw.js');

const bgSyncPlugin = new workbox.backgroundSync.Plugin('dashboardr-queue');

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

const route = new Route({
  match: ({url}) => url.pathname === '/api/add',
  handler: networkFirstStrategy,
  method: 'POST',
});

workbox.routing.registerRoute(route);

On registration I get an error: Uncaught ReferenceError: Route is not defined.

So I tried switching to another signature that avoids the Route object:

const bgSyncPlugin = new workbox.backgroundSync.Plugin('dashboardr-queue');

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

workbox.routing.registerRoute(
  'http://localhost:8081/api/add',
  args => {
    console.log('API hit');
    return networkFirstStrategy.handle(args);
  },
  'POST'
);

but this is logging another error:

Uncaught (in promise) TypeError: Request method 'POST' is unsupported
    at Object.<anonymous> (workbox-core.dev.js:836)
    at Generator.next (<anonymous>)
    at step (workbox-core.dev.js:14)
    at workbox-core.dev.js:25
    at <anonymous>
  | (anonymous) | @ | workbox-core.dev.js:836
  | step | @ | workbox-core.dev.js:14
  | (anonymous) | @ | workbox-core.dev.js:25
  | Promise rejected (async) |   |  
  | (anonymous) | @ | workbox-strategies.dev.js:492
  | step | @ | workbox-core.dev.js:14
  | (anonymous) | @ | workbox-core.dev.js:25
  | Promise resolved (async) |   |  
  | step | @ | workbox-core.dev.js:24
  | (anonymous) | @ | workbox-core.dev.js:32
  | (anonymous) | @ | workbox-core.dev.js:11
  | _getNetworkPromise | @ | workbox-strategies.dev.js:496
  | (anonymous) | @ | workbox-strategies.dev.js:395
  | step | @ | workbox-core.dev.js:14
  | (anonymous) | @ | workbox-core.dev.js:32
  | (anonymous) | @ | workbox-core.dev.js:11
  | handle | @ | workbox-strategies.dev.js:410
  | workbox.routing.registerRoute.args | @ | sw.js:111
  | handleRequest | @ | workbox-routing.dev.js:365
  | workbox.routing.self.addEventListener.event | @ | workbox-routing.dev.js:817

On the hope of an unpublished patch I built the Workbox packages directly from source & tried using those instead of the CDN, but no luck.

I understand from previous issues in sw-precache that the Cache API doesn't support POST, and that the background sync plugin should be working around that by using IndexedDB. Any ideas?

Notes:

  • the bg sync queue is not being created in indexedDB
  • the POST requests are making it through to my server
@jeffposnick
Copy link
Contributor

Hello @DavidScales—Sorry that you're running into issues.

I believe your first issues, regarding the definition of Route, is because you're using the bare symbol Route without the workbox.routing namespace. So new workbox.routing.Route({...}) should work.

Regarding your second attempt, I believe that failure is due to NetworkFirst implicitly adding request/response pairs to the cache whenever there's a successful response received, in a "cache-as-you-go" fashion. But POST requests can't be used as keys in the Cache Storage API. So I think the right approach for that guide would be to instruct developers to use the NetworkOnly strategy, which avoids cache interaction. I believe that NetworkOnly should still trigger the events that the Background Sync plugin listens for when there's a failure.

It looks like https://developers.google.com/web/tools/workbox/modules/workbox-background-sync needs a couple of changes, so assuming you're able to get things working with those tweaks, we could repurpose this bug to track those changes.

@gauntface
Copy link

This issue tracks a better error message for the first use case: #1223

Incoming PR for docs fix.

@DavidScales
Copy link
Author

No worries, thanks for the explanation @jeffposnick, that make sense. Switching to NetworkFirst seems to have solved the cache error.

The updated code:

const bgSyncPlugin = new workbox.backgroundSync.Plugin('dashboardr-queue');

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

workbox.routing.registerRoute(
  'http://localhost:8081/api/add',
  args => {
    console.log('API hit');
    return networkOnlyStrategy.handle(args);
  },
  'POST'
);

does store the failed request in IndexedDB (and appears to be syncing once online) 👍

I was still having issues using new workbox.routing.Route but it looks like the documentation change fixed that. For posterity in this thread, its also required changing the single object argument in Route into three arguments:

const bgSyncPlugin = new workbox.backgroundSync.Plugin('dashboardr-queue');

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

const route = new workbox.routing.Route(
  ({url}) => url.pathname === '/api/add',
  networkOnlyStrategy,
  'POST'
);

workbox.routing.registerRoute(route);

@gauntface
Copy link

@DavidScales just to confirm you are happy with this being closed? Nothing remains to be done right?

@DavidScales
Copy link
Author

@gauntface yes, thanks!

@garviand
Copy link

garviand commented Feb 8, 2018

@jeffposnick What is the reasoning behind not supporting POST requests for the caching strategies? Forgive my ignorance, but it would seem that caching the response from POST requests would be a very common use case.

@jeffposnick
Copy link
Contributor

@garviand, that's part of the Cache Storage API specification. See 5.4.4, step 3.2:

For each request whose type is Request in requests:

  • Let r be request’s request.
  • If r’s url's scheme is not one of "http" and "https", or r’s method is not GET, return a promise rejected with a TypeError.

I believe the rationale is that POST and PUT requests are traditionally used to make non-reversable changes to server-controlled resources, and a scenario in which you bypass the server and respond with a previously cached response would not meet developers' expectations.

@garviand
Copy link

garviand commented Feb 8, 2018

Makes a lot of sense, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants