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

Supporting Headers in Pending Beacon #50

Closed
emensc52 opened this issue Oct 18, 2022 · 12 comments
Closed

Supporting Headers in Pending Beacon #50

emensc52 opened this issue Oct 18, 2022 · 12 comments
Labels
api Issue with API specs

Comments

@emensc52
Copy link

We would like to request header support in PendingBeacon.

@emensc52
Copy link
Author

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
Copy link
Collaborator

mingyc commented Oct 18, 2022

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:

  • Currently only supports Get/Post via PendingGetBeacon & PendingPostBeacon, how do they interact with setHeader() which may lead to different method?
  • How to ensure "Follow third-party cookie rules for beacons"
  • (and probably more)

cc @philipwalton @clelland WDYT?

@fergald
Copy link
Collaborator

fergald commented Oct 19, 2022

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
Copy link

annevk commented Oct 19, 2022

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
Copy link
Author

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
Copy link
Collaborator

fergald commented Oct 20, 2022

@annevk the motivation for PendingBeacon is

  • there is no reliable way to do a fetch as end-of-page-life
  • this API allows pending requests to be updated, reducing network usage

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.

@mingyc mingyc added the api Issue with API specs label Oct 20, 2022
@annevk
Copy link

annevk commented Oct 20, 2022

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
Copy link
Collaborator

fergald commented Oct 20, 2022

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
Copy link
Author

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 #52 as a very real option here if it becomes a possibility.

@fergald
Copy link
Collaborator

fergald commented Oct 21, 2022

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
Copy link

ricea commented Oct 21, 2022

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 added a commit to mingyc/pending-beacon that referenced this issue Oct 26, 2022
mingyc added a commit that referenced this issue Oct 26, 2022
mingyc added a commit to mingyc/pending-beacon that referenced this issue Feb 2, 2023
This section summarize the discussions from WICG#52 and WICG#50
mingyc added a commit that referenced this issue Feb 2, 2023
This section summarize the discussions from #52 and #50, which suggests that Fetch API doesn't fit the requirements for beaconing at page discard.
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 8, 2023
This summarize the discussion from
WebKit/standards-positions#85 (comment)
and also potentially unblocks the need to add custom headers in WICG#50
mingyc added a commit that referenced this issue Mar 8, 2023
* [Explainer] Add alternative approach to extend `sendBeacon`

This summarize the discussion from
WebKit/standards-positions#85 (comment)
and also potentially unblocks the need to add custom headers in #50
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 9, 2023
mingyc added a commit that referenced this issue Mar 9, 2023
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 24, 2023
- Previous discussions about fetch-based API are in WICG#70, WICG#52, WICG#50, WebKit/standards-positions#85 (comment)
- Related new discussions in WICG#71, WICG#72, WICG#73, WICG#74, WICG#75
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 24, 2023
- Previous discussions about fetch-based API are in WICG#70, WICG#52, WICG#50, WebKit/standards-positions#85 (comment)
- Related new discussions in WICG#71, WICG#72, WICG#73, WICG#74, WICG#75
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 24, 2023
- Previous discussions about fetch-based API are in WICG#70, WICG#52, WICG#50, WebKit/standards-positions#85 (comment)
- Related new discussions in WICG#71, WICG#72, WICG#73, WICG#74, WICG#75
mingyc added a commit to mingyc/pending-beacon that referenced this issue Mar 24, 2023
- Previous discussions about fetch-based API are in WICG#70, WICG#52, WICG#50, WebKit/standards-positions#85 (comment)
- Related new discussions in WICG#72, WICG#73, WICG#74, WICG#75, WICG#76, WICG#77
mingyc added a commit that referenced this issue Mar 24, 2023
- Previous discussions about fetch-based API are in #70, #52, #50, WebKit/standards-positions#85 (comment)
- Related new discussions in #72, #73, #74, #75, #76, #77
@mingyc
Copy link
Collaborator

mingyc commented Nov 13, 2023

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

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

@mingyc mingyc closed this as completed Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue with API specs
Projects
None yet
Development

No branches or pull requests

5 participants