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

Background sync fallback in Safari not working #2386

Closed
Adam-Reeves opened this issue Mar 2, 2020 · 12 comments
Closed

Background sync fallback in Safari not working #2386

Adam-Reeves opened this issue Mar 2, 2020 · 12 comments
Assignees
Labels
Browser Compatibility Tied to service worker behavior in a specific browser. workbox-background-sync

Comments

@Adam-Reeves
Copy link

Library Affected:
workbox-background-sync

Browser & Platform:
Safari 13

Issue or Feature Request Description:
The fallback for browsers that don't support background sync doesn't appear to be working as expected in a PWA I am working on.

When offline, the request is added to the queue and I can see this in IndexedDB. However no event other than installing a new service worker (a solution that's not workable in practice) will trigger the requests in the queue to be retried.

Looking in Queue.ts in workbox/packages/workbox-background-sync/src I can see on lines 413-415:

      // If the browser doesn't support background sync, retry
      // every time the service worker starts up as a fallback.
      this._onSync({queue: this});

this._onSync() only appears to be called the first time this._addSyncListener() is called but as it's not attached to an event listener it's never called again which would explain why it doesn't work as I expected.

As a workaround I could make use of the replayRequests() function but before doing so I would like to confirm whether or not this behaviour is expected.

Here's the service worker code (more or less a straight copy and paste from the Workbox docs):

  // Clone the request to ensure it's safe to read when
  // adding to the Queue.  
    const promiseChain = fetch(event.request.clone())
      .catch((err) => {
        self.clients.matchAll().then(all => all.map(client => client.postMessage("offline")));
  
        if(typeof BroadcastChannel !== "undefined") {
            const broadcastChannel = new BroadcastChannel('wb_channel');
            broadcastChannel.postMessage('offline');
        }
        return queue.pushRequest({ request: event.request });
      });
    event.waitUntil(promiseChain);
});
@jeffposnick
Copy link
Contributor

Here's my understanding; CC: @philipwalton to investigate further.

Inside of the constructor for the Queue class, there's a call to this._addSyncListener():

this._addSyncListener();

Inside of _addSyncListener(), there's a check for background sync support. In browsers like Safari, where there's no support, this._onSync() should end up being called unconditionally:

} else {
if (process.env.NODE_ENV !== 'production') {
logger.log(`Background sync replaying without background sync event`);
}
// If the browser doesn't support background sync, retry
// every time the service worker starts up as a fallback.
this._onSync({queue: this});
}

This constructor, and therefore the eventual this._onSync() call, should run once per instance of the Queue class in your service worker code, each time the service worker starts up. (The BackgroundSyncPlugin has an internal instance of the Queue class.)

@jeffposnick jeffposnick added Browser Compatibility Tied to service worker behavior in a specific browser. workbox-background-sync labels Mar 2, 2020
@Adam-Reeves
Copy link
Author

Hi Jeff,

Could you clarify what you mean when you say 'each time the service worker starts up' ? As in the 'install' phase of the lifecycle of a service worker?

The issue I'm having lies in the fact that this._onSync() is only called once per instance like you said, meaning the queued requests never get re-sent during the lifecycle of that particular service worker.

If this is expected functionality then that's fine as I can work around it. I thought it best to check first though :)

@jeffposnick
Copy link
Contributor

There's more info at https://stackoverflow.com/a/38835274/385997

In general, a service worker starts and stops pretty frequently—unless, for some reason, you were doing something in a client page that would keep it alive indefinitely, like making a network request every N seconds.

One caveat is that some browsers (Chrome definitely does; not sure about Safari) will keep a service worker running indefinitely when you have DevTools open. You can read more about that behavior at https://stackoverflow.com/a/42741722/385997

@Adam-Reeves
Copy link
Author

Thanks for the info.

Just to be on the safe side I have re-tested with DevTools closed in Safari.

It still doesn't look this._onSync() is getting called at any point after the service worker has been installed.

@nip10
Copy link

nip10 commented Mar 3, 2020

Hi.
Sorry for hijacking the issue, not sure whether to create a new one or add to this one.

I'm seeing a similar behavior when running a web app in Android WebView (Android v7, webview implementation: Chrome Stable).

I've done some debugging using DevTools (w/ Remote Debug) and I can see the requests being added to the queue (in IndexedDB), followed by a warning Unable to register sync event for 'myQueueName'. DOMException: Background Sync is disabled. which is expected since WebView doesn't support background sync. The problem is that the queue is never cleared/accessed.

I've tried waiting for long periods of time when reconnecting the device, restarting the browser, run with DevTools open/closed, and the requests just keep getting added to the queue.

Edit - More details: workbox v5.0.0; background sync plugin "basic" config; POST request; both client and server on the same domain using https. here's the sw code. Working in Chrome v80 (both Windows and Android), Firefox v68.5.0 (Android) and Firefox v73.0.1 (Windows). The weird thing is that Firefox works fine (even without the Background Sync API) showing Background sync replaying without background sync event

@philipwalton
Copy link
Member

@nip10 can you open a separate issue for this? I think there might be a way for Chrome (with BG sync disabled) to fall back to the behavior used in Firefox and Safari.

Right now we feature detect if ('sync' in self.registration) for the reply logic, but your experience shows that we should probably make it conditional on the registration succeeding (and using the fallback when it doesn't).

@Adam-Reeves
Copy link
Author

@philipwalton so will the fix of making the reply logic conditional on the registration succeeding fix the issues I'm having with Safari?

@jszczypk
Copy link

I am testing with Safari Desktop 13.1.2 and found out that fallback to call _onSync method to be called for sure only when browser starts. In my setup I have PouchDB replicating in the background all the time, so most likely Safari can't stop service worker as long as the page is opened.

My suggestion would be to provide a way to call currently private method Queue._onSync manually and even a way to access BackgroundSyncPlugin created Queue from outside. We could then check if Queue is full, show notification to user about stored requests and give them possibility to force synchronization manually.
Another possibility would be to somehow use window.online event to inform Queue that it should replay.

@WonderPanda
Copy link

WonderPanda commented Oct 25, 2021

Any chance there are some updates here that would allow us to interact with the queue manually in the case that background sync isn't supported as suggested by @jszczypk?

@jeffposnick
Copy link
Contributor

#2955 exposes the QueueStore and StorableRequest classes publicly, and #2941 exposes the size() of the QueueStore used by a Queue. Both of those PRs should be part of the upcoming Workbox v6.4.0 release.

I'm going to be honest in that I have never had to work with those pieces of workbox-background-syncmanually, so I'm not complete sure if it addresses the use case you detail here. @tropicadri has worked with this portion of the Workbox codebase more recently and might have some insight to share.

@patrickcorrigan
Copy link

patrickcorrigan commented Jul 12, 2023

I'm also experiencing the same issue @jszczypk. It seems to only be called when opening the browser. Did you end up solving the problem?

@tomayac
Copy link
Member

tomayac commented Apr 25, 2024

Hi there,

Workbox is moving to a new engineering team within Google. As part of this move, we're declaring a partial bug bankruptcy to allow the new team to start fresh. We realize this isn't optimal, but realistically, this is the only way we see it working. For transparency, here're the criteria we applied:

Thanks, and we hope for your understanding!
The Workbox team

@tomayac tomayac closed this as completed Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Browser Compatibility Tied to service worker behavior in a specific browser. workbox-background-sync
Projects
None yet
Development

No branches or pull requests

8 participants