Should callback error message is ignored when element is not found

[JessefSpecialisterren]

Current behavior

When you throw an Error with a custom message from a .should callback, the custom message is only shown if the preceding .get finds at least one element:

Desired behavior

Cypress should always show the custom error message.

Test code to reproduce

it('shows the custom error message when the element is found', () => {
  cy.visit('https://example.cypress.io/')
  cy.get('html').should(() => { throw new Error('this gets shown') })
})

it('does not show the custom error message when no element is found (bug)', () => {
  cy.visit('https://example.cypress.io/')
  cy.get('nonexistent').should(() => { throw new Error('this doesn\'t get shown') })
})

Cypress Version

10.11.0

Node version

v16.16.0

Operating System

10.0.19044.2130

Debug Logs

No response

Other

This is likely a regression. Cypress's own docs contain an example that is likely affected by this bug: Source: https://docs.cypress.io/api/commands/should#Assert-class-name-contains-heading

[BlueWinds]

This is an unfortunate side effect of the way Cypress tries to display helpful error messages. Basically, if a .should() fails, we perform an existence check on the incoming element, and if that fails, display that error instead of the original one. This is usually helpful! For example,

cy.get('#does-not-exist').should('have.class', 'foo')

fails with Expected to find element #does-not-exist, rather than Expected 'undefined' to have class 'foo'.

I suppose we could skip this "perform extra existence check if assertion fails'" logic when the assertion is of the callback variety, but I'm not sure adding more edge cases and conditions to an already convoluted error generation process is the right solution here. Open to other ideas if anyone has them.

[JessefSpecialisterren]

@BlueWinds Thank you for the response! I agree that, for non-callback assertions, this logic makes sense 🙂. However, for callback assertions, it may actually be confusing. Getting Expected to find element seems to imply that Cypress checks for existence, where it actually only does that when your callback throws. This confused me a great deal when I first ran into it, with a callback like this:

cy.get('li').should($el => {
  $el.each((_, element) => {
    expect(element.textContent).to.equal('foo')
  })
}

The assertion would instantly pass when the list hadn't loaded yet and no <li> elements were found, even though the Cypress docs state "All DOM based commands automatically wait for their elements to exist in the DOM. You never need to write .should('exist') after a DOM based command." (https://docs.cypress.io/guides/core-concepts/introduction-to-cypress#Default-Assertions).

One might argue that, for callback assertions, it would be better to document that the user needs to check for existence themselves. Getting rid of the "perform extra existence check if assertion fails'" logic for callback assertions would help drive that point home and avoid confusion. It would also allow us to provide some explanation in the error message when a crucial element is not found, which is a Very Good Thing when you're testing a behemoth of an application like our AUT 😅

[BlueWinds]

That all plays into some work I've been considering around how .should() works in general (did you know assertions run as part of the previous command? Weird, I know) - but that's a longer-term project, probably won't happen in the next couple months.

As a more immediate solution, we'd absolutely welcome a docs PR to explain it more clearly.

https://github.com/cypress-io/cypress/blob/a451d17c666f2cfd50ee9b094a4c0a00e35e94f2/packages/driver/src/cy/assertions.ts#L301-L315 are the lines of code that replace your custom error with the generic 'does not exist' error, and checking for callback assertions would be something like isCallbackShould = cmds.find(cmd => _.isFunction(cmd.get('args')[0])), if you (or anyone else) wants to contribute a change in this area.

[BlueWinds]

Oh, or you could do something like isCypressErr(err) || isAssertionErr(err), to check if it's a cypress-generated error message - that's probably better than looking specifically for cmds (.should()s) with callbacks. I haven't played with it to be sure, but that'd be a more general solution to try out first.