cypress icon indicating copy to clipboard operation
cypress copied to clipboard

fix: do not re-use readstreams when retry an upload of the test replay recording

Open cacieprins opened this issue 8 months ago • 2 comments

  • Closes https://github.com/cypress-io/cypress/issues/29227

Additional details

If node-fetch began reading from the Test Replay recording archive for uploading, and encountered a retryable error, it would attempt to re-use the same ReadStream on subsequent retries. These retries would fail, because ReadStreams may only be opened for reading once.

This PR refactors the previous upload_stream logic:

  • A generic asyncRetry function is used instead of fetch-retry-ts. This function can be subsequently used in other refactors (e.g., reconnection in CriClient).
  • A putFetch function is used to determine Http vs. Network vs. response Parsing errors, for more streamlined error handling. This pattern can be subsequently re-used and reduced via DRY for other HTTP methods. Especially useful is the ability to define how the response body should be parsed (other options can be added as needed, like Blob or ArrayBuffer), and what the type of the response should be. This will aid in any future refactors of other API methods.
  • The logic for creating the artifact ReadStream and StreamMonitor (for stall detection) is moved to put_protocol_artifact.ts, which is wrapped in asyncRetry and leverages putFetch, all of which simplifies error handling and ensures both ReadStream and StreamMonitor are instantiated fresh for each upload attempt.

A subsequent PR will be opened to remove the now-unreferenced fetch-retry-ts package.

Steps to test

Run the record_spec system tests; run a test recording against staging; (tbd) use Charles proxy or similar to simulate initial failures and subsequent successes on upload

How has the user experience changed?

Retry attempts to upload that previously failed due to Stream issues may retry properly now. This will not resolve issues that cause the initial (or subsequent) upload attempt to fail caused by the upstream server or local network conditions, it simply ensures that we are retrying properly.

PR Tasks

cacieprins avatar Jun 25 '24 20:06 cacieprins