-
Notifications
You must be signed in to change notification settings - Fork 22.4k
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
Make the sendBeacon article match reality (including adding a best-practice example at least) #2692
Comments
This change adds data for the visibilitychange event — basically just by copying over the existing data for the onvisibilitychange event handler, but with Safari support marked as partial (due the fact that Safari apparently doesn’t fire the event as expected when visibilityState transitions to "hidden"). See also: https://bugs.webkit.org/show_bug.cgi?id=151234 (+ related bugs) Marginally related: https://github.com/mdn/sprints/issues/3722
Marginally related BCD PR: mdn/browser-compat-data#6763 |
This looks to be quite the authoritative reference: https://developers.google.com/web/updates/2018/07/page-lifecycle-api#event-focus Browser stack tests would be nice in a way, but will not tell you if real-world browsers are getting bogged down and drop the calls for performance reasons. Would be cool if a production service like Volument re-did their study with a couple different page visibility. It seems like the javascript on the docs page I linked to (written by Philip) might actually be the most reliable JS for most modern browsers |
Hey. Thanks for the constructive discussion around the topic. I'm happy to update the document with a snippet, that showcases the recommended use of sendBeacon. Can you hand it to me here directly? There seems to be a lot of pointers, but I'm having a hard time finding the exact code you are looking for. We are definitely going to make another test round with the given snippet once we move to production and have time for this again. We have a strong motivation since sendBeacon would significantly reduce our server load. Please note that the "visibilitychange" is not the event we want to use. It's different from "unload" and doesn't give clues that the session was ended. It's should be considered as a "hack" to overcome the unreliability of page unload event. Also, as far as I know, there is no evidence on "unload" event being broken on the browsers where it is supported. |
OK, thanks much @devinrhode2 — based on that feedback, I’ve made further updates to the MDN article such that in the place where we previously that example which used the
…and the See Also section now has this:
|
I don’t have a such a snippet handy. That’s in large part why I raised this issue. So I hope one concrete outcome from this issue is that we end up with a snippet we can add to the MDN So in the meantime, what I have done to try help developers is to make the additions I outlined in https://github.com/mdn/sprints/issues/3722#issuecomment-699283671.
At least for the general purpose of what to document for web developers in MDN articles — and as far as I understand so far from reading the https://developers.google.com/web/updates/2018/07/page-lifecycle-api article and @igrigorik’s comments for your blog post — But I could still be wrong about that. I’ll admit I still have a lot of learning to do myself about this area. However, I can also recognize that
As far as I have seen in the discussions so far, nobody has been claiming the
…and from https://www.igvita.com/2015/11/20/dont-lose-user-and-app-state-use-page-visibility/, the following:
|
My point is that if both the
|
I think the most important update to make to that page is to remove the use of If you want to show the old, bad way for contrast, then show both the old and the new way in the same code example, so it's clear that one is not recommended: // BAD! Do not use!
// The older, *extremely unreliable* way to send data to
// a server when a user leaves the page.
window.addEventListener('unload', function logData() {
var xhr = new XMLHttpRequest();
xhr.open('POST', '/log', false); // third parameter of `false` means synchronous
xhr.send(analyticsData);
});
// GOOD!
// The newer, more reliable way to send data to
// a server when a user leaves the page, including when a user:
// - Closes a tab
// - Switches tabs
// - Closes the browser app
// - Switches apps (on mobile)
// - Navigates away
// - Refreshes the page
document.addEventListener('visbilitychange', function logData() {
if (document.visbilityState === 'hidden') {
navigator.sendBeacon('/log', analyticsData);
}
}); I'd also recommend updating the warning text to recommend never using the On the Chrome team, we're trying to get the message out that using the unload event is never a good idea. For example, Lighthouse v6.2.0 has added a |
You're correct that The functional equivalent of the Here's one specific scenario:
In the above scenario, the This is not a bug, and it is not browser doing it wrong, it's the nature of how mobile platforms work. Sessions are not thought of in terms of distinct page loads, they're thought of in terms of in terms of foreground vs. background.
The In other words, if you're on page A and you click a link to page B, the Anyway, while yes, I do agree that the |
Is it guaranteed that the If all the above is true, then the documentation should absolutely recommend visibilitychange in favor of unload. And I would be really happy as a developer :) If we cannot detect the "unload" event in any meaningful way then the use-case of
I would probablhy use the more versatile and battle-tested |
There are always browser bugs (as with any API), but it is certainly the intention of the Page Visibility API that the Note: in terms of bugs, this issue tracks the list of bugs I'm aware of around
Yep :)
I think there's some (understandable) confusion here between the When a page is being unloaded, here are the events that are expected to fire:
Note: in the third case above, the The In short, if the browser is able to run event callbacks while a page is unloading then in such cases it's much better to be using the Sorry for the long explanation, hopefully that makes things more clear. |
Thanks for the explanation. It definitely sounds like I still think we should at least consider the fact that the |
From the current revision (emphasis mine):
I think recommending However, I think the current intro is emphasising this too much. It now sounds like There is much broader range of statistics and beacon use cases that developers may need to send beacons for. E.g. logging client-side errors, measuring feature-use (e.g. button click when opening a certain dialog), measuring success/failure paths (e.g. anonimised metadata associated with a user action, such as ajax-based saving of a comment on an issue tracker like this one), measuring performance timeline from the window-onload event, etc. For all these, it is imho the current best practice for developers to use Anecdotally – Some years ago at Wikipedia we briefly held a campaign to measure when attempts to contribute fail to complete. This was triggered on-click for an anchor link. Due to the termination of the browsing contexts upon page navigation, use of async XHR or image fetches was ineffective (this predated the Beacon API). Likewise, for things like error logging, these could happen at any time during the browsing session, even when there is no unload in sight. Yet, I would argue it is still best practice to use |
Along that same thread... I really feel like there's a better api that the
code inside of navigator.sendBeacon could be servicing. Maybe the fetch api
gets a new option for "backgroundable" or "critical" for `beacon: true`.
What's to stop web developers from using sendBeacon for everything? This
discussion seems to suggest that it would be more reliable. If that's not a
good idea, why? What caveats are there with sendBeacon, which should be
documented?
Also, it might be nice if sendBeacon returned a promise... but I perhaps a
promise gives developers the wrong idea. There's no *guarantee* a beacon
will finish (like if someone force-quits the browser, etc) but it's more
likely in the event of a user leaving a site. (please correct me if I'm
missing something)
…-Devin <http://devinrhode2.github.io>
On Mon, Sep 28, 2020 at 9:03 PM Timo Tijhof ***@***.***> wrote:
From the current revision
<https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon>
The *navigator.sendBeacon()* method asynchronously sends a small amount
of data over HTTP to a web server. It’s intended to be used in combination
with the visibilitychange event (but not with the unload and beforeunload
events).
I think recommending visibilitychange and strongly discouraging use of
unload is fine and would indeed help avoid developers from making this
common mistake.
However, I think the current intro is emphasises this too much. It now
sounds like sendBeacon() is exclusively designed for use with
visibility-related tracking.
There is much broader range of statistics and beacon use cases that
developers may need to send beacons for. E.g. logging client-side errors,
measuring feature-use (e.g. button click when opening a certain dialog),
measuring success/failure paths (e.g. anonimised metadata associated with a
user action, such as ajax-based saving of a comment on an issue tracker
like this one), measuring performance timeline from the window-onload
event, etc.
For all these, it is imho the current best practice for developers to use
sendBeacon() for all the same performance and reliability reasons that
make it appropiate for visibilitychange.
Anecdotally – Some years ago at Wikipedia we briefly held a campaign to
measure when certain attempts to contribute fail to compolete. This was
triggered on-click for an anchor link. Due to the termination of the
browsing contexts upon page navigation, use of async XHR or image fetches
was ineffective (this predated the Beacon API).
Likewise, for things like error logging, these could happen at any time
during the browsing session, even when there is no unload in sight. Yet, I
would argue it is still best practice to use sendBeacon() in that case so
that it reduces impact on the main thread, and because even if the event
being tracked isn't logically related to an upcoming unload, the user or
user agent may of course at any time close the tab, in which case you
wouldn't want those fetches to be aborted.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/mdn/sprints/issues/3722#issuecomment-700381580>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEDZKGXW7YEZVQWTOKXWLDSIE56ZANCNFSM4RZAHC7Q>
.
|
* Add visibilitychange event; Safari support partial This change adds data for the visibilitychange event — basically just by copying over the existing data for the onvisibilitychange event handler, but with Safari support marked as partial (due the fact that Safari apparently doesn’t fire the event as expected when visibilityState transitions to "hidden"). See also: https://bugs.webkit.org/show_bug.cgi?id=151234 (+ related bugs) Marginally related: https://github.com/mdn/sprints/issues/3722 * Mark onvisibilitychange & visibilityState partial This change marks support for the onvisibilitychange event handler and document.visibilityState property as partial_implementation:true for Safari (due the fact that Safari apparently doesn’t fire visibilitychange as expected when visibilityState transitions to "hidden").
@philipwalton I did a simple test with Chrome and Firefox where I closed a background tab and posted "beforeunload", "unload", and "visibilitychange" events to the opener window using It's pretty certain that we won't be using "visibilitychange" on our use case and I'm not sure it's the right event to recommend on the Beacon API documentation. |
@tipiirai please provide a repo, otherwise it's impossible to say what could be causing the failure you're seeing. I just tried creating a demo using Note, I didn't try using postMessage to inform the opening window, but I did try using
If there are bugs with |
Here are the steps to repeat the issue:
This is probably what you expected to happen since you mentioned earlier that none of the events can capture the unloading of the page when the tab is in the background. However, I realized that the unload and beforeunload are fired on the browsers I tested. edit: I'm using Chrome 86.0.4 on OSX. |
If the page was already in the background, then I wouldn't expect the If you want to specifically listen for the page being unloaded then you can listen for the |
Three things that I have trouble understanding on the visibilitychange recommendation:
|
As I mentioned above, you case use a combination of the
No, XHR is not reliable when the page is unloading. Since the
Yes, Chrome has now shipped bfcache, which means it will not fire the unload event when a user navigates away from a page—if that page is able to be cached. This behavior matches the current Safari behavior, which has been the way it is for several years. As for
|
Which implies that
We have similar results with sendBeacon on the study. Either due unload event not firing or sendBeacon call not able to reach the server. Hard for me to believe the I guess we can debate this on and on, but the conclusion will be that there is no way to reliably send information to the server prior to unloading of the document, which is the primary use case for sendBeacon. I don't seem to find any clear use cases for sendBeacon where it offers benefits over XHR. Would love to know one. |
@tipiirai I have found no evidence of sendBeacon being called but the request not happening or being aborted when a browser tab is closed or navigated away. As I understand it, the issue you found in your study bears no relation to When a tab is closed or navigated away, any on-going network request from subsources, hidden images, or XHR, are cancelled. This is not the case for sendBeacon. This means it is significanly more reliable for a wide range of use cases. This applies both to "close call" scenarios as in your story, but also improves innocuous scenarios that have nothing to do with the closing or navigating of tabs. Innocuous scenario – For example, when instrumenting something interactive with a web application (e.g. a modal dialog, or typeahead suggestions), any statistic or error logging beacon would could be lost if the user happens to close or navigate the page before these finish in the background. This can especially affect slower connections where a cellular radio may take a while to wake up before a new connection can be established. "Close call" scenarios:
Nothing is perfectly reliable, of course. On desktop one can force quit a browser process, disable WiFi, or pull out the battery or power cord. On mobile, there are also ways to quit applications. Where mobile browsers are different is that they popularized the idea of a "frozen" tab or even an entirely frozen/background browser application, which can then be closed from a tab switcher or app switcher without it waking up first. Thus no events fire. Under normal conditions, I have not been able to close a browser tab on desktop without any events firing during that interaction. On mobile this is fairly simple to do. This is where visibility and pagehide events come in. They are not a strict replacement for For visibilitychange on WebKit in particular, there was a bug that caused it to sometimes not fire, which Wikipedia currently works around by also listerning for |
Thanks for the detailed write-up! I guess the main issue is that there is no good replacement for the "unload" event, which is the primary use-case for sendBeacon. And without such an event one must resort to hacks and things get ugly very quickly. Duplicate events, missing events, and not knowing whether the page was unloaded or not. Ultimately the outcome is that you are not relying on the data you see on the server. You don't know whether it's the event that wasn't called (due to background tab for example) or was it the call that failed. That is: sendBeacon is designed for a use case that doesn't currently exist. Hope we get some "reliable-unload" event in the future. |
I find it weird that everyone here seems to read the results in such a way that sendBeacon is 100% reliable and the unload event is broken. If the failure rate is 100%, then it's easy to believe the culprit is the event and not beacon. However the data shows that the calls work mostly. Given the complexity of the call as opposed to firing just an event it just might be that the culprit is sendBeacon on some occasions. It's pretty clear that another study is needed before making such hasty conclusions. |
There is no replacement for the firing of an event at the exact moment that a user closes a tab or browser. If there is a spec or documentation page that suggested or promised otherwise, then someone should fix that! 🙂 I believe browsers decided years ago that it would be detremental to UX and incompatible with their interaction model, to allow apps to wake up their thread and run code. That's why unload isn't supported. The potential for abuse and tracking is also high. I think browsers have fundamentally declined this request for the Web, and is unlikely to change in the future.
I'm not sure this is true. sendBeacon is designed as alternative for XHR, fetch,
If your application is buffering information that you want to persist to a server, it is common to debouce and buffer this to save costs. That's great for performance. In the past people sometimes used Of course, since these events may fire multiple times, your code is responsible for clearing it buffer before or after each dispatch. Since JavaScript is single-threaded, though, this is fairly simple to do. E.g. making the array empty, or flip a boolean to indicate that the information was already sent.
I understand that with it sometimes working for you, it naturally casts equal suspicion on all technologies involved until they are individually ruled out as root cause. I guess what others and I are saying is that we have already exensively tested and understood these technologies for many years, and have a good intuition for what is and isn't likely to be relevant to a particular issue when observed in the wild. I don't deny the theoretical possibility, but I have trouble imagining that a browser might change the way its events are fired based on which code a developer has placed inside of an event handler. I encourage you to verify this for yourself on a simple page and play with it in different ways, you'll find your JavaScript doesn't run at all in the cases where the beacon appears to be lost. Even the "holy grail" of perfection for persisting data (the horrible synchronous XHR) or |
FYI: some excellent research related to the reliability of the various beaconing APIs and events: @tipiirai you might find this helpful ^ |
Issue moved to mdn/content mdn/sprints#1521 via ZenHub |
Thanks for the information guys. Pretty clear now, that our analytics service won't get much extra benefit from sendBeacon. |
Per discussion in the comments at https://volument.com/blog/sendbeacon-is-broken, we should update the https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon article to accurately reflect actual recommended best practices for using
sendBeacon
— which at least apparently definitely do not include using it with theonunload
event.@igrigorik, @tipiirai, @devinrhode2: I’m taking the liberty of pinging y’all on this in the hopes you can help us get the article refined into what it should be. But certainly feel free to unsubscribe yourselves if you lack time to weigh in on this here.
When I say “get that article refined into what it should be”, that necessarily may include adding more warnings for developers, given that https://volument.com/blog/sendbeacon-is-broken seems to indicate that developers at the very least must be very careful about how they use
sendBeacon
, and must have the right expectations about if/how it’ll work for their needs.But also at a minimum, I think the article should have an example that aligns with how
sendBeacon
is recommended to be used — which based on @igrigorik’s comments for @tipiirai’s blog post seems to clearly not include usingonunload
.So for starters, I’ve removed the example the article had previously — because that example used
onunload
— and I added an admonition at the top of the article about usingvisibilitychange
andpagehide
rather thanonunload
, as well as another another prominent Warning box in the body of the article, with a link to the blog post.One thing I’ve gleaned so far: using
pagehide
is maybe only a workaround for Safari due to limitations its current support? And the actual recommended long-term solution is: just usevisibilitychange
? If so, I guess the wording in the admonitions should be refined to reflect that, and any example we end up adding should also at least have comments to reflect that.Anyway, I am happy to put time myself into working on updating the article; but at this point, I have near-zero experience with actually using
sendBeacon
— which is why I pinged others for guidance and help.(Big thanks to @devinrhode2 for making the initial edit to the article which made me notice the problems with the article.)
The text was updated successfully, but these errors were encountered: