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.

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

Support SendBeacon? #3

Open
AutoSponge opened this issue Aug 31, 2017 · 7 comments
Open

Support SendBeacon? #3

AutoSponge opened this issue Aug 31, 2017 · 7 comments

Comments

@AutoSponge
Copy link

Could this support sendBeacon by default instead of fetch?

@vesparny
Copy link
Owner

It's something I was thinking about.
Maybe using a polyfill like https://github.com/miguelmota/Navigator.sendBeacon would be great.

Do you want to send a pull request?

@joehand
Copy link
Collaborator

joehand commented Aug 31, 2017

Haha I should have looked here earlier! I added server side support. Right now chrome doesn't accept json, see issue.

I tested this on the client and it worked with the above PR (I will leave the PR here for others).

try {
  var json = JSON.stringify(opts)
} catch (e) {
  console.log(opts)
  return Promise.reject(new Error('could not strigify data'))
}
var blob = new window.Blob([ json ], { type: 'text/plain; charset=UTF-8' })
return window.navigator.sendBeacon(url, blob)

@joehand
Copy link
Collaborator

joehand commented Sep 1, 2017

FYI - In general, the json support for sendBeacon seems a bit broken and strangely implemented across browsers. See this issue for more discussion.

@AutoSponge
Copy link
Author

@vesparny @joehand Based on that info, I'm not sure I should create a PR. The complications of falling back to fetch when sendBeacon fails (possibly due to a higher version client than server) tells me it might be better to just wait for that Chrome/FF impl to settle down, first. Or, the abstraction should go 1-level deeper, to the transport lib (e.g., instead of isomorphic-unfetch some lib that negotiates sendBeacon with fetch and XHR fallbacks).

Feel free to close this.

@benwiley4000
Copy link

benwiley4000 commented Apr 1, 2019

I have something working for this using application/x-www-form-urlencoded instead of application/json (I also had to fork the fair-analytics server to support form encoded POST bodies, but it was a one-liner using bodyParser).

I haven't tested it across many browsers but it works for Chrome. I fall back to XMLHttpRequest (synchronous) if sendBeacon is unavailable. By default we use normal fetch and we only use sendBeacon/XHR if a guaranteeRequest flag is passed.

master...d252c03

@joehand's suggestion of just using text encoding seems more obvious though and maybe better than form encoding? Not sure. At the moment I'm running the code above for my client currently since I need to be able to send events when the page unloads.

P.S. Now realizing maybe some folks are sending hierarchical analytics data in which case form encoding wouldn't cut it, so JSON-as-plain-text is probably better for that.

P.P.S. Note that unlike what @AutoSponge originally proposed, this doesn't use sendBeacon unconditionally. I don't think that's a great idea generally since there's no way to know if a sendBeacon request failed after being queued. Some folks might prefer to retry analytics on the client side if the network fails, so sendBeacon is really best reserved for last-minute-before-page-unload requests, I think.

@benwiley4000
Copy link

Just now reviewing my code, I realized I forgot to add a promise return value for the fallback XHR case, but I can add that if we want a PR.

@benwiley4000
Copy link

I updated my fork to use text-as-json and return a promise for the xhr response. master...ea909c9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants