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.request(...) fails if server is not available, with no option to return or catch the connection error #3161

Open
anaulin opened this issue Jan 17, 2019 · 23 comments
Labels
E2E Issue related to end-to-end testing type: enhancement Requested enhancement of existing feature

Comments

@anaulin
Copy link

anaulin commented Jan 17, 2019

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.

@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label Jan 18, 2019
@anaulin
Copy link
Author

anaulin commented Feb 7, 2019

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: percy/percy-cypress#32)

@chrisbreiding
Copy link
Contributor

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
Copy link

djones commented May 14, 2019

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

@brian-mann
Copy link
Member

@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
Copy link

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
Copy link
Contributor

flotwig commented May 20, 2019

@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:

.catch { backend: true }, (err) ->
$utils.throwErrByPath("request.loading_failed", {
onFail: options._log
args: {
error: err.message
stack: err.stack
method: requestOpts.method
url: requestOpts.url
}
})

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

@Robdel12
Copy link

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. 👍

Robdel12 added a commit to percy/percy-cypress that referenced this issue Aug 2, 2019
…HANGE) (#140)

BREAKING CHANGE:

## The problem

In all of the Percy SDKs we do a thing called a "heath check", which makes sure the Percy agent server is open and ready to accept the DOM we're going to `POST` to it. If the health check fails, we will disable Percy in the SDK so we're not failing your tests due to Percy not running. 

In the Cypress SDK, this was implemented in an interesting way due a limitation with `cy.request`.  The TL:DR of that is we can't `.catch` a failed request, so we needed to use `cy.exec` to health check & gracefully fall out. You can read a little more about that limitation here: cypress-io/cypress#3161

`cy.exec` has it's own issues, which are outlined here: #104 This is a major blocker for _a lot_ of customers. 

## What is this?

This will fix #104 (and hopefully #138).

`cy.task` will always execute within the version of node that is bundled with Cypress, so we no longer have to worry about `$PATH` issues that `cy.exec` faces. We're also no longer running a CLI command for the health check, this new implementation will make a HTTP request from node.

This does mean we need to update the docs to let users know there's an extra step to configure the SDK properly now.

### Upgrading to v2.x

With this change, you will now need to import this new task into your projects plugins (`cypress/plugins/index.js`) file. Without that, the SDK will not work at all. 


```js
/// In cypress/plugins/index.js

let percyHealthCheck = require('@percy/cypress/task')

module.exports = (on, config) => {
  // `on` is used to hook into various events Cypress emits
  // `config` is the resolved Cypress config

  // Make it possible to log things to stdout by calling 'cy.task('log', 'some message to log').
  // Useful for development and debugging.
  on("task", {
    log(message) {
      console.log(message);
      return null;
    }
  });
  on("task", percyHealthCheck);
};
```

## In the future

In an ideal world we would be able to `.catch` on `cy.request` and disable Percy after the first failed `POST` of the DOM. I think in the future we should look into working with Cypress to implement this so we can shrink the integration, make the SDK more reliable, and faster (since we won't make 2 network requests per-snapshot).

## FAQ

#### Why can't you use `fetch` over `cy.request` & `.catch` that?

We need to use `cy.request` since those requests aren't actually executed in the browser and avoid any CORS issues. 

## TODOs

- [x] Update Docs
- [x] How can I guarantee semantic release makes this a major version bump 
- [x] How does one integrate this task into their suite? It's not a default export.. And it runs in node. Maybe it can be apart of the default export
djones pushed a commit to percy/percy-cypress that referenced this issue Aug 2, 2019
# [2.0.0](v1.0.9...v2.0.0) (2019-08-02)

### Bug Fixes

* Use `cy.task` for health check rather than `cy.exec` (BREAKING CHANGE) ([#140](#140)) ([40550f7](40550f7)), closes [#104](#104) [#104](#104) [#138](#138)

### BREAKING CHANGES

* ## The problem

In all of the Percy SDKs we do a thing called a "heath check", which makes sure the Percy agent server is open and ready to accept the DOM we're going to `POST` to it. If the health check fails, we will disable Percy in the SDK so we're not failing your tests due to Percy not running.

In the Cypress SDK, this was implemented in an interesting way due a limitation with `cy.request`.  The TL:DR of that is we can't `.catch` a failed request, so we needed to use `cy.exec` to health check & gracefully fall out. You can read a little more about that limitation here: cypress-io/cypress#3161
@jennifer-shehane
Copy link
Member

Some followup on what decision were made on Percy's side - basically to move away from using the cy.request(). percy/percy-cypress#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

@cypress-bot cypress-bot bot added stage: ready for work The issue is reproducible and in scope and removed stage: needs investigating Someone from Cypress needs to look at this labels Aug 19, 2019
@jennifer-shehane jennifer-shehane added the type: enhancement Requested enhancement of existing feature label Aug 19, 2019
@Robdel12
Copy link

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
Copy link
Contributor

flotwig commented Aug 20, 2019

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:

.catch { backend: true }, (err) ->
$utils.throwErrByPath("request.loading_failed", {
onFail: options._log
args: {
error: err.message
stack: err.stack
method: requestOpts.method
url: requestOpts.url
}
})

Any new tests belong in in request_spec.coffee

@abanik0831
Copy link

any update on this?

@abbo2t
Copy link

abbo2t commented Apr 14, 2020

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

@jennifer-shehane
Copy link
Member

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

This comment has been minimized.

@stock-dd
Copy link

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
Copy link

aommm commented Oct 30, 2020

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
Copy link

yuankez commented Aug 6, 2021

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
Copy link

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

@pappacena
Copy link

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

@cypress-bot cypress-bot bot added stage: icebox and removed stage: ready for work The issue is reproducible and in scope labels Apr 28, 2022
@flotwig flotwig removed their assignment Sep 1, 2022
@Hayden-isitt
Copy link

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

@JanST123
Copy link

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
Copy link

bump

@nagash77 nagash77 added the E2E Issue related to end-to-end testing label Jun 5, 2023
@LukasLewandowski
Copy link

Any progress on this topic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing type: enhancement Requested enhancement of existing feature
Projects
None yet
Development

No branches or pull requests