Use page lifecycle APIs instead of window unload event (discussion needed)
patrickshaughnessy opened this issue · 2 comments
I observed an issue using beaver logger where on navigation (redirect) to a new page, the beaver logger payload is fired, but reported as "Stalled" in Chrome network tab. The request did not complete and seems to not have made it to the server.
Digging a little deeper I came across the following:
- Beaver-logger optionally uses the sendBeacon API, but currently may be using the wrong events to detect when to send the beacon payload (
unload
/beforeunload
). - MDN docs call out some discussion about sendBeacon being potentially broken Comments section has an interesting discussion about whether it's a problem with
sendBeacon
or a with the actual vendor implementations not actually firing sendBeacon when they're supposed to. - This article suggests that we should not rely on unload / beforeunload events, especially in mobile browsers, and this can cause a ton of missing logs.
- Google's recommendation is to never use the
unload
event in modern browsers! - Google also provides a library for normalizing the page lifecycle API cross-browser
I'm still trying to digest all of these resources (and a little concerned about the dates on these articles), but wondering if it's worth considering using the page visibility API or the cross-browser Google library implementation of it anyways to detect when to automatically send the logs rather than the unload
and beforeunload
events.
- So a couple of things here. The
unload
andbeforeunload
events basically flush any logs that haven't been sent in the last 60 seconds. Basically, if the browser doesn't support one of these well, the other is a backup. Ifbeforeunload
handles flushing the logs, then there won't be any ifunload
is handled. Given that, I'm wondering ifenableSendBeacon
is true, if we shouldn't send immediately rather than wait for the flush interval or requireflush()
to be called to immediately flush the log. - We are aware of the following limitations of using
sendBeacon
, https://caniuse.com/?search=sendBeacon, and put the ownership on the user to enable this feature since it is off by default. - Great article! We should consider this moving forward and update this repo to use these lifecycle states as well.
- I think using the above states as well as
beforeunload
andunload
should ensure we catch it in one of them. - Interesting! I'll consult with Daniel when he gets back from vacation. I'm not sure about bringing in this library as I think we can observe all possible states ourselves. But haven't looked at it real closely.
Thanks for bringing this up! Our goal in providing the ability to use sendBeacon
was to eliminate loosing any logs and so this really helps us uphold this. Were you thinking about working on this and submitting a PR?
@mnicpt Thanks for the reply! I'm seeing pageVisibility API supported almost across the board: https://caniuse.com/pagevisibility so maybe the Google library is not necessary anymore.
Since I'm not super familiar with this repo or these browser APIs and quirks, I'd probably want to sync up to understand the scope and expected outcomes before picking up this work. But yes, happy to take this on and implement after that.