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

Full Network Layer Stubbing / fetch support #687

Closed
brian-mann opened this issue Sep 23, 2017 · 101 comments · Fixed by #4176
Closed

Full Network Layer Stubbing / fetch support #687

brian-mann opened this issue Sep 23, 2017 · 101 comments · Fixed by #4176
Assignees
Labels
Epic Requires breaking up into smaller issues pkg/https-proxy This is due to an issue in the packages/https-proxy directory topic: network type: feature New feature that does not currently exist

Comments

@brian-mann
Copy link
Member

brian-mann commented Sep 23, 2017

Pull Request (in progress): #4176

What users want

  • To be able to stub any kind of HTTP request including window.fetch
  • To blacklist certain domains (google analytics, new relic, intercom)
  • To have more control over dynamically routing / stubbing requests and responses
  • To more easily stub graphQL calls

What we do now

  • We modify the host XMLHttpRequest object which limits us to only stubbing XHR's from the main application frame.

What we need to do

  • Throw away the entire existing system for stubbing
  • Move stubbing into the internal Cypress network proxy layer (outside of the browser)
  • Synchronize the state of the browser tests with the backend (so its aware what needs to be stubbed / routed)
  • Remove cy.server and instead just use cy.route
  • Expand the API's for more advanced configuration
  • Accept blacklistDomains option for cypress.json
  • As each request comes in, if we expose callback functions we'll need to constantly communicate with the driver for guidance on what to do - whether to proceed - halt, etc. This can get pretty complicated and might have significant performance issues. We'll need to experiment and we may not be able to provide callback functions anymore.

Things to consider

  • onRequest and onResponse currently provide the raw level xhr object. This would no longer be possible. We'd have to serialize the request / response into basic objects. No method calling would be available.
  • The onError callback could no longer exist
  • Additionally we hook into the onerror events for XHR's and will automatically fail the test if this is called. This would no longer be possible.
  • Blacklisting https cannot be dynamic. It would have to be set in cypress.json or environment variables. The reason is that for each blacklisted domain that goes over https, we would have to route that traffic to a self signed certificate. Browser freak out if traffic to https gets assigned one certificate, and then within the same session it gets another cert applied. What this means is that before spawning the browser we have to isolate that traffic and know ahead of time not to route it to the actual server. This enables us to send a single cert to the browser.
  • We could enable non https hosts to be dynamically blacklisted, but these API's would be async and likely difficult for users to understand / use. We might be able to bake these into cy.route but I'm not sure there's much of a use case here of only dynamically blacklisting non https traffic.
  • We will no longer be able to tell the difference between an XHR and any other kind of http request. We'd likely have to change the page events to say REQ and REQ STUB instead of XHR and XHR STUB.
  • It would be very difficult to provide accurate onResponse callback functions. It's possible for the internal server to know when an HTTP request has been completed, but it's not possible to know whether the browser has actually finished processing it. This may or may not be a problem.
  • How would we handle stubbing binary / streaming responses?
  • How would we handle stubbing things like images? (This is likely pretty simple)

Bonus Points

  • It would likely be possible to stub Websocket traffic in the same manner
  • The problem with this is that most Websocket libraries (ie socket.io) add their own "encoding" pattern on top of each websocket message frame. The ergonomics around stubbing this would be odd. The problem is this is library (and even version of library) specific. This may be a better use case for cy.stub, or perhaps a 3rd party extension
@brian-mann brian-mann added the type: feature New feature that does not currently exist label Sep 23, 2017
@brian-mann brian-mann self-assigned this Sep 23, 2017
@brian-mann
Copy link
Member Author

brian-mann commented Sep 24, 2017

Related to:

@jennifer-shehane jennifer-shehane added the stage: proposal 💡 No work has been done of this issue label Sep 25, 2017
@jennifer-shehane jennifer-shehane changed the title Proposal: Full Network Layer Stubbing / Blacklisting / fetch support Full Network Layer Stubbing / Blacklisting / fetch support Sep 25, 2017
@jennifer-shehane jennifer-shehane added the pkg/https-proxy This is due to an issue in the packages/https-proxy directory label Sep 25, 2017
@ericadamski
Copy link

@brian-mann I would be very willing to help out with this issue. This is a big blocker for use when testing our app.

@brian-mann
Copy link
Member Author

@ericadamski could you go into more detail about this? What exactly is a big blocker?

@ericadamski
Copy link

@brian-mann making network requests (using a fetch polyfill) and asserting on those requests is important to our integration testing. Currently we are just spying on the window.fetch and doing some simple assertions on the params request. It would be ideal to provide the same functionality as XHRs to completely omit the server and mock responses when needed, understanding that we could stub the fetch as well, but this means stubbing each individual request with a tailored response.

@brian-mann
Copy link
Member Author

You can do this right now using stubs. Albeit it's certainly not as nice as what we did for XHR stubbing, but it is possible and we have several examples of making this work. This is even in a new getting started video we're releasing later this week.

https://github.com/cypress-io/cypress-example-recipes/blob/master/cypress/integration/spy_stub_clock_spec.js

@ericadamski
Copy link

Yes we realize it can be done, albeit we would rather wait (or work toward) a more XHR style stubbing, it is just super nice.

@brian-mann
Copy link
Member Author

Yeah it is. Okay just making sure we're on the same page.

The proposal here does a decent job outlining what needs to be done but it will be a significant amount of work.

Likely the next steps just need to be formalizing a plan for the API's themselves. There have been many requests for additional features and we should start there.

@ericadamski
Copy link

That sounds good.

Is it worth creating separate issues for some of the requested features for I know almost nothing about graphQL queries 😜

@brian-mann
Copy link
Member Author

They mostly already are other issues. We need to factor in all of them and come up with a set of API's that unifies those different use cases and desired experiences.

@ericadamski
Copy link

Right OK, I see what you are saying.

@ericadamski
Copy link

ericadamski commented Oct 10, 2017

@brian-mann side note, has it been thought of moving away from coffeescript and into es6?

@paulpruteanu
Copy link

Do you consider this feature in the next release? I reckon it would be highly valuable, for instance when

  • authentication/ other actions are performed at script level (<script src/> instead of XHR)
  • needing to bypass other scripts/ pixel images which takes a lot of time to load

@brian-mann
Copy link
Member Author

Yeah blacklisting specific requests is part of this feature roadmap

orlandohohmeier pushed a commit to dcos/dcos-ui that referenced this issue Dec 29, 2017
Add flags (query parameters) to identify and stub Mesos Operator API
requests to fix the integration tests that were otherwise failing or
flaky due to parsing errors in the `MesosStateStore`. The store failed
to parse the `GET_MASTER` response as we could only register one stub
for both the `GET_MASTER` and `SUBSCRIBE` requests resulting in JSON
parse errors due to the different response formats (RecoredIO<JSON>,
JSON).

The flags are necessary as Cypress can only stub requests based on the
URL and request method and doesn't provide the means to dynamically stub
requests based on the payload as documented in
cypress-io/cypress#521. We will remove the
flags once the Cypress team rewrote the network handling and added
support for dynamic stubs as documented in
cypress-io/cypress#687 or we've  addressed
https://jira.mesosphere.com/browse/DCOS_OSS-2021.
orlandohohmeier pushed a commit to dcos/dcos-ui that referenced this issue Dec 29, 2017
Add flags (query parameters) to identify and stub Mesos Operator API
requests to fix the integration tests that were otherwise failing or
flaky due to parsing errors in the `MesosStateStore`. The store failed
to parse the `GET_MASTER` response as we could only register one stub
for both the `GET_MASTER` and `SUBSCRIBE` requests resulting in JSON
parse errors due to the different response formats (RecoredIO<JSON>,
JSON).

The flags are necessary as Cypress can only stub requests based on the
URL and request method and doesn't provide the means to dynamically stub
requests based on the payload as documented in
cypress-io/cypress#521. We will remove the
flags once the Cypress team rewrote the network handling and added
support for dynamic stubs as documented in
cypress-io/cypress#687 or we've  addressed
https://jira.mesosphere.com/browse/DCOS_OSS-2021.
orlandohohmeier pushed a commit to dcos/dcos-ui that referenced this issue Dec 29, 2017
Add flags (query parameters) to identify and stub Mesos Operator API
requests to fix the integration tests that were otherwise failing or
flaky due to parsing errors in the `MesosStateStore`. The store failed
to parse the `GET_MASTER` response as we could only register one stub
for both the `GET_MASTER` and `SUBSCRIBE` requests resulting in JSON
parse errors due to the different response formats (RecoredIO<JSON>,
JSON).

The flags are necessary as Cypress can only stub requests based on the
URL and request method and doesn't provide the means to dynamically stub
requests based on the payload as documented in
cypress-io/cypress#521. We will remove the
flags once the Cypress team rewrote the network handling and added
support for dynamic stubs as documented in
cypress-io/cypress#687 or we've  addressed
https://jira.mesosphere.com/browse/DCOS_OSS-2021.
@bahmutov
Copy link
Contributor

Temporary experimental window.fetch polyfill available in v4.9.0

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Jun 29, 2020

The experimentalFetchPolyfill configuration option has been released as part of 4.9.0. When this option is true, Cypress will automatically replace window.fetch with a polyfill that Cypress can spy on and stub.

You should be able to remove any of the following code regarding fetch support:

Cypress.on('window:before:load', (win) => {	
  delete win.fetch	
})	

or

cy.visit('/', {	
  onBeforeLoad (win) {	
    delete win.fetch	
  },	
})	

And replace it with the below in your configuration file (cypress.json)

{
  "experimentalFetchPolyfill": true
}

You can see a full example of this in use here: https://github.com/cypress-io/cypress-example-recipes/tree/master/examples/stubbing-spying__window-fetch

If you encounter any issues with the new experimentalFetchPolyfill, please open a new issue with a reproducible example.

@alexkrolick
Copy link

Is it possible to stub network calls in a way that that doesn't care about whether the app uses fetch or XHR? Seems like in most cases that is an implementation detail we don't want to enshrine in a test.

@ngbrown
Copy link

ngbrown commented Jun 29, 2020

I was using xhook for stubbing and unfetch to polyfill fetches for Cypress. It doesn't appear that xhook is compatible with experimentalFetchPolyfill.

Using xhook with experimentalFetchPolyfill resulted in every network request failing. Once I removed xhook and just used experimentalFetchPolyfill, only the tests that relied on a stubbed response failed. So still waiting for #4176.

Looks good otherwise.

@jennifer-shehane
Copy link
Member

@alexkrolick We're working on bigger changes as part of #4176 to improve the experience behind network requests in general. The current workaround is meant to be helpful for users starting out so they don't have to dive through issues to figure out how to get fetch working.

@bahmutov
Copy link
Contributor

@ngbrown could you please provide a small reproducible example showing how experimentalFetchPolyfill fails?

@samtsai
Copy link
Contributor

samtsai commented Jul 17, 2020

I was using xhook for stubbing and unfetch to polyfill fetches for Cypress. It doesn't appear that xhook is compatible with experimentalFetchPolyfill.

Using xhook with experimentalFetchPolyfill resulted in every network request failing. Once I removed xhook and just used experimentalFetchPolyfill, only the tests that relied on a stubbed response failed. So still waiting for #4176.

Looks good otherwise.

cc @bahmutov

xhook replaces fetch as well and changes the interface, you can support both if you properly restore native/polyfilled-fetch after enabling xhook, something like this below:

return cy
    .readFile("node_modules/xhook/dist/xhook.min.js", { log: false })
    .as("xhook")
    .then((xhook) => {
      Cypress.on("window:before:load", (win) => {
        // save original fetch (which is probably Cypress's polyfill)
        const originalFetch = win.fetch
        win.eval(xhook)
        win.fetch = originalFetch // make sure we restore `fetch` to original
        win.xhook.before((req) => {
          let url = req.url
          if (url.indexOf("someapi") > -1 && req.body) {
            // do stuff
          }
        })
      })
    })

@samtsai
Copy link
Contributor

samtsai commented Jul 23, 2020

I was using xhook for stubbing and unfetch to polyfill fetches for Cypress. It doesn't appear that xhook is compatible with experimentalFetchPolyfill.
Using xhook with experimentalFetchPolyfill resulted in every network request failing. Once I removed xhook and just used experimentalFetchPolyfill, only the tests that relied on a stubbed response failed. So still waiting for #4176.
Looks good otherwise.

cc @bahmutov

xhook replaces fetch as well and changes the interface, you can support both if you properly restore native/polyfilled-fetch after enabling xhook, something like this below:

return cy
    .readFile("node_modules/xhook/dist/xhook.min.js", { log: false })
    .as("xhook")
    .then((xhook) => {
      Cypress.on("window:before:load", (win) => {
        // save original fetch (which is probably Cypress's polyfill)
        const originalFetch = win.fetch
        win.eval(xhook)
        win.fetch = originalFetch // make sure we restore `fetch` to original
        win.xhook.before((req) => {
          let url = req.url
          if (url.indexOf("someapi") > -1 && req.body) {
            // do stuff
          }
        })
      })
    })

So this turns out not to be as straightforward as I thought. I finally have the two working in my project but it required me to patch xhook if anyone is interested in I can share.

@joshribakoff
Copy link
Contributor

It would be very difficult to provide accurate onResponse callback functions. It's possible for the internal server to know when an HTTP request has been completed, but it's not possible to know whether the browser has actually finished processing it. This may or may not be a problem.

You can correlate it by hashing content length or other things like the URL & use https://developers.google.com/web/tools/chrome-devtools/network/resource-loading to have Cypress "see" in the network tab if the network request finished.

To play devil's advocate, it could also be argued people should not test in this way, because a normal user would not see if a network request has finished, and tests should closely match how a user behaves (maybe waiting for some element on the page to update in response to the network request finishing instead).

@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: needs review The PR code is done & tested, needs review stage: work in progress labels Aug 25, 2020
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Aug 31, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 1, 2020

Released in 5.1.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v5.1.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Sep 1, 2020
@jennifer-shehane
Copy link
Member

The features requested in this issue are now possible as part of cy.route2().

This is currently experimental and requires being enabled by passing "experimentalNetworkStubbing": true through your Cypress configuration. This will eventually be merged in as part of our standard API.

Please see the cy.route2() docs for full details: https://on.cypress.io/route2

If you encounter any issues or unexpected behavior while using cy.route2() we encourage you to open a new issue so that we can work out all the issues before public release. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Epic Requires breaking up into smaller issues pkg/https-proxy This is due to an issue in the packages/https-proxy directory topic: network type: feature New feature that does not currently exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.