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

cy.visit does not send headers / change HTTP method when baseUrl contains :80 #5367

Closed
Richard-Walton opened this issue Oct 15, 2019 · 12 comments · Fixed by #5478
Closed

cy.visit does not send headers / change HTTP method when baseUrl contains :80 #5367

Richard-Walton opened this issue Oct 15, 2019 · 12 comments · Fixed by #5478
Assignees

Comments

@Richard-Walton
Copy link

Richard-Walton commented Oct 15, 2019

Current behavior:

cy.visit({ url, method, headers }) does not appear to honour the method, or headers

Desired behavior:

cy.visit({ url, method, headers }) should send the specified method and headers.

Steps to reproduce: (app code and test code)

cy.visit({ 
  url: '/ui', 
  method: 'POST',
  headers: {
    'mock-user': JSON.stringify(mockUser)
  }
});

I include an annotated Cypress debug log:

debug-log

Changing the method to GET still results in the headers not being sent

Versions

I tested Cypress 3.4.1 and 3.2.0.
Ubuntu LTS
Chrome 77

UPDATE: Cause tracked down to: #5367 (comment)

@Richard-Walton
Copy link
Author

Ok, something odd is going on here.

cy.request works (Passing the same url, method, and headers as I use in cy.visit).

BUT, I can get cy.visit to work. My app lives behind a load balancer. If I make my tests target the app directly (rather than going via the LB) the visit command works.

I'm not sure how to explain that behaviour?! cy.request works in both cases. The LB logs show that Cypress cy.visit doesn't send the headers etc (I don't think the LB is stripping them out... especially given cy.request works)

@Richard-Walton
Copy link
Author

Richard-Walton commented Oct 15, 2019

Further info: There is definitely some Cypress-level caching going on which I've just noticed.

The first time I run the tests (with cy.visit({ url, method, headers })) I see two requests made to my server... the first request contains correct method/headers, the 2nd is a GET without headers.

The first request/response is seemingly ignored (even though this request is exactly what I want!), and only the 2nd request/response is provided to the test runner.

Afterwards, clicking the re-run tests button results in only 1 request being made to my server (The one without the correct method/headers).

It almost feels like a CORS issue (two requests made, but in a CORS situation the first request is usually an OPTIONS req)... ?

@Richard-Walton
Copy link
Author

So, it seems the cookie header persists between the two requests (as long as the cookie is set via cy.setCookie. That's the only workaround I can see right now - to set my mock data in the cookie. Though this doesn't solve the method always being a GET (even when POST is specified).

@flotwig
Copy link
Contributor

flotwig commented Oct 15, 2019

Further info: There is definitely some Cypress-level caching going on which I've just noticed.

Yes, I can shed some more light on this:

cy.visit() doesn't actually do a normal Javascript navigation (for example, by setting window.location.href or submitting a <form>). Instead:

  1. cy.visit asks the Cypress backend to retrieve the remote content (the red arrow from your annotation)
  2. Once it receives the response from the backend, it validates the content (is HTML, is a 200 status code, ...)
  3. If it's a valid response, it is cached
  4. Then, the frontend is navigated to the requested URL using a normal HTTP GET with no special headers (the blue arrow from your annotation)
  5. The Cypress proxy serves the cached content from (3) in response

So, although you don't see the correct method and headers in DevTools, they are actually being sent in (2). Only one request is sent to the origin server in a cy.visit.


Are you 100% sure that your destination server isn't receiving a POST? I just tested locally on 3.4.1 and couldn't reproduce:

it("", () => {
  cy.visit({
    method: 'POST',
    url: 'http://requestbin.net/r/165n5db1'
  })
})

Request received:

image

@Richard-Walton
Copy link
Author

Richard-Walton commented Oct 15, 2019

@flotwig Thank you for the reply.

Yes, my server does get a POST - But only the first time I start the test runner. The response my server gives to the initial POST seems to be ignored as cypress makes a 2nd GET to the server immediately afterwards. When the tests are re-run (i.e I don't close/re-open the test runner) the server only receives a GET.

Edit: Just re-read your comment. I don't think (5) happens as you described... I appear to get the response to (4), not (3)

@flotwig
Copy link
Contributor

flotwig commented Oct 15, 2019

@Richard-Walton The 2nd GET (step 4 in my explanation) should not make it to your server. It's just pulling the response to your POST (step 3) from the proxy layer.

I hear what you're saying, but I can't reproduce this problem unfortunately. Can you provide a minimum reproducible example? A Git repo with a server setup that shows the problem would be perfect.

@Richard-Walton
Copy link
Author

I would love to, but I'm not sure how. The app being tested is running behind a load balancer (https://www.pulsesecure.net/products/virtual-traffic-manager/) - so the setup is a little complicated. I'll have a think.

So, it seems perhaps, for reasons that the 2nd request makes it to my server. Perhaps the logic in the cypress backend for determining which requests it handles / vs which ones make it to the origin is buggy? The only other bit of information I can offer is that the url I am visiting contains a query string param (http://cc2?version=)... perhaps it is relevant.

@Richard-Walton
Copy link
Author

Richard-Walton commented Oct 15, 2019

Haha, I got it! So...my baseUrl is set to http://cc2:80.

Removing the port fixes the issue. So, I assume there is some cache code which doesn't take into account the presence/lack of presence of :80 for HTTP requests so that it sees a request to cc2 being different to the cached cc2:80 response?

This also would make sense as to why when I target the app directly, rather than via the LB, it works. When targeting directly I set my baseUrl to cc2:<none-standard-app-port> (e.g. cc2:34562) and so, I'm guessing the port part of the URL is always sent (nothing is trying to be clever about when/when not to send the port and so the cache is always hit when we expect it to be).

^ Anyway, all just guess work. Right now, I can easily not append :80 to the baseUrl when testing my app via the load balancer.

I'll keep tracking this issue if you need any more info from me. Happy to help where I can :)

@Richard-Walton
Copy link
Author

@flotwig Any thoughts on the above^ ?

@flotwig
Copy link
Contributor

flotwig commented Oct 17, 2019

Ugh, that definitely sounds possible. The cy.visits are cached by URL, maybe we're keeping the default port when storing the response but not when pulling it back out, or vice versa. Thanks for tracking this down, this should be pretty easy to fix now.

@flotwig flotwig self-assigned this Oct 17, 2019
@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Oct 17, 2019
@flotwig flotwig added topic: network and removed stage: ready for work The issue is reproducible and in scope labels Oct 17, 2019
@Richard-Walton Richard-Walton changed the title cy.visit does not send headers / change HTTP method cy.visit does not send headers / change HTTP method when baseUrl contains :80 Oct 18, 2019
@jennifer-shehane jennifer-shehane added the stage: ready for work The issue is reproducible and in scope label Oct 28, 2019
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels Nov 6, 2019
brian-mann added a commit that referenced this issue Nov 8, 2019
* Add a bunch of extra tests in 2_cookies_spec

* Try not doing weird things to cookies

* if cypress.env.debug is set, log command execution thru server

* improve elctron logging - we missed this when doing cri-client logging

* make video_capture frames part of verbose

* cleanup

* use the data from getCookie to run deleteCookies

* fix screenshots

* still resolve with cleared cookie

* cy.getcookie now gets ALL cookies from ALL domains

* return Promise for followRedirect, not req.init wrap

* allow passing domain: null to cy.getCookies to get all

* update request_spec to be clearer

* still need to call followRedirect option during sendStream

* beautify the e2e tests

* correct e2e test + snapshot

* always discard default ports when get/set buffer - fixes #5367

* make cy.clearCookies({ domain: null }) clear ALL cookies

* update spec

* use string url as key

* rebalance some e2e tests to make time for new cookies e2e tests

* always add default port to buffer url

* jk, remove default port

* set hostOnly: true when appropriate when setting cookies back on visit

* update tests

* Revert "if cypress.env.debug is set, log command execution thru server"

This reverts commit 623ed44.

* try to clean diff, cookies_no_baseurl didnt change

* WIP move the expected cookies array out of snapshot and expect inline

* finish updating tests

* add missing snapshot

* remove useless onstdout


Co-authored-by: Brian Mann <[email protected]>
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Nov 8, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 8, 2019

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

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 8, 2019

Released in 3.6.1.

@cypress-io cypress-io locked as resolved and limited conversation to collaborators Dec 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants