-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix: don't show EPIPE error #32873
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
fix: don't show EPIPE error #32873
Conversation
|
|
The failures appear to be unrelated to the PR |
|
@alexsch01 Thanks, we'll review when we have time and also make sure tests pass and merge. |
|
The readme change entry is in the wrong spot but I'm not going to fix it until all potential checks are complete |
AtofStryker
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.
I'm not sure we would want to block all broken pipe errors coming back from the process but I can't think of a reason as to what specifically. So I'm OK moving forward with the change. @cacieprins cam you think of a scenario where this would be problematic?
@alexsch01 are you able to add a unit test to https://github.com/cypress-io/cypress/blob/develop/packages/data-context/test/unit/data/ProjectConfigIpc.spec.ts
Since the original issue happens sporadically when doing CTRL+C, it's tricky to test this |
@alexsch01 I'm more so talking about a unit test that emits and error to a mock child process and verifies the reject handler isn't called. I can take a look in setting that up if you'd like if that seems a bit challenging to do. |
|
@AtofStryker done |
| resolve() | ||
|
|
||
| return | ||
| } |
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: EPIPE handler resolves with wrong type
When an EPIPE error occurs in registerSetupIpcHandlers(), the promise resolves with undefined instead of the expected SetupNodeEventsReply object containing setupConfig, requires, and registrations properties. This causes runtime errors when callers try to access these properties, such as in ProjectConfigManager.handleSetupTestingTypeReply() which iterates over result.registrations.
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 ipc main process is dead so I don't actually see this being a problem
| resolve() | ||
|
|
||
| return | ||
| } |
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: Missing resolved flag prevents proper error handling
When EPIPE occurs in loadConfig(), the code calls resolve() but doesn't set resolved = true. If another error occurs afterward, handleChildProcessError will incorrectly call reject() on an already-resolved promise instead of calling onError(), causing subsequent errors to be silently ignored rather than properly reported.
|
@alexsch01 looks like there is a test failure? https://app.circleci.com/pipelines/github/cypress-io/cypress/77256/workflows/d625d494-edf6-4c36-9c39-2ac19b8f6d40/jobs/3309659
|
|
@AtofStryker addressed |


Additional details
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?Note
Suppress EPIPE errors from the config child process during load/setup, add a unit test, and note the fix in the changelog.
packages/data-context/src/data/ProjectConfigIpc.tsEPIPEin_childProcess.on('error')withinloadConfig()andregisterSetupIpcHandlers()by logging viadebug, resolving the pending promise, and returning early.NodeJS.ErrnoException.packages/data-context/test/unit/data/ProjectConfigIpc-real-child-process.spec.tsto simulateEPIPEand assert debug messages for both code paths.cli/CHANGELOG.md(15.8.0Bugfixes) to document the EPIPE terminal error fix.Written by Cursor Bugbot for commit 47d8cb5. This will update automatically on new commits. Configure here.