@tsteur opened this Pull Request on February 5th 2018 Owner

refs #6641

Anyone keen on doing some more testing with this?

Basically, what I experienced is, that we eg send a POST request on a form submit, then an unload happens and all pending requests are directly cancelled without hitting the Matomo tracking server, then we fall back to GET and those GET requests might fail because the URL is too long etc.

What would now happen here is when such a POST request is cancelled, and the unload was triggered, we would send them again as POST request which makes it more likely to succeed vs GET but this time using sendBeacon.

Also if any tracker plugin sends any tracking requests via unload plugin event, those tracking requests would be directly sent via sentBeacon as well vs before they likely did not finish on time before the next page was loaded.

@mattab commented on February 5th 2018 Owner

FYI Haven't tested the code but browser compatibility looks pretty good now:

compat1
mobile

After reading the doc of sendBeacon it looks like the ideal solution for what we're doing! this should resolve some of the web server errors (at least on apache)

@tsteur commented on February 8th 2018 Owner

@mattab I tested it again and also checked in the logs and it worked.

There is one thing that is a bit annoying but it always has been like this already: I noticed sometimes 2 POST requests. Eg a user submits a form, a POST request is sent, a few ms later the "unload" event occurs and the browser cancels these requests, then we retry them and send the second request. In the callback where we usually fallback to get request, we cannot know if the initial POST was already successfully transferred to the server or not (eg maybe request completed but response wasn't sent yet) and in the worst case this could cause the same data to be tracked twice. This affects all POST requests that are made shortly before unload. I wonder if we should always execute such requests with like a 50ms timeout for example?

In my tests this prevent this problem like this:

                if (isPageUnloading && sendPostRequestViaSendBeacon(request)) {
                    return;
                }

                setTimeout(function () {
                    if (isPageUnloading && sendPostRequestViaSendBeacon(request)) {
   // meanwhile maybe the unload event occured
                        return;
                    }
                    sendRequestTheRegularPostWay();
                }, 50);
@tsteur commented on February 8th 2018 Owner

FYI: I believe this timeout I just explained is the way to go and helpful... therefore pushed the change.

@mattab commented on February 9th 2018 Owner

Great find with the new setTimeout and 50 ms which will help track more accurate data 👍

Powered by GitHub Issue Mirror