WICG / pending-beacon

A better beaconing API
Other
43 stars 8 forks source link

Supporting Headers in Pending Beacon #50

Closed emensc52 closed 9 months ago

emensc52 commented 1 year ago

We would like to request header support in PendingBeacon.

emensc52 commented 1 year ago

More details: There is an effort to move view pings to the more reliable pending beacon. There is also a parallel effort to register these view pings with Chrome's privacy sandbox api. The privacy sandbox api requires 2 things to register an attribution src: the request containing the Attribution-Reporting-Eligible header, and a response containing the Attribution-Reporting-Register-Trigger with registration data. In order to tie these efforts together, we need to support headers in pending beacon. https://github.com/WICG/attribution-reporting-api/blob/main/EVENT.md

mingyc commented 1 year ago

What's currently supported in PendingBeacon inherits from navigator.sendBeacon(), as one of the primary goal is to make a more reliable version of the latter.

Digging into discussion around sendBeacon()

Aside from the above, to allow customizing headers, the shape of API also needs a big update:

cc @philipwalton @clelland WDYT?

fergald commented 1 year ago

I don't think we need to be concerned with Cookies. Adding support for cookies is a separate problem.

From a privacy perspective, it seems like there's not much difference between data going in a header and data going in the URL/body. If the page could have done a fetch with those headers, I don't see why it shouldn't send a beacon with them. It's just a question of whether there is a strong use case with no alternative. To be clear, I don't consider "use fetch+keepalive+headers" to be an alternative, if was we wouldn't need PendingBeacon.

@toddreifsteck @annevk @igrigoric who were very involved in previous discussions.

annevk commented 1 year ago

I need more details on why Fetch's keepalive is not good enough. That's the replacement for sendBeacon(). Adding another beacon API with the same limitations we'll eventually have to overcome seems like a step backwards.

emensc52 commented 1 year ago

Chrome will only recognize the registration in request headers: https://github.com/WICG/attribution-reporting-api/blob/main/EVENT.md#triggering-attribution. Why this API is designed this way and does not allow for data being in the payload or a URL parameter would be a question for Chrome's Attribution Reporting API, which we are trying to contact to understand this limitation.

Fetch's keepalive has some reliability issues. Research has shown that many pings are dropped due to its reliability and PendingBeacon is directly aimed at mitigating this.

fergald commented 1 year ago

@annevk the motivation for PendingBeacon is

The explainer has full details but I don't see a way to solve the fundamental problem with fetch+keepalive. If you think we shouldn't be pursuing this API at all, let's open a separate issue.

annevk commented 1 year ago

I guess I would prefer we attempt to extend Fetch's keepalive in some way over inventing a new way to do a fetch. It also seems that if keepalive is fundamentally broken in some way we should fix that so folks get more reliability.

fergald commented 1 year ago

This is not about keepalive being broken or not. There is no reliable way to send e.g. CLS-score when the page is destroyed. The only way to make it somewhat reliable is to over-send (e.g. send latest value every time it changes).

Please discuss augmenting window.fetch in #52.

emensc52 commented 1 year ago

Unfortunately the use case for our using PendingBeacon goes beyond just adding headers. Chrome's Attribution Reporting API parses the response in the renderer whereas PendingBeacon move the request/response onto the browser processer. So the renderer will never be able to read the response (not to mention the tab could be long closed at that point). I think Ming-Ying also mentioned that there is some discussion on potentially having an option where the beacon could be sent out before the navigation away happens? And potentially also being able to have the option to have requests/responses on the renderer?

However, even if these options don't come to fruition, we'd like to keep this request alive since both APIs are currently being developed and changes could happen. We'll also be looking at https://github.com/WICG/pending-beacon/issues/52 as a very real option here if it becomes a possibility.

fergald commented 1 year ago

Browser process vs renderer process are implementation details that the spec would not talk about however, beacon has been designed so that it can be outside the renderer process for browsers that have that concept. We cannot make network requests from the renderer reliable, especially on mobile where RAM is scarce and keeping a backgrounded renderer alive just for the sake of a keep-alive network request is not going to be an option.

I think a key point here is that your ask is for headers in the request (which seems reasonable to me) but also you want support for headers in the response and that is arguably not in keeping with the idea of beacons at all.

Beacons don't have a response and while we have made that clear in the API (there is just is no way to get the response), I think we should also make it clear in the spec. We should probably say that, on success, the whole response should be ignored, body and headers. No cookies should be updated, no cache entries should be updated, no headers with side-effects, like attribution reporting, should be processed.

I don't think we want a list of things that will/won't work with a beacon. "nothing at all" seems like the sanest option.

ricea commented 1 year ago

Beware of scope creep. Based on my experience with WebSocket, people will demand every feature the HTTP protocol can possibly support. Each feature added will have a significant long-term maintenance cost.

mingyc commented 9 months ago

The API is now repurposed as fetchLater() and supports almost the same args as fetch() does.

See the spec PR https://github.com/whatwg/fetch/pull/1647 and explainer https://github.com/WICG/pending-beacon/blob/main/docs/fetch-later-api.md