fix: Use post message instead of web sockets for spec bridges. #24243

mjhenkes · 2022-10-13T16:34:07Z

Closes Cypress fails to visit in cy.origin block after ~30 cy.origin blocks #22874
Closes Move XmlHttpRequest and Fetch patches to cross-origin injection code #23967

User facing changelog

cy.origin now supports > 30 unique origin spec bridges per test

Additional details

Chrome has a limit of 30 protected web sockets per page, previously each spec bridge had it's own websocket to communicate with the server, this pr changes the spec bridge to instead use postmessage to communicate to the primary cypress instance and use that websocket to communicate with the server. This avoids the 30 websocket limit.

This pr also moves fetch and xhr patches from the driver to the injection and makes them async.

Steps to test

A new test was added verifying this change in cy.origin.cy.ts

How has the user experience changed?

The user experience has remained the same.

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
Has a PR for user-facing changes been opened in cypress-documentation?
Have API changes been updated in the type definitions?

…use post message to communicate with the primary instance.

cypress-bot · 2022-10-13T16:34:10Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-10-13T16:41:16Z

Test summary

4121 • 0 • 1104 • 0 • 1

Run details


Project	cypress
Status	Passed
Commit	`49ee41f`
Started	Oct 19, 2022 5:41 PM
Ended	Oct 19, 2022 5:57 PM
Duration	16:09 💡
OS	Linux Debian - 11.4
Browser	WebKit 16

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/xhr.cy.js		1
1	src/cy/commands/xhr > abort > aborts xhrs currently in flight

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

…ridges

mjhenkes · 2022-10-17T20:28:43Z

packages/app/src/runner/event-manager.ts

-        return Cypress.primaryOriginCommunicator.toSpecBridgePromise(specBridge, 'snapshot:generate:for:log', {
-          name,
-          id: eventID,
+        return Cypress.primaryOriginCommunicator.toSpecBridgePromise({


toSpecBridgePromise now takes an object, so i can name parameters.

mjhenkes · 2022-10-17T20:29:51Z

packages/app/src/runner/event-manager.ts

@@ -611,8 +611,8 @@ export class EventManager {
    })

    // Reflect back to the requesting origin the status of the 'duringUserTestExecution' state
-    Cypress.primaryOriginCommunicator.on('sync:during:user:test:execution', ({ specBridgeResponseEvent }, origin) => {
-      Cypress.primaryOriginCommunicator.toSpecBridge(origin, specBridgeResponseEvent, cy.state('duringUserTestExecution'))
+    Cypress.primaryOriginCommunicator.on('sync:during:user:test:execution', (_data, { origin, responseEvent }) => {


response event has moved to be part the second parameter object and no longer part of data which should be only information sent by the caller.

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts

mjhenkes · 2022-10-17T20:37:48Z

packages/driver/src/cross-origin/communicator.ts

@@ -19,42 +19,41 @@ const SNAPSHOT_EVENT_PREFIX = `${CROSS_ORIGIN_PREFIX}snapshot:`
 * @param event the name of the event to be promisified.
 * @param specBridgeName the name of the spec bridge receiving the event.
 * @param communicator the communicator that is sending the message
+ * @param timeout - in ms, if the promise does not complete during this timeout, fail the promise.


added a timeout parameter since some of the automation/backend calls take longer than a second.

mjhenkes · 2022-10-17T20:38:23Z

packages/driver/src/cross-origin/communicator.ts

 }: {
  resolve: Function
  reject: Function
-  data?: any


data is no longer modified since the responseEvent is sent back separately

mjhenkes · 2022-10-17T20:39:33Z

packages/driver/src/cross-origin/communicator.ts

@@ -105,7 +104,7 @@ export class PrimaryOriginCommunicator extends EventEmitter {
        data.data.err = reifySerializedError(data.data.err, this.userInvocationStack as string)
      }

-      this.emit(messageName, data.data, data.origin, source)
+      this.emit(messageName, data.data, { origin: data.origin, source, responseEvent: data.responseEvent })


Group event metadata in a single object.

mjhenkes · 2022-10-17T20:40:48Z

packages/driver/src/cross-origin/communicator.ts

+   * @param data - any meta data to be sent with the event.
+   * @param responseEvent - the even to be responded with when sending back a result.
+   */
+  toSource (source: Window, event: string, data?: any, responseEvent?: string) {


Added toSource to shortcut when responding back to a post message that expects a response. This also allows us to work for both spec bridge messages and AUT messages.

mjhenkes · 2022-10-17T20:43:26Z

packages/driver/src/cross-origin/communicator.ts

   */
-  toPrimary (event: string, data?: Cypress.ObjectLike, options: { syncGlobals: boolean } = { syncGlobals: false }) {
+  toPrimary (event: string, data?: Cypress.ObjectLike, options: { syncGlobals: boolean } = { syncGlobals: false }, responseEvent?: string) {


we should consider making toPrimary take an object too. I didn't make the change here because i was already making enough changes, but would be willing to update if everyone agrees.

mjhenkes · 2022-10-17T20:45:51Z

packages/driver/src/cross-origin/cypress.ts

@@ -172,26 +175,18 @@ const attachToWindow = (autWindow: Window) => {
    patchFormElementSubmit(autWindow)
  }

-  Cypress.removeAllListeners('app:timers:reset')


moved this to the injection

mjhenkes · 2022-10-17T20:46:18Z

packages/driver/src/cross-origin/cypress.ts

  cy.urlNavigationEvent('before:load')

  cy.overrides.wrapNativeMethods(autWindow)

+  // @ts-expect-error - the injected code adds the cypressApplyPatchesOnAttach function to window
+  autWindow.cypressApplyPatchesOnAttach()


Added this as the one place to apply any patches on attach.

mjhenkes · 2022-10-17T20:46:31Z

packages/driver/src/cross-origin/cypress.ts

-  patchFetch(Cypress, autWindow)
-  patchXmlHttpRequest(Cypress, autWindow)


moved to the injection

mjhenkes · 2022-10-17T20:47:19Z

packages/driver/src/cross-origin/events/socket.ts

-  const onBackendRequest = (...args) => {
-    webSocket.emit('backend:request', ...args)
-  }
+  const onRequest = async (event, args) => {


we now delegate any websocket connections to the primary cypress instance and communicate with it via post message.

mjhenkes · 2022-10-17T20:48:16Z

packages/driver/src/cross-origin/patches/utils/index.ts

@@ -1,17 +0,0 @@
-export const captureFullRequestUrl = (relativeOrAbsoluteUrlString: string, window: Window) => {


moved to the runner

mjhenkes · 2022-10-17T20:49:50Z

packages/runner/injection/cross-origin.js

-  if (window.Cypress) {
-    // A spec bridge has attached so we don't need to forward errors to top anymore.
-    window.removeEventListener('error', handleErrorEvent)
-  } else {


this is now cleaned up in the cypressApplyPatchesOnAttach function

mjhenkes · 2022-10-17T20:51:29Z

packages/runner/injection/patches/fetch.ts

  // if fetch is available in the browser, or is polyfilled by whatwg fetch
  // intercept method calls and add cypress headers to determine cookie applications in the proxy
  // for simulated top. @see https://github.github.io/fetch/ for default options
-  if (!Cypress.config('experimentalSessionAndOrigin') || !window.fetch) {
+  if (!window.fetch) {


experimental session and origin config doesn't really apply here because it is only injected if the flag is enabled anyway

mjhenkes · 2022-10-17T20:51:52Z

packages/runner/injection/patches/fetch.ts

    return
  }

  const originalFetch = window.fetch

-  window.fetch = function (...args) {
+  window.fetch = async function (...args) {


we're now async so we can await the backend call.. i think previously it was a race condition.

AtofStryker

we can probably remove the beforeEach in validation.cy.ts now which might be a good signal if too many spec bridges causes a problem again

packages/driver/src/cross-origin/communicator.ts

packages/runner/injection/cross-origin.js

packages/runner/injection/patches/fetch.ts

packages/runner/injection/patches/utils/index.ts

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

…ridges

packages/driver/src/cross-origin/cypress.ts

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts

packages/driver/src/cross-origin/communicator.ts

Co-authored-by: Bill Glesias <bglesias@gmail.com>

…ridges

* fix: spec bridges no-longer create their own spec bridge but instead use post message to communicate with the primary instance. * fix test errors * fix tests, ignore change to cookie * Move patch code into injection * Code cleanup * Fix most failing tests * Enable patching for both spec bridges and the primary cypress instance. * clean up * Updates from self PR review * Remove before each limiting spec bridges. * rename attach function * whoops, better call the function with the correct parameters * Apply suggestions from code review Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> * Update packages/driver/src/cross-origin/communicator.ts Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Bill Glesias <bglesias@gmail.com> * updated test name Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com> Co-authored-by: Bill Glesias <bglesias@gmail.com>

mjhenkes added 3 commits October 12, 2022 16:28

fix: spec bridges no-longer create their own spec bridge but instead …

6bf8ac1

…use post message to communicate with the primary instance.

fix test errors

2ab9f47

fix tests, ignore change to cookie

84d6ed7

mjhenkes self-assigned this Oct 13, 2022

mjhenkes added 6 commits October 14, 2022 09:34

Move patch code into injection

aba2192

Code cleanup

5c24166

Merge branch 'develop' into matth/fix/cy-origin-fails-after-30-spec-b…

f21928b

…ridges

Fix most failing tests

b2870c1

Enable patching for both spec bridges and the primary cypress instance.

957b77d

clean up

97d958f

mjhenkes commented Oct 17, 2022

View reviewed changes

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts Show resolved Hide resolved

mjhenkes commented Oct 17, 2022

View reviewed changes

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts Show resolved Hide resolved

mjhenkes commented Oct 17, 2022

View reviewed changes

packages/driver/cypress/e2e/e2e/origin/patches.cy.ts Show resolved Hide resolved

mjhenkes commented Oct 17, 2022

View reviewed changes

mjhenkes requested a review from AtofStryker October 17, 2022 20:57

AtofStryker reviewed Oct 17, 2022

View reviewed changes

AtofStryker self-requested a review October 17, 2022 21:12

Remove before each limiting spec bridges.

1b43524

mjhenkes mentioned this pull request Oct 17, 2022

fix: Improve document.cookie patch #23643

Merged

2 tasks

mjhenkes added 2 commits October 18, 2022 08:34

rename attach function

950dd37

whoops, better call the function with the correct parameters

751e3f5

chrisbreiding self-requested a review October 18, 2022 14:09

chrisbreiding requested changes Oct 18, 2022

View reviewed changes

mjhenkes and others added 3 commits October 18, 2022 10:17

Apply suggestions from code review

97a8c93

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

Update packages/driver/src/cross-origin/communicator.ts

982f0ca

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

Merge branch 'develop' into matth/fix/cy-origin-fails-after-30-spec-b…

69c2318

…ridges

mjhenkes requested a review from chrisbreiding October 18, 2022 15:18

mjhenkes commented Oct 18, 2022

View reviewed changes

packages/driver/src/cross-origin/cypress.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

78aacc0

chrisbreiding approved these changes Oct 18, 2022

View reviewed changes

AtofStryker reviewed Oct 18, 2022

View reviewed changes

mjhenkes and others added 4 commits October 18, 2022 15:17

Apply suggestions from code review

101d19f

Co-authored-by: Bill Glesias <bglesias@gmail.com>

updated test name

b0c1e46

Merge branch 'develop' into matth/fix/cy-origin-fails-after-30-spec-b…

6cad0c5

…ridges

Merge branch 'develop' into matth/fix/cy-origin-fails-after-30-spec-b…

f0aa7c0

…ridges

AtofStryker self-requested a review October 18, 2022 21:32

AtofStryker approved these changes Oct 18, 2022

View reviewed changes

Merge branch 'develop' into matth/fix/cy-origin-fails-after-30-spec-b…

49ee41f

…ridges

mjhenkes merged commit 208efbf into develop Oct 19, 2022

mjhenkes deleted the matth/fix/cy-origin-fails-after-30-spec-bridges branch October 19, 2022 18:17

This was referenced Oct 19, 2022

Cypress fails to visit in cy.origin block after ~30 cy.origin blocks #22874

Closed

Move XmlHttpRequest and Fetch patches to cross-origin injection code #23967

Closed

AtofStryker mentioned this pull request Nov 9, 2022

cy.origin throws CORS error when using Microsoft b2c credentials #23661

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Use post message instead of web sockets for spec bridges. #24243

fix: Use post message instead of web sockets for spec bridges. #24243

mjhenkes commented Oct 13, 2022 •

edited

cypress-bot bot commented Oct 13, 2022

cypress bot commented Oct 13, 2022 •

edited

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

mjhenkes Oct 17, 2022

AtofStryker left a comment

		patchFetch(Cypress, autWindow)
		patchXmlHttpRequest(Cypress, autWindow)

		@@ -1,17 +0,0 @@
		export const captureFullRequestUrl = (relativeOrAbsoluteUrlString: string, window: Window) => {

fix: Use post message instead of web sockets for spec bridges. #24243

fix: Use post message instead of web sockets for spec bridges. #24243

Conversation

mjhenkes commented Oct 13, 2022 • edited

User facing changelog

Additional details

Steps to test

How has the user experience changed?

PR Tasks

cypress-bot bot commented Oct 13, 2022

cypress bot commented Oct 13, 2022 • edited

Test summary

Run details

Flakiness

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AtofStryker left a comment

Choose a reason for hiding this comment

mjhenkes commented Oct 13, 2022 •

edited

cypress bot commented Oct 13, 2022 •

edited