Delayed 304 responses when using proxy

[bjowes]

Current behaviour:

When a page loads resources, and the resources have been loaded before, the web server returns a 304 response. When running Cypress without proxy, this works fine. When running Cypress with proxy, there is a 5 second delay before the response reaches the browser.

Desired behavior:

There should be no additional delay on 304 responses with a proxy.

Steps to reproduce: (app code and test code)

I have created a minimal demo for the issue. Please see:
https://github.com/bjowes/delayed-response-demo

The README contains instruction on using the demo.

In this simplistic case, the tests still pass. In my real scenario I am testing an Angular app with many more resources. The tests fail nearly every time due to timeout caused by delayed 304 responses.

Versions

Tested with Cypress 3.3.1 on Windows 10 and OS X 10.14.4.
Browser: Chrome v74.
Also tried in headless mode with Electron 61.
I haven't double checked but I believe the behaviour has been present at least since Cypress 3.1.0.

Comment

Since the proxy I use is made by me (and based on the node-http-mitm-proxy library), I realise that the error may lie in the proxy itself. However, I have verified that using Chrome with my proxy but without Cypress works just fine.

The only thing the node-http-mitm-proxy does to the traffic is setting transfer-encoding to chunked and removing the content-length header (if any). I have observed that if I remove this in the case of 304 response (which has no content) it works fine. Hence I presume there is some issue related to chunked encoding consisting of only an empty chunk.

Personally I can live with my workaround but I believe that it is not an unusual approach by proxies to always set transfer-encoding on responses to chunked, especially those that allow modification of the content.

[jennifer-shehane]

Hey @bjowes, we'll take a look.

Could you provide your current workaround? It may be helpful to others in this situation at the moment. Thanks!

[bjowes]

Sure thing @jennifer-shehane !
Workaround version of node-http-mitm-proxy: https://github.com/bjowes/node-http-mitm-proxy
Version 1.0.2 of cypress-ntlm-auth plugin uses this workaround, will be released during the day.

[bjowes]

Note that the workaround is not implemented exactly as I originally described. The published workaround checks if there are any response filters registered in the proxy (response filters are used to modify the response content). If there aren't (which there won't be in case of cypress-ntlm-auth) the transfer encoding is left as it was in the response from the server.

[brian-mann]

@flotwig what are your initial thoughts about this issue as well as #4313 (comment)?

[flotwig]

@brian-mann For this issue, probably retrying under the hood when we shouldn't - there's an exact 5 second delay. For #4313, if it's not related to this, not sure.

[flotwig]

The 5 seconds thing being from 3.3.0 turned out to be a red herring, this issue is present at least as early as Cypress 3.2.0.

I recorded a few scenarios, all in Cypress 3.2.0:

• With the Transfer-Encoding: chunked proxy and cache disabled, no 304's are sent and it works fine: https://drive.google.com/open?id=11gpt40ZAbHguv3c8SXLxoY7iLQI_Y0IF
• Without chunked and cache disabled, no 304's are sent so it also works fine: https://drive.google.com/open?id=1VzLd-EJhrZt_Rjxn6PGd07J_zSZYp8Rj
• Without chunked and cache enabled, 304's are not delayed: https://drive.google.com/open?id=1BVrAZBMDxVzi4HT75E1k5v9CqZweq-RM
• With the Transfer-Encoding: chunked proxy and cache enabled, 304's take ~5 seconds to complete: https://drive.google.com/open?id=1bS6YM6uETbzNqpEBNYUOUYUyb02XXC0R

Trying to see if I can localize the issue to a particular part of the proxy layer now.

[bjowes]

Good progress @flotwig! I suspected that this issues was present also in earlier releases.

Did you consider the possibility that this may not be a proxy issue - maybe the same delay would occur with cache and chunked even without the proxy. In my case, the proxy causes chunked encoding so those go hand in hand, but it could just as well occur without a proxy. Just a hunch 😎

[flotwig]

Yeah, you're probably right in that it's not specific to proxies.

I recorded the conversations between the browser <-> Cypress <-> the mitm proxy:

Conversation between the browser and Cypress:

Conversation between Cypress and the chunked proxy:

Conversation between the chunked proxy and the server:

I think that node-http-mitm-proxy doesn't actually send valid Transfer-Encoding: chunked replies. The correct way to indicate a 0-length body with chunked encoding is to send an empty chunk before ending:

0\r\n
\r\n

...but node-http-mitm-proxy doesn't send anything at all, just a blank body. Still, Chrome can handle this without needing to wait 5 seconds, so Cypress should be able to too.

EDIT: This Node.js test seems to suggest that this is expected: https://github.com/nodejs/node/blob/8b4af64f50c5e41ce0155716f294c24ccdecad03/test/parallel/test-http-chunked-304.js#L58-L63

EDIT 2: Relevant comment: https://github.com/nodejs/node/blob/00ba75ed5ef2132ccbe043b3993bd90e7362966b/lib/_http_outgoing.js#L319-L321

[bjowes]

Yeah, found that in RFC 7230 section 3.3.3 point 1. I think the RFC is a bit too permissive - you can put any header you like on a 304 response, but you can’t send a body. That is a good recipe to confuse developers... It would probably have been easier if certain headers were simply not allowed in 304 responses (and similar non body responses)

But as it stands, I assume cypress needs to ignore those headers when parsing 304 responses.

[cypress-bot]

The code for this is done in cypress-io/cypress#4358, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

[cypress-bot]

Released in 3.3.2.