onPush not awaiting onBackgroundMessage event handler

[niyoko]

• Operating System version: ArchLinux 5.8.5
• Browser version: Chromium 85
• Firebase SDK version: 7.19.1
• Firebase Product: messaging

There's always additional notification when there is background push message coming, saying something like "The site has been updated in the background". I suspect the culprit is code below

      if (typeof this.bgMessageHandler === 'function') {
        this.bgMessageHandler(payload);
      } else {
        this.bgMessageHandler.next(payload);
      }

As you can see, this.bgMessageHandler is called without await, so when I return promise from onBackgroundMessage callback to display custom notification, it is not await-ed.

[zwu52]

Hi niyoko,
thanks for reaching out. Can you share your send curl or payloads?

I am guessing you are sending a FM data notification without a notification payload. For privacy/security reason the browser notifies the end user that some site is receiving background push and processing if no display notification is shown. Below are some solutions to this behavior:

1. Call ​serviceWorkerRegistration.showNotification in onBackgroundMessage hook. You can customize a display message that is less scary than the force-shown message.

2. Leave as it is. The "The site has been updated in the background" would appear once every 10 (IIRC) background messages.

3. Add a notification payload to your pushes. This might be a good practice in informing the user that your site is doing something in background.

But these should probably be documented in the API doc.
🍻

[niyoko]

Yes, it is data only payload.

Here is our payload format

{
  "to": "our_token",
  "data": {
    "eventName": "BusinessEvent\\Order\\Created",
    "actionUrlPlatform": "/relative/url",
    "title": "Title",
    "body": "Body",
    "notificationId": "123",
    "notificationRecipientId": "456"
  }
}

We already call showNotification from onBackgroundMessage and return the resulting Promise. The browser correctly show our notification but it also shows "The site has been updated..." notification. We are not passing notification payload because we want the notifications to merge when there are a lot of successive notifications coming in short timespan as described here.

messaging.onBackgroundMessage(function(payload) {
  var data = payload.data || {};
  var title = data.title || '';
  var options = {
    body: data.body || '',
    icon: 'https://domain/link/to/our/logo',
    badge: 'https://domain/link/to/our/logo',
    lang: 'id',
    data: { cta: data.actionUrlPlatform },
  };

  if (!data.eventName) {
    return self.registration.showNotification(title, options);
  } else {
    options.data.eventName = data.eventName;
    return self.registration
      .getNotifications()
      .then(function(notifications) {
        var currentNotification, i;
        for (i = 0; i < notifications.length; i++) {
          if (notifications[i].data && notifications[i].data.eventName === data.eventName)
            currentNotification = notifications[i];
        }

        return currentNotification;
      })
      .then(function(currentNotification) {
        var messageCount;
        if (currentNotification) {
          messageCount = (currentNotification.data.newMessageCount || 0) + 1;
          options.data.newMessageCount = messageCount;
          title = '[' + messageCount + '] ' + title;
          currentNotification.close();
        } else {
          options.data.newMessageCount = 1;
        }

        return registration.showNotification(title, options);
      });
  }
});

[zwu52]

niyoko,
thanks for sharing. I am able to reproduce. I think this is because the onBackgroundMessage is doing a "long-running" task and not being await as you suspected.

[zwu52]

@niyoko niyoko,
The solution to fix this might mean blocking briefly for each background message. I am doing some experiments and I found that looping though 1000 messages (arbitrary messages stored in notification box) and check for collapsing takes ~0.03s to complete and no silent push warning would be shown (on my device). I'd like to know more info on the devices that you seeing this issue:

• How often do you see the warning
• if possible, do you mind running some experiments on devices with the issue and get a delta t for running the onBackgroundMessage logic (I am using performance.now())

[tristan-morris]

Thanks for getting to the bottom of this issue @zwu52 and @niyoko. Not to ditto..

My code exhibits similar behaviour. On a Pixel 2 XL running as a Trusted Web App, the code below will throw the updated in background warning for maybe every second message throughout the day. It's more consistently shown when the phone hasn't been recently used, so I've assumed there's been more state to wake up/restore which takes longer to execute. As far as speed goes to execute the block of code, it's been pretty quick! But there's more going on.

This screenshot is from console on my Pixel 2 XL. Each "run" maybe two seconds apart. That 6.4 ms (I'm assuming the unit is actually ms) for the first execution, then followed by 0.70ms is solid difference.

Code within my service worker:

messaging.onBackgroundMessage(function(message) {
  console.log("onBackgroundMessage", message);

  let notificationOptions = {
    body: message.data.body,
    icon: message.data.icon,
    badge: message.data.badge,
    data: message.data
  };
  return self.registration.showNotification(
    message.data.title,
    notificationOptions
  );
});

(Removing the console.log dramatically improves the execution speed - down to 0.09ms and occasionally "0").

[zwu52]

Hi Triston,
thanks for taking the time to run experiments and sharing. Fixing PR was created and checked in. The solution was to create a timeout to allow showNotification async function to complete. Fix will be releasesd next week. Thanks for the input.

I am going to keep this thread open for a while before closing it. Feel free to post any question if any.

Edit: released. There will be an upcoming doc update. But make sure to call showNotification first in the onBackgroundMessage hook before any long running task.

[corysimmons]

What if you need to show a notification with data collected from a fetch?

[zwu52]

@corysimmons
thanks for reaching out. yes, that would be unsupported by the SDK ATM. I am not sure what is your exact use case, but one thing you could consider doing is to include whatever data in the data payload if it's applicable. Please file a feature request ticket If fetching data and display is an absolute need, we likely need a new API for that since the current one is non-async.

[corysimmons]

I ended up just passing notification data along with client.postMessage and then in the service worker triggering a notification to focus the tab in question onclick. Worked nicely.