w3c / beacon

Beacon
https://w3c.github.io/beacon/
Other
46 stars 22 forks source link

Enforce payload limits via Fetch API #39

Closed igrigorik closed 7 years ago

igrigorik commented 7 years ago

Beacon requests are allowed to continue while/if a fetch group is being terminated - e.g. as the page unloads. To ensure that such requests do not negatively affect the consequent navigation, or incur high costs for the user, we limit the amount of data that can be queued via this method.

The enforcement is applied within Fetch API, which tracks the amount of in-flight data queued with the keepalive flag set, and returns a network error if the limit is exceeded.

Background: https://github.com/w3c/beacon/issues/38


^ that's the end goal, ideally... as that would lead to consistent behavior between sendBeacon and plain fetch() with keepalive flag set to true. This also means that size enforcement has to live in Fetch API, as otherwise we'd end up with two different byte counters.

The challenge, however, is how to get an early return code for sendBeacon? Unlike fetch, which returns a promise we need to return true/false back to the developer: if we didn't trigger a network error before request is queued, return true; otherwise return false. @annevk any suggestions for best way to layer this?

annevk commented 7 years ago

That's the same question as asked during that PR. Can implementations actually synchronously get that size data? E.g., I don't think all can for FormData.

igrigorik commented 7 years ago

@annevk the fact that we have three existing implementations of sendBeacon (FF, Edge, Chrome) suggests that the practical answer is "yes". That said, resolving this fact against Fetch is gnarly since "extract a body" is buried deep within... Effectively what we have is:

Ideally, we wouldn't have to do the check in sendBeacon and defer it all to Fetch, but I don't see any easy way to reconcile the two worlds here. I guess I can keep the existing (sync check) logic in Beacon and then replicate similar checks within Fetch. Less than ideal, but I can't think of a better way. WDYT?

annevk commented 7 years ago

Do all existing implementations support FormData too? Anyway, if that is true I guess everyone has a way to synchronously get the size of these objects already, despite arguments at TPAC to the contrary by the same implementers... So I guess the primitive we should create is getting the size of these objects and reuse that somehow.

igrigorik commented 7 years ago

I believe so. We have a pending PR for web-plat tests (see https://github.com/w3c/web-platform-tests/pull/4024/files#diff-0c312b518bce4321b6708240f0a48519R38) that tests FormData and those seem to pass just fine in all existing implementations.

So I guess the primitive we should create is getting the size of these objects and reuse that somehow.

Should that live in Fetch?

annevk commented 7 years ago

Yeah, I guess Fetch is fine. Can refactor later if needed. We might also want to expose the size directly on these objects then…

igrigorik commented 7 years ago

Ok, I reverted some of the earlier logic change: we'll run the synchronous size check (as before) and return false if that fails; the call to fetch happens after that and runs in parallel with us returning from sendBeacon. In effect, the only substantive change here is that we changed "user agent may enforce limit" to "user agent will enforce limit", with actual enforcement happening in Fetch.

Speaking of Fetch, I think we need the following:

With above in place, both Beacon initiated and Fetch initiated requests will share the same enforcement logic and have the same behavior.

@annevk @toddreifsteck does that sound reasoanble? I can put together a Fetch PR for this..

annevk commented 7 years ago

I think we already keep track of transmitted bytes through https://fetch.spec.whatwg.org/#concept-body-transmitted. This sounds reasonable, but what seems to be missing is a definition of how you calculate the size of the objects.

andrewwakeling commented 7 years ago

Duplicating my question from #38:

If a create a series of empty payloads and choose to transmit data via the URL, what is the expected outcome, with regard to the quota and the behaviour of the browser?

igrigorik commented 7 years ago

For future reference...

@andrewwakeling see https://github.com/w3c/beacon/issues/38#issuecomment-268567846.

igrigorik commented 7 years ago

@toddreifsteck Fetch update is ready to land - see https://github.com/whatwg/fetch/pull/419#issuecomment-282359922. Please review the update here... I believe it should be good to merge.

p.s. also a bump for https://github.com/w3c/web-platform-tests/pull/4024 :-)

toddreifsteck commented 7 years ago

LGTM! Merging. Tests are coming very soon.