fix: Improve error handling around calls to this.next in middleware

[mike-plummer]

• Closes cypress.run module api throws "Error running proxy middleware: Cannot call this.next() more than once in the same middleware function." #22825

Additional details

Issue was reported in v10.3.0 - a CT test structured in a very particular way could get a "this.next() function getting called twice within a given middleware invocation" error message. This was due to bad error-handling logic within Cypress and disguised an actual error in the CT server instance. This particular error condition is fixed in this PR specifically for component tests.

Other users reported similar error messages in v10.x and v11.2.0 in e2e tests but were unable to provide a reproduction or debug logs. I have been unable to reproduce the issue in an e2e test in v12.5.1 - there's' been changes to this area of Cypress recently so it's possible this was "accidentally fixed". In case it was not I'm adding more error logging - this should help identify the root error and give us additional info if it happens to be a Cypress bug (or allow the user to fix issues in their own config/logic).

Note: This additional error logging is not strictly necessary since the correct error is being exposed if you print a network request failure to devtools console from the command log, enable DEBUG logging, or inspect from a cy.intercept, but those troubleshooting steps may not be intuitive for this sort of error based on how the original issue unfolded. I'm open to opinions on how to handle this if the logging feels too heavy-handed.

Steps to test

Clone reproduction case, follow instructions in README

This example shows current behavior, new behavior with fix for internal error, and new behavior with improved error logging

How has the user experience changed?

1. Fixed potential internal error in middleware for CT tests
2. Added logging to terminal for errors propagating out of middleware so they are more evident - user could previously access if they inspected the failed network call from command log or printed from a cy.intercept but this should make them more evident
3. Added logging to "double this.next invocation" condition to help identify if a previously-handled error may have contributed - this should help diagnose bad middleware customizations or intercept logic

PR Tasks

• Have tests been added/updated?
• Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
• [na] Has a PR for user-facing changes been opened in cypress-documentation?
• [na] Have API changes been updated in the type definitions?

[cypress]

39 flaky tests on run #43899 ↗︎

0

26786

1271

0

39

Details:

Fix unit tests
Project: cypress	Commit: fbd5d51590
Status: Passed	Duration: 18:18 💡
Started: Feb 7, 2023 9:10 PM	Ended: Feb 7, 2023 9:29 PM

  e2e/origin/cookie_behavior.cy.ts • 4 flaky tests • 5x-driver-electron

View Output Video

Test
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)
... > same site / cross origin > XMLHttpRequest > sets cookie on same-site request if withCredentials is true, and attaches to same-site request if withCredentials is true
... > same site / cross origin > fetch > sets same-site cookies if "include" credentials option is specified from request, but does not attach same-site cookies to request by default (same-origin)

  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test
... > correctly returns currentRetry
... > correctly returns currentRetry
... > correctly returns currentRetry

  create-from-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
... > runs generated spec		Screenshot

  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs		Screenshot

  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found		Screenshot

The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[lmiller1990]

Great work to track this down - looks like the original user who reported it did a good job debugging, too.

I did not pull the try the reproduction yet, but the code looks good - left a few minor but non-blocking comments.

[flotwig]

I think the type error this works around could be due to us using an outdated @types/node... but that may be more than a 1-line change to update.

It doesn't look like logError will log cause, so I'm not sure this is going to get logged by line 218:

cypress/packages/errors/src/errorUtils.ts

Lines 55 to 71 in 07052d8

	export const logError = function (err: CypressError | ErrorLike, color: AllowedChalkColors = 'red') {
	console.log(chalk[color](err.message))
	if (err.details) {
	console.log(chalk.magenta(`\n${err.details}`))
	}
	// bail if this error came from known
	// list of Cypress errors
	if (isCypressErr(err)) {
	return
	}
	console.log(chalk[color](err.stack ?? ''))
	return err
	}

[mike-plummer]

Updated logError to recurse down cause (if present) and log to a fixed depth in case there's a loop - rare, but I've been bitten by that in the past

Took a look at updating the @types/node bindings - it would end up touching a 3-4 other packages and my yarn was having serious indigestion trying to bump the version so I may leave as-is with the type workaround just to keep this PR targeted