feat: adding timeout option to writeFile command #19015
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for taking the time to open a PR!
|
|
|
tbiethman
commented
Nov 19, 2021
|
Test summary
Run details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||
mjhenkes
reviewed
Nov 22, 2021
…lue for read/write timeouts. Reverting server readFile/writeFile changes to remove explicit abort workflows.
…50-writefile-timeout
tbiethman
requested review from
jennifer-shehane
and removed request for
a team
…50-writefile-timeout
tbiethman
commented
Nov 30, 2021
mjhenkes
previously approved these changes
Nov 30, 2021
flotwig
suggested changes
Nov 30, 2021
…asing HTTP buffer max to node Buffer max.
tbiethman
dismissed stale reviews from mjhenkes, ryanthemanuel, and flotwig
via
152a121
…otentially impactful overhead
…ing for commands with active logs.
…50-writefile-timeout
mjhenkes
approved these changes
Dec 6, 2021
flotwig
approved these changes
Dec 6, 2021
| const verifyAssertions = () => { | ||
| return Cypress.backend('read:file', file, _.pick(options, 'encoding')) | ||
| return Cypress.backend('read:file', file, _.pick(options, 'encoding')).timeout(options.timeout) |
There was a problem hiding this comment.
Not with this PR, no. I'm going to log a separate issue to track and evaluate how we want to handle command/server timeouts and increase the determinism of those workflows.
…50-writefile-timeout
tbiethman
changed the title
tgriesser
added a commit
that referenced
this pull request
Dec 8, 2021
…text * 10.0-release: (45 commits) fix: various Nav Bar fixes (#19283) build: add patch package as a dev dependency for fe-shared chore: hoist is - fun with cached dependencies build: hoist is hard build: better hoisting strategy fix: remove windows and mac workflow from branch revert: remove change about node version 17 build: remove testing of desktop-gui assets build: run window & mac CI in this branch build: more fixes build: remove toycode mdi from launchpad rename patch because of dev dep build: fix merge issue in packages generation chore: update sass for windows compatibility fix: Do not crash when a ill formed URL request is proxied (#19274) fix: remove desktop-gui from circle.yml change whitepace in patch fix: adding timeout option to writeFile command (#19015) release 9.1.1 fix: patch-package is not applied in dist'ed build (#19239) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User facing changelog
A
timeoutoption has been added to the writeFile command, with a default value ofdefaultCommandTimeout.The default
maxHttpBufferSizefor the internal socket server has been increased to its maximum value.Additional details
During the investigation of #3350, it was discovered that writing large files could take a long time and cause the command to fail due to a non-specific Mocha timeout error. Typically, this failure was the result of us spending a long time encoding a very large file or overflowing the maximum size of the HTTP buffer as defined by the socket server.
The writeFile command has been updated to receive an explicit timeout option to give users more direct control over the command timeouts. It also uses a custom timeout implementation rather than relying on the default Mocha timeout; this allows us to report a more accurate error message when a timeout does occur. The default timeout value remains the
defaultCommandTimeoutvalue from the Cypress config.The readFile command already receives a timeout option, but it is only used as part of its associated assertion retry logic. I added a custom timeout for readFile as well to ensure that we can report the appropriate error messages when the readFile command itself times out. This doesn't require a user-facing API change.
How has the user experience changed?
Old error message:
New error message:
PR Tasks
cypress-documentation? Linktype definitions?Have new configuration options been added to thecypress.schema.json?