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

Document the event handler created by workbox-core's clientsClaim() #2070

Closed
qqilihq opened this issue Jun 5, 2019 · 16 comments · Fixed by google/WebFundamentals#9316
Closed
Labels

Comments

@qqilihq
Copy link

qqilihq commented Jun 5, 2019

Library Affected:
workbox-sw, workbox-window

Browser & Platform:
all browsers

Issue or Feature Request Description:

I'm following the “Offer a page reload for users” from “Advanced Recipes”. The sample code suggest to listen to the WB controlling event and then perform the reload. This event is never triggered for me.

When I change it to activated it works.

Is this an error in the docs or am I doing something wrong?

When reporting bugs, please include relevant JavaScript Console logs and links to public URLs at which the issue could be reproduced.

service-worker.js:

// …
addEventListener('message', event => {
  if (event.data && event.data.type === 'SKIP_WAITING') {
    skipWaiting();
  }
});
// …

service-worker-registration.js:

const wb = new Workbox('/sw.js');
wb.addEventListener('waiting', event => {
  if (alert('Everything is new. Wanna see?')) {
    wb.addEventListener('controlling', event => {
      // will never be called
      window.location.reload();
    }
    wb.messageSW({ type: 'SKIP_WAITING' });
  }
});
wb.register();
@jeffposnick
Copy link
Contributor

"activated" represents the state of a given service worker. In practice, it effectively means that the service worker has completed whatever initial setup and cleanup steps that were configured in the install and activate handlers. When a service worker is "activated," it means that it's ready to start handling fetch events.

"controlling" represents the relationship between an activated service worker and a given client, like the window client representing your web app. When an activated service worker is "controlling" a given client, it means that all of that specific client's network requests will result in fetch events on the service worker.

So it's possible for a service worker to be "activated" (meaning that it could theoretically handle fetch events), but not be in control of any clients (meaning that it won't actually get a chance to handle fetch events).

Whether or not your newly activated service worker takes control of an existing window client depends on whether you're using workbox.core.clientsClaim() (which calls clients.claim() under the hood) or not. By default, a newly "activated" service worker will not take control over window clients that already existed prior to the activation.

This is explained in more detail at https://developers.google.com/web/fundamentals/primers/service-workers/lifecycle#clientsclaim

Hopefully that clears things up! I'm going to close this issue, but let me know if there is still confusion and we can revisit.

@qqilihq
Copy link
Author

qqilihq commented Jun 5, 2019

@jeffposnick Thanks for the feedback -- that definitely makes sense in theory, and I get that listening to controlling seems like the better solution.

Feeling a bit stupid now and I'm still not really sure what was f*** up, but I just had to restart Chrome and afterwards the controlling events get triggered properly. Hope it really was that easy 🙈

Thanks again!

@hannandesai
Copy link

@jeffposnick I face the same issue as faced by @qqilihq , in my case i am call periodically workbox update method to check for update, but in my case new service get activated but controlling event never gets fired.

Is there somethinng i am missing please guide me.

Thanks in Advance!

@jeffposnick
Copy link
Contributor

@hannandesai, if you could provide details about which version of workbox-window you're using, as well as a code snippet illustrating your usage, we can advise.

@jeffposnick jeffposnick reopened this Mar 16, 2021
@chrishoage
Copy link

@jeffposnick We are having very similar problems as hannandesai.

We are using workbox-window and workbox-webpack-plugin 6.1.1

interface WorkboxWorkerAction {
  type: keyof typeof serviceWorkerActions;
}

// Ensure to keep `createMockWorker` in sync with this when making changes
interface WorkboxWorker {
  register(): Promise<ServiceWorkerRegistration | undefined>;
  checkUpdate(): void;
  skipWaiting(): void;
  sendMessage(action: WorkboxWorkerAction): Promise<void>;
}

export default function createWorkboxWorker(): WorkboxWorker {
  const wb = new Workbox(path.join([__PUBLIC_PATH__, 'sw.js']));
  let isUncontrolled = true;

  wb.addEventListener('controlling', () => {
    window.location.reload();
  });

  wb.addEventListener('waiting', () => {
    adapters.resolve('appStore').dispatch(
      updateAvailable({
        isUpdateAvailable: true,
      })
    );
  });

  return {
    register() {
      return new Promise((resolve, reject) => {
        wb.register().then((registration) => {
          // After a "Hard Refresh" browsers will not attach the window to a
          // previously active Service Worker. When this happens we mark the
          // worker registration as uncontrolled and clear the media providers
          // so that when the user loads the app normally the next time they get
          // an up to date plex.tv/media/providers that isn't loaded from cache.
          // A strict `null` check here is intentional, since `controller` will
          // specifically have a `null` value, where if `serviceWorker` is not
          // supported or disabled it will return `undefined`.
          // See: https://developers.google.com/web/fundamentals/primers/service-workers/lifecycle#shift-reload
          isUncontrolled = window.navigator?.serviceWorker.controller === null;

          if (registration?.active && isUncontrolled) {
            log(
              '[Workers] Service Worker registered but is uncontrolled, possibly due to a hard refresh'
            );

            messageSW(registration.active, {
              type: serviceWorkerActions.CLEAR_CLOUD_MEDIA_PROVIDERS,
            });
          }

          resolve(registration);
        }, reject);
      });
    },

    checkUpdate() {
      log('[Workers] Checking for updates');
      wb.update();
    },

    skipWaiting() {
      log('[Workers] Installing new service worker');
      wb.messageSkipWaiting();
    },

    sendMessage(action: WorkboxWorkerAction) {
      return new Promise<void>((resolve, reject) => {
        // When the page does not claim the service worker wb.sendMessage will
        // never resolve nor reject. When we know we are in this state force a
        // rejection in the promise
        if (isUncontrolled) {
          reject();
        } else {
          wb.messageSW(action).then((response) => {
            if (response === serviceWorkerActions.DONE) {
              resolve();
            } else {
              reject();
            }
          });
        }
      });
    },
  };
}

In our serviceWorker.ts we have the following message handler

self.addEventListener('message', (event) => {
  if (!event || !event.data) {
    return;
  }

  switch (event.data.type) {
    case serviceWorkerActions.SIGN_OUT:
    case serviceWorkerActions.CLEAR_CLOUD_MEDIA_PROVIDERS:
      mediaProvidersCachePlugin.deleteCacheAndMetadata().then(
        () => {
          event.ports[0].postMessage(serviceWorkerActions.DONE);
        },
        () => {
          event.ports[0].postMessage(serviceWorkerActions.FAIL);
        }
      );
      break;

    case serviceWorkerActions.SKIP_WAITING:
      self.skipWaiting();
      core.clientsClaim();
      break;
  }
});

We have code that creates a singleton from createWorkboxWorker and registers the service worker during our app initialization.

We then have a component that calls checkUpdate from the singleton every hour, to prompt the user about an update.

After calling update we observe the waiting service worker, however it appears as if the service worker skips waiting on its own, and installs, and never receives a controlling to reload the page.

If a user manually reloads the page, the service worker is installed as expected.

If a user loads the app and an update is available, the modal displays as expected and pressing "Update" correctly fires the controlling event, triggering the reload and the app update.

I have read though https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerRegistration/update along with the workbox documentation, but have not been able to figure out this issue.

It is unclear if we're holding wb.update() wrong, or we have some other error in our implantation.

Thank you for any guidance and time you can provide to help with this problem!

@hannandesai
Copy link

hannandesai commented Mar 17, 2021

@jeffposnick Thanks for the reply.
I am using worbox-window 6.1.2 & workbox-cli 6.1.2

Below is my code for service worker registration using workbox-window


function registerServiceWorker() {
  if (environment.production && 'serviceWorker' in navigator) {
    let serviceWorker;
    let interval;
    const wb = new Workbox('sw-main.js');

    const registerSWUpdateInterval = () => {
      console.log("called");
      wb.update().then(function () {
        setTimeout(() => { registerSWUpdateInterval(); }, 1000);
      }, (error) => {
        setTimeout(() => { registerSWUpdateInterval(); }, 1000);
      });
    }

    wb.addEventListener('activated', (event) => {
      if (!event.isUpdate) {
        console.info('Service worker activated for the first time!');
        messageSW(event.sw, { type: "CLIENTS_CLAIM" });
      } else {
        console.info('Service worker activated!');
      }
    });

    // Add an event listener to detect when the registered
    // service worker has installed but is waiting to activate.
    wb.addEventListener('waiting', (event) => {
      console.info(`A new service worker has installed, but it can't activate` +
        `until all tabs running the current version have fully unloaded.`);
      if (serviceWorker && serviceWorker.waiting) {
        serviceWorker.waiting.addEventListener("controlling", (event) => {
          console.log("controlling")
          messageSW(event.sw, { type: "APP_VERSION_UPDATE" });
        })
        wb.messageSkipWaiting();
      }
    });

    wb.addEventListener('installed', (event) => {
      if (!event.isUpdate) {
        console.info('Service worker installed for the first time');
      } else {
        console.info('Service worker installed');
      }
    });

    wb.register().then((sw) => {
      serviceWorker = sw;
      registerSWUpdateInterval();
    });
  } else {
    console.log("Service worker feature is not available/supported.");
  }
}


Below is my service worker file

/// <reference lib="es2018" />
/// <reference lib="webworker" />
// External Imports
import { precacheAndRoute, getCacheKeyForURL } from "workbox-precaching";
import { clientsClaim } from 'workbox-core';
import { registerRoute } from "workbox-routing";
import { cacheNames } from 'workbox-core';
const path = require("path");

// Internal Imports

declare const self: ServiceWorkerGlobalScope;

// self.skipWaiting();
// clientsClaim();

precacheAndRoute(self.__WB_MANIFEST);

const appMatchUrlCB = ({ url, request, event }) => {
    return (url.origin.indexOf("http://localhost:4200") >= 0 && path.extname(url.pathname).length <= 0);
}

const appHandlerCB = async ({ url, request, event, params }) => {
    const cache = await caches.open(cacheNames.precache);
    const response = await cache.match(
        getCacheKeyForURL('/index.html')
    );
    return response;
}

const sendMessageToClient = (command, data?) => {
    self.clients.matchAll().then((clients) => {
        clients.forEach((client) => {
            client.postMessage({ 'command': command, 'data': data });
        })
    });
}

self.addEventListener('message', (event) => {
    console.log(event);
    if (event.data && event.data.type === 'SKIP_WAITING') {
        self.skipWaiting();
    } else if (event.data && event.data.type === "APP_VERSION_UPDATE") {
        sendMessageToClient(event.data.type);
    } else if (event.data && event.data.type === "CLIENTS_CLAIM") {
        // here i tried both client claim method as below none of them is calling controlling event.
        // self.clients.claim();
        clientsClaim();
    }
})


registerRoute(appMatchUrlCB, appHandlerCB);

Here i check for uodates manually after every one minute.
I tried clientsClaim both at global level and after activate but none of them is triggering controlling event.

@jeffposnick jeffposnick changed the title Page reload confusions about controlling vs. activated event Document the event handler created by workbox-core's clientsClaim() Mar 17, 2021
@jeffposnick
Copy link
Contributor

Hello @chrishoage and @hannandesai! Thanks for sharing that. I'll provide specific debugging feedback on each implementation.

Before I do that, I just wanted to clarify what workbox-core's clientsClaim() does, with an eye towards getting this information into our official documentation.

Prior to Workbox v6, workbox-core exposed two helper methods that wrapped underlying service worker API calls: self.skipWaiting() and self.clients.claim(). Both of these wrapped methods would register an appropriate event handler (install for skipWaiting(), and activate for clientsClaim()) and then add a call to the underlying method inside of that handler.

This behavior—registering and event handler which called the underlying method in response to the relevant event—ended up confusing a number of developers, and we didn't do a good job of explaining it in our documentation. Specifically, developers who called workbox-core's wrappers inside of another event handler, like message, will likely not get the behavior they expect, because they'll end up with a new install or activate handler registered, and there's a chance that either installation or activation has already completed at that point.

In the case of the skipWaiting() wrapper, there is actually no need to call self.skipWaiting() inside of an install event. self.skipWaiting() can be "safely" called at any point in time within the service worker's lifecycle. (It may or may not have any effect, depending on whether the service worker in question would have delayed activation, but the worse case scenario is that it's just a no-op.) In order to avoid developer confusion, we effectively deprecated the workbox-core wrapped in v6; calling it now will log a message telling you to switch to self.skipWaiting() instead, and it no longer registers the install handler:

function skipWaiting() {
// Just call self.skipWaiting() directly.
// See https://github.com/GoogleChrome/workbox/issues/2525
if (process.env.NODE_ENV !== 'production') {
logger.warn(`skipWaiting() from workbox-core is no longer recommended ` +
`and will be removed in Workbox v7. Using self.skipWaiting() instead ` +
`is equivalent.`);
}
self.skipWaiting();
}

The clientsClaim() wrapper is more complicated, though, because it actually does have value—calling the underlying self.clients.claim() method from inside of a service worker that is not active will result in an InvalidStateError exception. So just unconditionally putting self.clients.claim() at the top-level of your service worker, outside of an event handler, will cause your service worker to throw an exception before it ever gets a chance to install. For that reason, the workbox-core clientsClaim() method still registers an activate event handler in Workbox v6:

function clientsClaim() {
self.addEventListener('activate', () => self.clients.claim());
}

(Again, apologies that this nuance is currently unclear from reading the docs.)

This is all to say that if you're using workbox-core's clientsClaim(), you almost certainly want to put it at the top-level execution context of your service worker, outside of any other event handlers, so that it can register it's own activate handler immediately. If you need to only conditionally claim clients upon activation, and sometimes you don't want to claim clients (which does not seem like a common use case, but still...), then you can call self.clients.claim() directly, instead of calling the wrapper, inside of, e.g., a message event. If you do that, though, it's your responsibility to ensure that the current service worker is active before you call it, or else handle the possible exception that might be thrown.

Long-term, maybe we should remove the workbox-core clientsClaim() wrapper as well, but in the meantime, I will follow-up with better documentation.

@chrishoage
Copy link

chrishoage commented Mar 18, 2021

@jeffposnick Thank you for the detailed write up. In our case, we only added core.clientsClaim() very very recently in an attempt to solve the problem at hand.

We noticed there were a few places in the docs that clientsClaim was called after skipWaiting and thought it couldn't hurt to try to see if it helped with the controlling event not firing after using update() (Obviously this didn't work)

You can safely ignore that line in my example above, because the behavior we are experiencing happens with or with out it.

@hannandesai
Copy link

@jeffposnick Thanks you for detailed explanation this help me understands clientsClaim() wrapper clearly.

But as i mentioned i tried clientsClaim() wrapper at top level also but still it wont trigger the controlling event, and just to inform controlling event is called when i manually reload the page but when we try to update service worker using workbox update method it wont get called.

@jeffposnick
Copy link
Contributor

Yup, understood. I will dive into both code samples soon. I just know that clientsClaim() can be confusing to use and wanted to clarify that off the bat.

@jeffposnick
Copy link
Contributor

jeffposnick commented Mar 18, 2021

@hannandesai: Just a few random comments:

I had some issues getting your code to compile as-is, because you're doing things like const path = require("path"); inside of a service worker, which won't work. I'm assuming you have something custom in your build process that somehow translates that into code that will run inside of the ServiceWorkerGlobalScope, but I'm not sure what it is. So I pared down your sample code a bit instead of running it directly.

But in any case, I see some problems with the following code:

    wb.addEventListener('waiting', (event) => {
      console.info(`A new service worker has installed, but it can't activate` +
        `until all tabs running the current version have fully unloaded.`);
      if (serviceWorker && serviceWorker.waiting) {
        serviceWorker.waiting.addEventListener("controlling", (event) => {
          console.log("controlling")
          messageSW(event.sw, { type: "APP_VERSION_UPDATE" });
        })
        wb.messageSkipWaiting();
      }
    });

In this case, serviceWorker.waiting is an instance of ServiceWorker, not of Workbox. There's no event called controlling that is ever fired on a ServiceWorker, so listening for that won't accomplish anything.

A Workbox object can listen for controlling events, though (it's one of the "synthetic" events that workbox-window dispatches), so that's what I assume you intended to do.

In the course of exploring this issue, though, I realized that the Workbox object's controlling event isn't dispatched if the new service worker that takes control isn't the same as the service worker that originated from the register() method. That's a bug—we should fire controlling in either case, with the isExternal property set to true or false—and I created #2786 to track that.

So, once #2768 is resolved, you should be able to write:

const wb = new Workbox('sw.js');

// Set up your event listeners for waiting, conditionally call messageSkipWaiting(), etc.

// Listen for changes to navigator.serviceWorker.controller:
wb.addEventListener('controlling', (event) => {
  if (event.isExternal) {
    // A new service worker is in control that wasn't associated with the original registration.
    // This will happen if the new service worker was detected via wb.update().
  } else {
    // The service worker associated with the original registration is in control.
  }
});

wb.register();

// Don't use an interval of 1000 in production, obviously...
setInterval(() => wb.update(), 1000);

In the meantime, instead of using wb.addEventListener('controlling', () => {...}), you could just use the underlying service worker API directly to listen for the controllerchange event conditionally, only if there's already a service worker in control (to avoid firing it when it's the initial service worker):

if (navigator.serviceWorker.controller) {
  navigator.serviceWorker.addEventListener('controllerchange', (event) => {
    // If this executes, there was an initial service worker in control during the page load,
    // and now a service worker has taken control.
  });
}

The rest of the general logic could remain the same in that case.

@hannandesai
Copy link

@jeffposnick yup, sorry as i forgot to correct the code, that listening controlling event for serviceWorker.wating is just to give it a try, otherwise i am using as you mentioned.

Will definitely looking forward for #2786 to resolve.

Thanks in Advance

@jeffposnick
Copy link
Contributor

@hannandesai, just to reiterate, if you need this functionality you don't have to wait for #2786. You can add

navigator.serviceWorker.addEventListener('controllerchange', () => {...});

to your code right now if you want to be notified of the controller changing all the time. If you only want to be notified when the controller changed after being previously being set to a different service worker, add the event listener conditionally:

if (navigator.serviceWorker.controller) {
  navigator.serviceWorker.addEventListener('controllerchange', () => {...});
}

That should be functionally identical to what the Workbox object would expose via its controlling event.

@jeffposnick
Copy link
Contributor

@chrishoage, looking at your workbox-window example now, I think it's directly related to #2786, in that

  wb.addEventListener('controlling', () => {
    window.location.reload();
  });

won't currently fire if the service worker that takes control was installed and activated due to a wb.update() check. Apologies for that.

The same workaround mentioned above should apply, where you listen to the controllerchange event that's already part of the service worker API.

@chrishoage
Copy link

@jeffposnick Thank you for your help in diagnosing the problem. Good to know we were not holding it wrong.

We will wait for the PR linked in the GHI to land.

@jeffposnick
Copy link
Contributor

(The documentation changes, which this issue tracks, have already landed.)

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

Successfully merging a pull request may close this issue.

4 participants