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.

Sign up for GitHub

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

Jump to bottom

Consider reverting tracker to navigator.sendBeacon() #1470

Closed
guigrpa opened this issue Aug 29, 2022 · 10 comments
Closed

Consider reverting tracker to navigator.sendBeacon() #1470

guigrpa opened this issue Aug 29, 2022 · 10 comments

Comments

@guigrpa
Copy link
Contributor

guigrpa commented Aug 29, 2022

In #1160, it was proposed to switch from navigator.sendBeacon() in the tracker to fetch() with keepalive: true. The goal was to avoid some ad blockers from preventing analytics data to go through. This change was implemented in #1163.

I was thinking about the visit duration accuracy issues (#570, #1047) and have found some limitations with the fetch approach, especially if, as suggested by @gnarlex here, we want to send an event when a visitor leaves:

I'm not completely familiar with the analytics script. But couldn't we add a visibilitychange handler in the analytics script and use Navigator.sendBeacon() there to send information to umami once the user leaves a page?

Unfortunately, fetch() even with keepalive: true does not send events reliably on visibilitychange events. I've done some testing with the following code and include my results below:

document.addEventListener('visibilitychange', () => {
  const event_name =
    document.visibilityState === 'visible' ? 'VISIT_START' : 'VISIT_END';
  window.umami.trackEvent(event_name);
});
Case Chrome Firefox Safari Mobile Safari
Refresh ❌ ✅ ❌ ❌
Switch to ≠ (full-screen) app in same screen ✅ ✅ ✅ ✅
Switch to ≠ tab in same window ✅ ✅ ✅ ✅
Navigate away ❌ ❌ ❌ ❌
Close tab ❌ ❌ ❌ ❌
Close browser ❌ ❌ ❌ ❌

Note that fetch() only sends the event reliably when the web application is still alive. (Browser versions: Chrome v104, Firefox v103, Safari v15.3, Mobile Safari v15.6)

Switching (back) to navigator.sendBeacon() improves the results substantially:

document.addEventListener('visibilitychange', () => {
  const event_name =
    document.visibilityState === 'visible' ? 'VISIT_START' : 'VISIT_END';
  const queued = navigator.sendBeacon(
          'http://xx.xx.xx.xx:3000/api/collect',
          JSON.stringify({ type: 'event', payload: { /* ... */ event_name } })
        );});
Case Chrome Firefox Safari Mobile Safari
Refresh ✅ ✅ ✅ ✅
Switch to ≠ (full-screen) app in same screen ✅ ✅ ✅ ✅
Switch to ≠ tab in same window ✅ ✅ ✅ ✅
Navigate away ✅ ✅ ✅ ✅
Close tab ✅ ✅ ✅ ❌
Close browser ✅ ❌ ❌ ❌

This is much better: we now have post-mortem events in many cases. Based on this, I propose reverting to navigator.sendBeacon(). In order to avoid the ad blocking problems, we could add fetch as fallback, as @rohandebsarkar suggested in #1160.

What do you think?
@guigrpa guigrpa changed the title Reconsider tracker switch to fetch() instead of navigator.sendBeacon() Reconsider switching back tracker to navigator.sendBeacon() Aug 29, 2022
@guigrpa guigrpa changed the title Reconsider switching back tracker to navigator.sendBeacon() Reconsider reverting tracker to navigator.sendBeacon() Aug 29, 2022
@guigrpa guigrpa changed the title Reconsider reverting tracker to navigator.sendBeacon() Consider reverting tracker to navigator.sendBeacon() Aug 29, 2022
@Maxime-J
Copy link
Contributor

Hello, given your testing code,
trackEvent doesn't use the keepalive flag, so indeed, it doesn't give good results in these cases compared to navigator.sendBeacon()

Results would probably be on par if you test with fetch and keepalive

fetch('http://xx.xx.xx.xx:3000/api/collect', {
  method: 'POST',
  body: JSON.stringify({ type: 'event', payload: { /* ... */ event_name } }),
  headers: { 'Content-Type': 'application/json' },
  keepalive: true
})

Would be interesting to see the results 👍

@guigrpa
Copy link
Contributor Author

guigrpa commented Aug 29, 2022

Ah, from reading #1160 I was assuming that keepalive was being used. I will then add that case and post the results in this issue!

@guigrpa
Copy link
Contributor Author

guigrpa commented Aug 29, 2022

Here's a new summary, including fetch with/without keepalive and navigator.sendBeacon. With fetch() keepalive, the situation is similar to navigator.sendBeacon(), but we still don't receive VISIT_END in Firefox for some important cases (navigate away, close tab). Currently the best solution seems to be navigator.sendBeacon()…

Case Chrome Firefox Safari Mobile Safari
1. fetch() (no keepalive)
Refresh ❌ ✅ ❌ ❌
Switch to ≠ (full-screen) app in same screen ✅ ✅ ✅ ✅
Switch to ≠ tab in same window ✅ ✅ ✅ ✅
Navigate away ❌ ❌ ❌ ❌
Close tab ❌ ❌ ❌ ❌
Close browser ❌ ❌ ❌ ❌
2. fetch() (keepalive: true)
Refresh ✅ ✅ ✅ ✅
Switch to ≠ (full-screen) app in same screen ✅ ✅ ✅ ✅
Switch to ≠ tab in same window ✅ ✅ ✅ ✅
Navigate away ✅ ❌ ✅ ✅
Close tab ✅ ❌ ✅ ❌
Close browser ✅ ❌ ❌ ❌
3. navigator.sendBeacon()
Refresh ✅ ✅ ✅ ✅
Switch to ≠ (full-screen) app in same screen ✅ ✅ ✅ ✅
Switch to ≠ tab in same window ✅ ✅ ✅ ✅
Navigate away ✅ ✅ ✅ ✅
Close tab ✅ ✅ ✅ ❌
Close browser ✅ ❌ ❌ ❌

@guigrpa
Copy link
Contributor Author

guigrpa commented Aug 29, 2022

Btw, for the new case (2.) I used exactly what @Maxime-J suggested:

fetch('http://xx.xx.xx.xx:3000/api/collect', {
  method: 'POST',
  body: JSON.stringify({ type: 'event', payload: { /* ... */ event_name } }),
  headers: { 'Content-Type': 'application/json' },
  keepalive: true
})

@Maxime-J
Copy link
Contributor

Ah, from reading #1160 I was assuming that keepalive was being used.

And it was for a little while, not in trackEvent() though, only for events attached to links.
It was refactored recently and links handled differently.

But I follow you, tracker needs a way to send events in a persistant way.
Your tests will certainly help them to chose a method for these requests.

@rohandebsarkar
Copy link
Contributor

But this will still not improve the visit duration accuracy. This will only solve the persistence issue faced by the trackEvent() function when the visibility changes.

For improving the accuracy of the visit duration, we would need a way to remember when the user actually leaves the site, this can be done by doing some database changes. I proposed this on #570 (comment).

@guigrpa
Copy link
Contributor Author

guigrpa commented Aug 29, 2022

@rohandebsarkar Moving to sendBeacon is not sufficient, but it would be a step in that direction. Umami users could then store events on page leave and use data in their database to manually calculate their visit duration following their (arbitrary) definition.

@rohandebsarkar rohandebsarkar mentioned this issue Aug 29, 2022
Closed
@guigrpa
Copy link
Contributor Author

guigrpa commented Aug 29, 2022

@rohandebsarkar Regarding #1471: Are you also considering the navigator.sendBeacon() (with fetch fallback)?

@mikecao
Copy link
Collaborator

mikecao commented Aug 29, 2022

The tracker has been updated to no longer use fetch keepalive (doesn't work in Firefox) or navigator.sendBeacon (blocked by all adblockers). Update will be in the next release.

@mikecao
Copy link
Collaborator

mikecao commented Sep 6, 2022

Fixed in v1.38.0.

@mikecao mikecao closed this as completed Sep 6, 2022
@rehanvdm rehanvdm mentioned this issue Mar 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mikecao @guigrpa @Maxime-J @rohandebsarkar