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

Should sendBeacon support GET? #22

Closed
toddreifsteck opened this issue Jan 8, 2016 · 14 comments
Closed

Should sendBeacon support GET? #22

toddreifsteck opened this issue Jan 8, 2016 · 14 comments
Assignees
Milestone

Comments

@toddreifsteck
Copy link
Member

A large number of existing RUM solutions depend on GET via XHR or the image tag. To ensure that sendBeacon can be used by these existing solutions, should we update it to support GET?

I think it is possible to add without significant change to the API surface if we mimic the Fetch WebIDL.

If thoughts are positive, I'll put together a pull request. Thoughts?

@yoavweiss
Copy link
Contributor

After asking around, I think this would be a most welcome addition, and is likely to help adoption.

👍

@igrigorik
Copy link
Member

Based on first-hand experience with a few large(r) clients, I do think it might be a reasonable addition:

  • Some services rely on processing pipelines that analyze requests logs only, and all the data they need is encoded directly in the URL itself (i.e. there are no payloads). I know a few services that have since allowed POST for such use cases as well, but they don't send a payload and still rely on encoding data in the URL's. These are not impossible hurdles, but they are unnecessary hurdles that have hurt adoption.
  • Some services invoke redirect chains, because they need the same beacon to be registered against multiple backends... And POST + 3xx is funny business. The UA converts POST to GET and the payload is dropped. This is correct behavior, but it's a gotcha for many. As such, technically we're already and unintentionally doing GET's in some cases..

My main question and concern here would be around defining a backwards compatible API, since we already have two implementations that default to POST, and I don't think we can change the default. Exposing GET as an optional method might be an option though.

/cc @annevk @sicking.

@sicking
Copy link

sicking commented Jan 13, 2016

And POST + 3xx is funny business. The UA converts POST to GET and the payload is dropped

That should only be the case for some 3xx values. IIRC a POST+307 should result in another POST. Is that not the case in Chrome?

Anyhow, I don't have a strong opinion here. This is a question for http-folks. In theory a GET should never result in side effects and just return a response which means that using sendBeacon which doesn't expose the response is kind of useless. But again, I'll defer to http-folks.

The only thing I'd say is that we discussed allowing fetch() to opt in to send-beacon like behavior where the request can be delayed. That might be a more generic route to go.

@annevk
Copy link
Member

annevk commented Jan 13, 2016

Indeed, 307 and 308 preserve the method and payload. Also, if you did do GET, you cannot have a payload.

@toddreifsteck
Copy link
Member Author

@sicking I think the theory and the reality are at odds with each other in this scenario, unfortunately. Many analytics systems currently use GET via xhr or image URL and the query string to send telemetry to the cloud. These systems usually ignore the response. The intent of adding GET support to sendBeacon is to allow these customers to move from xhr to sendBeacon for telemetry.

@toddreifsteck toddreifsteck self-assigned this Jan 20, 2016
@plehegar plehegar added this to the v1 milestone Feb 5, 2016
@cbentzel
Copy link

cbentzel commented Jun 8, 2016

There is some interest in GET beacon support for AMP analytics as well: ampproject/amphtml#2446 (comment)

@annevk
Copy link
Member

annevk commented Jun 8, 2016

I think I'm starting to lean somewhat in favor of trying to define sendBeacon() in terms of fetch() as @sicking mentions above. Keeping them around as too distinct primitives is just going to be confusing in the long run.

@igrigorik
Copy link
Member

Turns out, there is a good use case for HEAD as well...

It's common for navigation clicks to be bounced through several layers of redirects (as a means of registering the click) before arriving at the actual landing page. Each redirect in this chain only knows about the next hop; each redirect is owned and managed by a separate entity - i.e. there is no end-to-end visibility.

Now, imagine we want to use sendBeacon to move the redirect chain out of the navigation path: click triggers an async sendBeacon that follows the same redirect chain; browser navigates directly to the destination page. If sendBeacon is GET/POST then it will bounce through all of the redirects and then land and fetch the destination page... resulting in an unnecessary fetch of the HTML markup.

One obvious solution to this is to modify the chain to avoid the final redirect, but this turns out to be pretty hard in practice and is an adoption barrier for using sendBeacon.

Alternatively.. If sendBeacon allowed HEAD requests, then the request would still bounce through all the redirects and land on the destination page, but we would not fetch the HTML, which avoids the duplicate bytes. This make it deployable in existing ecosystem.

@igrigorik
Copy link
Member

Closing.. Solved via #27 + whatwg/fetch#388 (comment).

@emmettbutler
Copy link

Following the trail of issues and PRs a few years later, I'm looking for some clarification on the most up-to-date recommendation. Please see this StackOverflow question.

@igrigorik
Copy link
Member

A request has an associated keepalive flag...This can be used to allow the request to outlive the environment settings object, e.g., navigator.sendBeacon and the HTML img element set this flag

I'm surprised to see img mentioned there, nor do I see any plumbing in HTML spec for keepalive on images.. Surprised because I would not expect the UA to allow an image request to continue as the page unloads, whereas the whole point of sendBeacon and keepalive flag is to allow the request outlive the navigation context. @yoavweiss are you familiar with this particular corner of the Fetch+HTML image processing specs?

Ideally, we should track down the WPT tests for this behavior on images. If they don't exist (which would be my bet), then I wouldn't count or rely on this.

@yoavweiss
Copy link
Contributor

A request has an associated keepalive flag...This can be used to allow the request to outlive the environment settings object, e.g., navigator.sendBeacon and the HTML img element set this flag

I'm surprised to see img mentioned there, nor do I see any plumbing in HTML spec for keepalive on images.. Surprised because I would not expect the UA to allow an image request to continue as the page unloads, whereas the whole point of sendBeacon and keepalive flag is to allow the request outlive the navigation context. @yoavweiss are you familiar with this particular corner of the Fetch+HTML image processing specs?

Your surprise is well justified. That note is not correct, and images definitely do not outlive their documents. The keepalive flag is not defined for their fetch in the HTML spec, nor in implementations as far as I know.

It seems like the note was added by @annevk at this commit. @annevk - any recollection as to why this was added?

@annevk
Copy link
Member

annevk commented Mar 30, 2019

See whatwg/html#1655 and links from there.

@yoavweiss
Copy link
Contributor

Your surprise is well justified. That note is not correct, and images definitely do not outlive their documents. The keepalive flag is not defined for their fetch in the HTML spec, nor in implementations as far as I know.

Well, turns out the note is partially correct. Images are kept alive, but only when set inside page dismissal events.

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

8 participants