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

Bug: experimentalFetchPolyfill modify network requests query string #7813

Closed
martin-pikalek opened this issue Jun 25, 2020 · 15 comments
Closed
Assignees
Labels
experiment: fetch polyfill Issues when using experimentalFetchPolyfill v4.9.0 🐛 Issue present since 4.9.0

Comments

@martin-pikalek
Copy link

martin-pikalek commented Jun 25, 2020

Current behavior:

When experimentalFetchPolyfill is turned on, network requests with query string sent by tested application have modified query string by FetchPolyfill.

For example: request sent by application with url

https://domain.com/api/filter?param1=false&param2=value1&param2=value2

is modified to

https://domain.com/api/filter?param1=false&param2=value1,value2

Modified request of course return different response by backend server or cannot be processed.

Desired behavior:

experimentalFetchPolyfill feature should not modify query string of any requests.

Test code to reproduce:

Sorry, I'm not application developer (Frontend or Backend) to prepare small test application to give you reproducible example. If you are able to reproduce this with my description of problem, I would be glad.

Versions : Cypress 4.9.0 , MacOS Catalina 10.15.5

Tested in Chrome 83 and Electron 80

@martin-pikalek martin-pikalek changed the title experimentalFetchPolyfill modify network requests query experimentalFetchPolyfill modify network requests query string Jun 25, 2020
@martin-pikalek martin-pikalek changed the title experimentalFetchPolyfill modify network requests query string Bug: experimentalFetchPolyfill modify network requests query string Jun 25, 2020
@TomaszG
Copy link
Contributor

TomaszG commented Jun 26, 2020

I can confirm, this happens to me as well. As a result, data to the tested app is not loaded correctly. Turning that option off and using the old fetch replacement fixes the issue.

@jennifer-shehane jennifer-shehane added experiment: fetch polyfill Issues when using experimentalFetchPolyfill v4.9.0 🐛 Issue present since 4.9.0 labels Jun 26, 2020
@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label Jun 26, 2020
@bahmutov
Copy link
Contributor

bahmutov commented Jun 26, 2020

@martin-pikalek there is no definite standard on how the duplicate query parameters should be sent, but I checked the source code for the polyfill we use and did not see anything that would change the URL parameter.

https://github.com/developit/unfetch/blob/master/src/index.mjs

Just to confirm, I have created https://github.com/cypress-io/cypress-test-tiny/tree/query-string
where the app requests the following

const url = 'https://jsonplaceholder.cypress.io/comments?param1=false&param2=value1&param2=value2'
window.fetch(url)

No polyfill

experimental fetch polyfill is false

Screen Shot 2020-06-26 at 5 17 43 PM

Polyfill

experimental fetch polyfill is true, so you see the XHR in the command log

Screen Shot 2020-06-26 at 5 18 18 PM

Seems the same query in both cases. We would need a reproducible example thus to work on this issue

@bahmutov bahmutov added stage: needs information Not enough info to reproduce the issue and removed stage: needs investigating Someone from Cypress needs to look at this labels Jun 26, 2020
@JessicaSachs
Copy link
Contributor

We need additional info about the failing request. I believe that this is an issue. We should not be modifying user requests in any way (regardless of Web Standards). Can someone please take a look at @bahmutov’s example repository and see if there is a difference between your own internal application?

@ghost
Copy link

ghost commented Jun 26, 2020

I will try to look tomorow. But it is strange. I compared request url on same environment of application normaly in browser and then in cypress with experimental polyfill and query in url is changed in cypress and test fails. Sorry but I cannot show you our application data.

@ghost
Copy link

ghost commented Jun 26, 2020

I found it because after turning off this experimental polyfill test did not fail.

@bahmutov
Copy link
Contributor

Please provide a reproducible example, since I could not reproduce it and did not see how it would be modifying the URL parameter.

@JessicaSachs
Copy link
Contributor

JessicaSachs commented Jun 29, 2020

@Ethnity @TomaszG or @martin-pikalek when you make the fetch request, what shows up in your command log? You can write a bare-bones test and blur out sensitive information.

Screen Shot 2020-06-29 at 12 06 04 PM

@martin-pikalek
Copy link
Author

@JessicaSachs first image is from cypress with experimentalPolyfill and second image is from browser. PUT request with query param is send after clicking the button.

Screenshot 2020-06-30

Screenshot 2020-06-30-2

@TomaszG
Copy link
Contributor

TomaszG commented Jun 30, 2020

Now I'm not sure if the bug which I have observed is the same, let me know if I should move it to a separate ticket.

My API request is:
http://localhost:3001/api/sth1/sth2?sth3=4dcedc8e-ba9a-11ea-b78f-0242ac1a0002&sth4="sth_5"&sth6=false

The logged response with experimentalFetchPolyfill turned on is a text response:

response: "["Peer ID 4dd129a8-ba9a-11ea-b78f-0242ac1a0002","Peer ID 4dd3b9b6-ba9a-11ea-b78f-0242ac1a0002"]"
responseText: "["Peer ID 4dd129a8-ba9a-11ea-b78f-0242ac1a0002","Peer ID 4dd3b9b6-ba9a-11ea-b78f-0242ac1a0002"]"
responseType: ""

However, with experimentalFetchPolyfill turned off (and whatwg-fetch turned on instead - link) I get:

response: Blob {size: 95, type: "application/json"}
responseText: [Exception: DOMException: Failed to read the 'responseText' property from 'XMLHttpRequest': The value is only accessible if the object's 'responseType' is '' or 'text' (was 'blob').]
responseType: "blob"

which, when logged using the following code:

.wait('@alias').then(xhr => {
            Cypress.Blob.blobToBinaryString(xhr.responseBody)
              .then(JSON.parse)
              .then(responseBody => {
                cy.log(responseBody);
              });
          })

gets me the same result:

 "["Peer ID 4dd129a8-ba9a-11ea-b78f-0242ac1a0002","Peer ID 4dd3b9b6-ba9a-11ea-b78f-0242ac1a0002"]"

However, in the first case, the application cannot parse the response correctly and therefore, the data is not loaded to it.

@martin-pikalek
Copy link
Author

martin-pikalek commented Jun 30, 2020

@TomaszG I think it is maybe different issue. I have issue with duplicity query parameters that are stacked together by experimentalFetchPolyfill which causing wrong response from backend server.

@martin-pikalek
Copy link
Author

@JessicaSachs first image is from cypress with experimentalPolyfill and second image is from browser. PUT request with query param is send after clicking the button.

Screenshot 2020-06-30 Screenshot 2020-06-30-2

@JessicaSachs I tried it with chrome runner and electron runner - both showing exactly same problem with merged duplicity query parameters (after that, backend server do not return right response, button click action is not performed correctly and test fails). When I turned 'experimentalFetchPolyfill' off for whole test or only for that test suite, correct action in UI is made (test succeed, but of-course i do not see mocked request in Cypress UI)

@bahmutov
Copy link
Contributor

Can you work around this using cy.route2? Since this experimental fetch polyfill is a temporary measure, we don't plan to investigate it or enhance it.

@martin-pikalek
Copy link
Author

Can you work around this using cy.route2? Since this experimental fetch polyfill is a temporary measure, we don't plan to investigate it or enhance it.

@bahmutov I did not tried it on that project yet but I will. I am using route2 on another project and seems working perfectly, did not find any issues yet :) . I will write result soon if the problem was solved by using route2 instead of route.

@alxndr
Copy link

alxndr commented Nov 13, 2020

Updating from 5.4.0 to 5.6.0 has solved my issue — the app code's modifications to Headers are now being preserved (in an un-stubbed request) with experimentalFetchPolyfill turned on.


Initial report for posterity:

I have found that with the experimentalFetchPolyfill option turned on, a request using fetch with a Headers object will lose the custom header value.

I am testing an app which adds an auth header to the requests which the front-end makes to the back-end. However when I look at one of these requests in the Network dev tool, it does not include the auth header any more, and the back-end returns an error. With "experimentalFetchPolyfill": false, "experimentalNetworkStubbing": true the header is present and the back-end returns a successful response.

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Dec 30, 2020

experimentalFetchPolyfill has been deprecated in Cypress 6.0.0. We will not be addressing any new work on this feature. We encourage you to use cy.intercept() to intercept requests using the Fetch API instead.

@jennifer-shehane jennifer-shehane removed the stage: needs information Not enough info to reproduce the issue label Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experiment: fetch polyfill Issues when using experimentalFetchPolyfill v4.9.0 🐛 Issue present since 4.9.0
Projects
None yet
Development

No branches or pull requests

6 participants