-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat: exit with code 112 when api errors prevent cloud orchestrated runs (record mode, parallel run mode) #32635
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
base: develop
Are you sure you want to change the base?
Conversation
cypress
|
Project |
cypress
|
Branch Review |
feat-api-failure-exit-112
|
Run status |
|
Run duration | 18m 33s |
Commit |
|
Committer | Cacie Prins |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
7
|
|
5
|
|
1018
|
|
4
|
|
19973
|
View all changes introduced in this branch ↗︎ |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
44.99%
|
|
---|---|
|
187
|
|
157
|
Accessibility
97.97%
|
|
---|---|
|
4 critical
8 serious
2 moderate
2 minor
|
|
101
|
Tests for review
The first 5 failed specs are shown, see all 623 specs in Cypress Cloud.
e2e/origin/cookie_login.cy.ts • 1 flaky test • 5x-driver-firefox
Test | Artifacts | |||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
... > past Max-Age, before Expires -> not logged in |
|
Test | Artifacts | |||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
src/cy/commands/files > #readFile > retries to read when ENOENT |
|
Test | Artifacts | |||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
... > stops waiting when an xhr request is canceled |
|
Test | Artifacts | |||||||
---|---|---|---|---|---|---|---|---|
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary |
|
Test | Artifacts | |
---|---|---|
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary |
Test Replay
|
return require('./errors').logException(err) | ||
.then(() => { | ||
debug('calling exit 1') | ||
await require('./errors').logException(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason we can just import errors at the top of the file since we are starting to move away from CJS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of how the integration tests are set up, this has to be dynamically required for now :(
break | ||
} | ||
case 'returnPkg': { | ||
const pkg = await require('./modes/pkg')(options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just import pkg from '@packages/root'
at the top of the file since its a bundled package now and just reference it here instead of ./modes/pkg
? Would be much simpler
break | ||
} | ||
case 'info': { | ||
await require('./modes/info')(options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just hoist this import or await import it? If not I'll likely get to this in the launcher refactor
}) | ||
}) | ||
|
||
it('errors and exits with 1 when posix exit codes are enabled', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats the reason the posix tests are added here and not in the posix PR? Is there some type of integration between these two I am missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I see its the differences between 112 vs 1 exit code
@@ -0,0 +1,8 @@ | |||
[31mWe encountered an unexpected error communicating with our servers.[39m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is so much easier to diff now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It really really is!
cli/CHANGELOG.md
Outdated
|
||
**Features:** | ||
|
||
- When the `run` command requires successful negotiation with the Cypress Cloud API and the `--posix-exit-codes` flag is set, Cypress will now exit with code `112` when it cannot determine which spec to run next due to network conditions. These Cloud API negotiations are required when either `--record` or `--parallel` flags are set. Addresses [#32485](https://github.com/cypress-io/cypress/issues/32485). Addressed in [#32635](https://github.com/cypress-io/cypress/pull/32635). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- When the `run` command requires successful negotiation with the Cypress Cloud API and the `--posix-exit-codes` flag is set, Cypress will now exit with code `112` when it cannot determine which spec to run next due to network conditions. These Cloud API negotiations are required when either `--record` or `--parallel` flags are set. Addresses [#32485](https://github.com/cypress-io/cypress/issues/32485). Addressed in [#32635](https://github.com/cypress-io/cypress/pull/32635). | |
- When `cypress run` is run with the `--record` or `--parallel` flags and the `--posix-exit-codes` flag is set, Cypress will now exit with code `112` when it cannot determine which spec to run next due to network conditions. Addresses [#32485](https://github.com/cypress-io/cypress/issues/32485). Addressed in [#32635](https://github.com/cypress-io/cypress/pull/32635). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the CLI docs https://docs.cypress.io/app/references/command-line, these are called "options" rather than "flags"
record
, or record
AND parallel
are valid choices, parallel
on its own is an error condition - see https://docs.cypress.io/app/references/error-messages#The---ci-build-id---group---tag---parallel-or---auto-cancel-after-failures-flags-can-only-be-used-when-recording so you don't even need to mention it.
Suggest:
When cypress run
is used with the --record
option together with the --posix-exit-codes
option ...
packages/server/lib/cypress.ts
Outdated
} | ||
} | ||
|
||
debug('calling exit 1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove I believe
} | ||
debug('end of startInMode, exit 0') | ||
|
||
return exit(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to have an 'exitErr'? We used to have handling of this here. I'm not sure when this would happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exitErr
is in the catch block above - this would get hit when one of the switch
cases that doesn't return
is hit (e.g., info
).
Before, every switch case return
ed a promise, even if they didn't need to (this is an old-style way of preventing the Zalgo effect that is rendered obsolete with async
/await
).
packages/server/lib/modes/record.ts
Outdated
import { getError, AllCypressErrorNames } from '@packages/errors' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import { getError, AllCypressErrorNames } from '@packages/errors' | |
import { getError } from '@packages/errors' | |
import type { AllCypressErrorNames } from '@packages/errors' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 660ab1b
packages/server/lib/modes/record.ts
Outdated
}) | ||
default: | ||
return throwErr('CLOUD_UNKNOWN_INVALID_REQUEST', { | ||
throw throwErr('CLOUD_UNKNOWN_INVALID_REQUEST', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding why this would change to throw either.
}) | ||
}) | ||
|
||
it('exits with code 1 for parallel cloud API failures when posix-exit-codes is enabled', function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This title is incorrect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title was correct, assertion was wrong ;) addressed in 3f096f2
Additional details
Adds
112
as a potential exit code when Cypress is run with POSIX complaint exit codes enabled (--posix-exit-codes
). This exit code is used when--record
or--parallel
mode is unable to communicate with Cypress Cloud API due to network conditions.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
? docs: Adds documentation for the112
exit code when posix-compliant exit codes are enabled. cypress-documentation#6294type definitions
?Note
Adds POSIX exit code 112 when Cloud network errors block recorded/parallel runs, introduces network-specific error types/messages, makes Cloud API timeouts configurable, and updates tests/snapshots accordingly.
112
when--posix-exit-codes
is enabled and Cloud network errors prevent proceeding in recorded (--record
) or parallel (--parallel
) runs.CLOUD_CANNOT_PROCEED_IN_PARALLEL_NETWORK
andCLOUD_CANNOT_PROCEED_IN_SERIAL_NETWORK
with tailored messaging; added toschema.graphql
anderrors.ts
plus snapshots.startInMode
/exitErr
flow to emit code112
for these errors; removemodes/exit.ts
and inline handling.CYPRESS_INTERNAL_API_TIMEOUT
; use as default across Cloud API calls.112
, configurable timeouts, and messaging; update changelog for15.5.0
.Written by Cursor Bugbot for commit 4152a21. This will update automatically on new commits. Configure here.