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

Cookies are not sent using fetch from electron client #4433

Closed
souphan-adsk opened this issue Jun 11, 2019 · 11 comments · Fixed by #4720
Closed

Cookies are not sent using fetch from electron client #4433

souphan-adsk opened this issue Jun 11, 2019 · 11 comments · Fixed by #4720

Comments

@souphan-adsk
Copy link

Current behavior:

When the Electron client used by cypress run performs fetch, no cookies are being sent.
This seems to be a behaviour in Cypress' Electron client handling, as I cannot seem to notice it on a raw Electon client.

Desired behavior:

Cookies should be sent. Running against a Chrome browser passes the test.

Steps to reproduce: (app code and test code)

A full minimal project can be found at https://github.com/souphan-adsk/cypress-electron-ajax-cookie

Here are key code sample:

Cypress Spec

describe('Cookie Test', function() {
	it('should send ajax cookie', function() {
		cy.visit('localhost:8880/get-cookie-test');
		cy.wait(500);
		cy.get('#ajax-cookie-value').invoke('text').then((value) => {
			expect(value).to.equal('test=OK')
		});
	});
});

Express

expressApp.get('/get-cookie-test', function(request, response) {
	console.log(request.originalUrl, 'headers.cookie =>', request.headers.cookie);
	response.cookie('test', 'OK', {
		expires: new Date(1000 * 60 * 60 * 24 + Date.now()),
	});
	response.send('<html><body>'
		+ `<p>Cookie value: <span id="cookie-string">${request.headers.cookie}</span></p>`
		+ '<p>Ajax Cookie value: <span id="ajax-cookie-value">...</span></p>'
		+ '<script type="text/javascript">'
		+ '  setTimeout(async () => {'
		+ '    let resp = await fetch("/cookie-echo");'
		+ '    document.getElementById("ajax-cookie-value").innerText = await resp.text();'
		+ '  }, 0);'
		+ '</script>'
		+ '</body></html>'
	);
});

expressApp.get('/cookie-echo', function(request, response) {
	console.log(request.originalUrl, 'headers.cookie =>', request.headers.cookie);
	response.send(request.headers.cookie);
});

Versions

Cypress 3.3.1
Electron 61
OSX

@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label Jun 17, 2019
@jsdw
Copy link

jsdw commented Jun 17, 2019

I have seen the same behaviour using Cypress 3.3.1 and OSX; non-fetch requests send cookies as expected, but fetch based requests do not.

@Steakeye
Copy link

Experiencing a similar issue working for a client and our tests suddenly stopped failing in headless mode; when debugging the issue it appears as though the cookies we need - which we set earlier before all tests run - are not being passed in the request from the electron client; this is confirmed when using dev tools to look at the network traffic of the headless mode but with --headed flag passed. Everything works fine when using the Cypress UI to access the tests and run them in Chrome.

@Steakeye
Copy link

Steakeye commented Jun 24, 2019

Our workaround has been to use the --browser chrome flag; additionally, in CI we now use a different docker image; we switched from using a base image to one that included Chrome (from cypress/base:8 to cypress/browsers:node8.9.3-chrome73).

@tgelu
Copy link

tgelu commented Aug 1, 2019

I experienced the same issue. I figured that the culprit was that the fetch implementation in chrome differs slightly from the one in electron.
According to the fetch specs the credentials property on the Request object you pass to fetch defaults to omit if not specified. That means, among other things, that cookies are not sent with the request unless you specifically say so by passing credentials: "include" or credentials: "same-origin" as described here https://fetch.spec.whatwg.org/#concept-request-credentials-mode
In chrome, though, if you do not specify credentials it seems like it defaults to "same-origin", contrary to WHATWG, which means the cookies will be sent with each fetch request.
I fixed my issue by setting the credentials: "same-origin" on all the requests where I needed that.

@Steakeye
Copy link

Steakeye commented Aug 1, 2019

@tgelu

I experienced the same issue. I figured that the culprit was that the fetch implementation in chrome differs slightly from the one in electron.
According to the fetch specs the credentials property on the Request object you pass to fetch defaults to omit if not specified. That means, among other things, that cookies are not sent with the request unless you specifically say so by passing credentials: "include" or credentials: "same-origin" as described here https://fetch.spec.whatwg.org/#concept-request-credentials-mode
In chrome, though, if you do not specify credentials it seems like it defaults to "same-origin", contrary to WHATWG, which means the cookies will be sent with each fetch request.
I fixed my issue by setting the credentials: "same-origin" on all the requests where I needed that.

This is because the spec changed; modern Chrome uses the updated spec while the Electron instance of Chromium that Cypress uses is out of date.

@tgelu
Copy link

tgelu commented Aug 1, 2019

@Steakeye - does this mean this is not an issue with cypress itself but with electron?

@Steakeye
Copy link

Steakeye commented Aug 1, 2019 via email

@bahmutov
Copy link
Contributor

bahmutov commented Aug 1, 2019

yes, and we are trying to upgrade Electron to the latest v6 in this pull request #4720

@cypress-bot cypress-bot bot added stage: work in progress and removed stage: needs investigating Someone from Cypress needs to look at this labels Aug 13, 2019
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Sep 12, 2019
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Sep 24, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 24, 2019

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

1 similar comment
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 24, 2019

The code for this is done in cypress-io/cypress#4720, 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 Oct 23, 2019

Released in 3.5.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants