cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.71k stars 3.16k forks source link

cy.request(...) fails if server is not available, with no option to return or catch the connection error #3161

Open anaulin opened 5 years ago

anaulin commented 5 years ago

Current behavior

When you use cy.request(...) to connect to a server that isn't running, it throws a CypressError that is not possible to catch or otherwise handle gracefully:

CypressError: cy.request() failed trying to load:

http://localhost:5338/percy/healthcheck

We attempted to make an http request to this URL but the request failed without a response.

We received this error at the network level:

  > Error: connect ECONNREFUSED 127.0.0.1:5338

Desired behavior

I would expect that something similar to failOnStatusCode: false exists to allow me to handle the "connection refused" case in the same way as I can handle a, say, 404 code.

An alternative could be a catch method to be chained off of cy.request(...).

Context

I'm adding new functionality to the Percy Cypress plugin, which is implemented as a custom Cypress command (https://github.com/percy/percy-cypress). I need to do a check to see if the local percy agent service is running, before proceeding with any more work. Our users typically want to be able to run their test suite with or without "Percy mode", so when the local percy service is not running, we'd like to issue a warning but otherwise allow tests to proceed normally.

Because I want to avoid CSP or other security issues in the browser, I want to use cy.request(...) or some other capability that allows me to do HTTP requests from outside of the browser context.

If I was doing this for my own test suite, I could use a new cy.task(...) to implement this healthcheck, but in the context of a Cypress plugin, that would require users to do additional setup in their plugins/index.js, in addition to what they already have to do in support/commands.js.

An alternate solution to this problem would be exposing an API in Cypress that allows me to programmatically add a cy.task(...) that I can then use in my custom command.

Another alternative would be a hook to catch this CypressError within a command, and the ability to inspect it and re-throw it if appropriate.

anaulin commented 5 years ago

Would love to hear if you have any suggestions or thoughts on this.

I've written a workaround for this using cy.exec(...) and curl, but I don't love it (PR here: https://github.com/percy/percy-cypress/pull/32)

chrisbreiding commented 5 years ago

If I was doing this for my own test suite, I could use a new cy.task(...) to implement this healthcheck, but in the context of a Cypress plugin, that would require users to do additional setup in their plugins/index.js, in addition to what they already have to do in support/commands.js.

An alternate solution to this problem would be exposing an API in Cypress that allows me to programmatically add a cy.task(...) that I can then use in my custom command.

We're in the early stages of looking into a way to make this easier for plugin authors and users. Probably some way to auto-load plugins. For now, as you said, you would have to instruct your users to load your plugin in 2 places.

We'll also consider something like failOnStatusCode: false for cy.request to support this use-case.

I think your solution with cy.exec might be the best option for the time being.

djones commented 5 years ago

Would like to bump this. The workaround we've had to come up with is causing us some pain. See https://github.com/percy/percy-cypress/issues/104

brian-mann commented 5 years ago

@jennifer-shehane and @flotwig can you check to see if this issue will be fixed by 3.3.0 proxy improvements?

If so, please link + close it and maybe search for any other related issues that will also be closed by that PR...

Robdel12 commented 5 years ago

Curious to know as well 😃 We're seeing some users report more errors with 3.3 and the workaround we have to do the healthcheck: exec-fail

flotwig commented 5 years ago

@brian-mann @Robdel12 Unfortunately, Cypress 3.3.0 won't change this behavior. Any network error is caught by this code block and thrown, which ends the test:

https://github.com/cypress-io/cypress/blob/be3e9ed1dcd2b638e851165e5c741d153118a6d8/packages/driver/src/cy/commands/request.coffee#L240-L249

We could add an option so that cy.request returns the error instead of throwing it, but as it stands there is nothing.

Robdel12 commented 5 years ago

Thanks for the update Zach! I was hopeful 😊

I'm going to debug what's going on in the screenshot above but a user is reporting that causes their test suite to fail when Percy isn't running (even though we're passing failOnNonZeroExit: false) with the v3.3.0.

So far I haven't been able to reproduce but I'll open an issue if I can. 👍

jennifer-shehane commented 5 years ago

Some followup on what decision were made on Percy's side - basically to move away from using the cy.request(). https://github.com/percy/percy-cypress/pull/140

But, we want to make some option available to cy.request() to make this easier. From internal discussions, some options like ‘failOnNetworkErrors: false’, ‘returnError: true’ @flotwig

Robdel12 commented 5 years ago

FWIW, we plan on going forward with something like adding a .catch to cy.request. It would be great to collaborate on that 😊 We don't have any issues doing the work and opening a PR but don't want to step on toes or drop a surprise PR.

flotwig commented 5 years ago

I was thinking we could add a new option to cy.request:

cy.request({
    url: 'http://something.invalid',
    failOnNetworkError: false // default: true
})
.then(res => {
    // res is an object with the Node.js Error `code` and `message` attached
    // should probably either have `res.error = true` or `res.error = { code, message }`
    // so users can tell it apart from a success
})

@Robdel12 If you want to open a PR to add this feature, that would be great! I believe you just need to update commands/request.coffee to accept the new option and then add a condition in the .catch for cy.request here:

https://github.com/cypress-io/cypress/blob/beaa105b50a2a5a1b989688396b25c58b2013772/packages/driver/src/cy/commands/request.coffee#L244-L253

Any new tests belong in in request_spec.coffee

abanik0831 commented 4 years ago

any update on this?

abbo2t commented 4 years ago

I would also be interested in any updates with this request.

jennifer-shehane commented 4 years ago

There has been no work done for this feature. You can look at the GitHub tags to see the progress - this is still stage - ready for work. We would be open to a PR.

mikila85 commented 4 years ago

+1

stock-dd commented 4 years ago

Absolutely terrible. Cypress disallows using your own REST client or everything breaks. Meanwhile they don't allow to catch errors? Currently at work in a situation where a cy.request breaks and there is no way to find out how. Please fix this.

aommm commented 3 years ago

Just expressing my interest in this feature.

My use case is that I want to add some logging whenever certain cy.request calls fail. I'm wishing for perhaps something like this:

cy.request("/seed/user", "POST", user).catch(e => {
  cy.log("Failed to call /endpoint. Are you sure the backend is running?");
  throw e;
})

(I have also tried to putting the cy.request inside a try {...} block, but that didn't seem to work)

yuankez commented 3 years ago

Just want to bring this up again, do we have a proper way to handle the cy.request() error like @aommm example? Using a .catch seems like a good idea.

swiec-qualio commented 2 years ago

Also expressing my interest here. Would be great to bring retry-ability on cy.request level

pappacena commented 2 years ago

It is quite bad to not have any way to handle errors...

Hayden-isitt commented 1 year ago

God I'd love to be able to handle all of these errors I'm receiving

JanST123 commented 1 year ago

Also interested in a feature which allows to handle network errors. In our tests we want to assert that a server is no longer reachable, so a network error is expected, but currently it just fails the test..

sstraatemans commented 1 year ago

bump

LukasLewandowski commented 3 weeks ago

Any progress on this topic?