w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
378 stars 42 forks source link

unexpected statusCode/reasonPhrase for responseCompleted for 304 Not Modified responses #761

Open juliandescottes opened 3 months ago

juliandescottes commented 3 months ago

I initially thought there was a bug on the platform side in Firefox for 304 Not Modified responses because I would usually get "304 Not Modified" as statusCode+reasonPhrase in network.responseStarted, and then "200 OK" in network.responseCompleted.

To workaround the issue we started caching the statusCode & reasonPhrase in responseStarted. However as I am now implementing network.continueResponse where we can actively modify those, the workaround can no longer be used.

After digging a bit more in our implementation and in the spec, it actually seems that this is a correct behavior. The fetch spec clearly states that for 304 responses, the cached ("validated") response should replace the response received from the server. Therefore, in the network.responseCompleted event, we should no longer expose the 304 response, but rather the validated response (which might be 200 OK).

However I'm worried that from a consumer standpoint, it will be a bit unexpected. I can already see that it breaks Puppeteer in https://github.com/puppeteer/puppeteer/blob/69f9bca6f0a394c5bce1afbbeec3a98ae6f7c6df/test/src/network.spec.ts#L205-L206, which mostly cares about the responseCompleted status.

@OrKoN based on this I imagine the Chrome implementation still returns 304 Not Modified for network.responseCompleted. Do you know if there is any dedicated logic in CDP to ensure that?

OrKoN commented 3 months ago

I think for the final response started + completed we expect a 200 but for the redirect response a 304. The link you posted seems to be pointing to the redirect responses. I do not think there is a dedicated logic but CDP emits events for each of the intermediate requests.

Do you have links to the spec to see at what point the webdriver instrumentation is called for this case?

juliandescottes commented 3 months ago

I think for the final response started + completed we expect a 200 but for the redirect response a 304. The link you posted seems to be pointing to the redirect responses. I do not think there is a dedicated logic but CDP emits events for each of the intermediate requests.

Do you have links to the spec to see at what point the webdriver instrumentation is called for this case?

304 is not really a redirect though? We shouldn't have a new started / completed set of events for a 304 response?

Edit: adding spec references:

OrKoN commented 3 months ago

Probably you are right. I am not sure about the CDP implementation and I looked up that we also have some bugs for 304 responses https://github.com/puppeteer/puppeteer/issues/10145