-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat(cli): add discriminated union types for errors #32909
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?
feat(cli): add discriminated union types for errors #32909
Conversation
- Add typed error interfaces with discriminated unions - Refactor error creation functions to use types - Add helper functions and type guards - Remove TODO comment about type enforcement This improves type safety and makes error handling more maintainable while maintaining backward compatibility.
|
|
|
- Move error-types.ts to cli/lib/ to match tsconfig rootDir - Fix import path from ../types/error-types to ./error-types - Add backward compatibility by making error functions accept plain objects - Use normalizeError() helper to convert plain objects to typed errors This resolves the original build errors: - TS6059: File not under rootDir - TS2345: Argument type issues with CypressCliError
| description: 'Unknown Cypress CLI error', | ||
| solution: genericErrorSolution, | ||
| } | ||
| } |
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.
Bug: Broken Type Guard Skips Required Code Field Validation
The asCliError function checks for 'type' in e but doesn't verify 'code' in e, yet casts to CypressCliError which requires both properties in the discriminated union. This could allow objects with only type to pass through as valid CypressCliError instances, breaking type discrimination logic.
| // If it's already a CLI error, return as-is | ||
| if (error && typeof error === 'object' && 'type' in error && 'code' in error) { | ||
| return error as CypressCliError | ||
| } |
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.
Bug: Type/Code Validation Gap Causes Runtime Errors
The asCliError function checks for both 'type' in error and 'code' in error before casting to CypressCliError, but doesn't validate that these properties have the correct literal type values required by the discriminated union. An object with arbitrary type and code values would pass this check but fail at runtime when used with type guards.
jennifer-shehane
left a comment
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.
@lucgonp Thanks! There are some errors in the build - can you look at them? I believe you would see them running nx run cypress:prebuild, but let me paste the current output since I'm not sure you can view it.
$ yarn postinstall && tsx ./scripts/start-build.ts
$ patch-package && tsx ./scripts/post-install.ts
patch-package 8.0.0
Applying patches...
@types/[email protected] ✔
@types/[email protected] ✔
@types/[email protected] ✔
@types/[email protected] ✔
set -e
rm -rf types/jquery/dist
in file types/chai-jquery/index.d.ts changing reference for types chai to relative path ../chai/index.d.ts
sed -i <reference types="chai" /> <reference path="../chai/index.d.ts" /> types/chai-jquery/index.d.ts
in file types/chai-jquery/index.d.ts changing reference for types jquery to relative path ../jquery/index.d.ts
sed -i <reference types="jquery" /> <reference path="../jquery/index.d.ts" /> types/chai-jquery/index.d.ts
in file types/sinon-chai/index.d.ts changing reference for types chai to relative path ../chai/index.d.ts
sed -i <reference types="chai" /> <reference path="../chai/index.d.ts" /> types/sinon-chai/index.d.ts
in file types/sinon-chai/index.d.ts changing reference for types sinon to relative path ../sinon/index.d.ts
sed -i <reference types="sinon" /> <reference path="../sinon/index.d.ts" /> types/sinon-chai/index.d.ts
sed -i from "sinon"; from "../sinon"; types/sinon-chai/index.d.ts
set -e
rm -rf build
mkdir -p build/bin
mkdir -p build/types
cp bin/cypress build/bin/cypress
cp NPM_README.md build/README.md
cp .release.json build/.release.json
cp -R types/*.ts build/types/
cp -R types/bluebird build/types
cp -R types/lodash build/types
cp -R types/mocha build/types
cp -R types/minimatch build/types
cp -R types/sinon build/types
cp -R types/sinon-chai build/types
cp -R types/chai build/types
cp -R types/chai-jquery build/types
cp -R types/jquery build/types
exec tsc -p tsconfig.build.json
lib/exec/spawn.ts(271,47): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/exec/xvfb.ts(45,49): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/exec/xvfb.ts(54,47): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/download.ts(110,29): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/install.ts(243,31): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/install.ts(250,33): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/unzip.ts(221,30): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/verify.ts(142,33): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
lib/tasks/verify.ts(368,31): error TS2345: Argument of type '{ description: string; solution: string; }' is not assignable to parameter of type 'CypressCliError'.
Type '{ description: string; solution: string; }' is missing the following properties from type 'UnexpectedError': type, code
/root/cypress/node_modules/shelljs/src/common.js:110
if (config.fatal) throw new Error(logEntry);
^
Error: exec:
at Object.error (/root/cypress/node_modules/shelljs/src/common.js:110:27)
at execSync (/root/cypress/node_modules/shelljs/src/exec.js:120:12)
at Object._exec (/root/cypress/node_modules/shelljs/src/exec.js:223:12)
at Object.exec (/root/cypress/node_modules/shelljs/src/common.js:335:23)
at shell (/root/cypress/cli/scripts/start-build.ts:26:7)
at Object.<anonymous> (/root/cypress/cli/scripts/start-build.ts:42:43)
at Module._compile (node:internal/modules/cjs/loader:1706:14)
at Object.transformer (/root/cypress/cli/node_modules/tsx/dist/register-D46fvsV_.cjs:3:1104)
at Module.load (node:internal/modules/cjs/loader:1441:32)
at Function._load (node:internal/modules/cjs/loader:1263:12)
Node.js v22.19.0
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
This improves type safety and makes error handling more maintainable while maintaining backward compatibility.
Additional details
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?Note
Adds discriminated union types for CLI errors and refactors error handling utilities to use strong typing, with changelog updates.
cli/lib/error-types.tsandcli/types/error-types.tsdefiningCypressCliErrordiscriminated union, specific error interfaces, type guards, factories (ErrorFactories), and helpers (createTypedError,asCliError,normalizeError).cli/lib/errors.tsto use typed errors: import types, return typed objects in factories (missingApp,binaryNotExecutable,notInstalledCI,smokeTestFailure,childProcessKilled,CYPRESS_RUN_BINARY.notValid).CypressCliError(getError,formErrorText,raise,throwFormErrorText,exitWithError); include platform info typing.asCliErrorutility; makeinvalidSmokeTestDisplayError.solutionargument optional and handle missing message.cli/CHANGELOG.mdunder15.6.1.Written by Cursor Bugbot for commit 4aa5b3e. This will update automatically on new commits. Configure here.