chore: Updates based on PR feedback (#21137)

* add generic to cy.origin type * fix log type, update/add comments * fix comment indentation * specific generic * move RemoteState to internal types * add on links to experimental flag descriptions * chore: reduce nesting by flipping condition * fix test title * simplify failing log * rename variable * delete error property * fix types * fix type * remove unnecessary todo * update wait test * jquery -> this * update comment * remove vestigial autoRun * use finally * re-throw non-security errors * move back getting index * add new state types * remove unnecessary export * startsWith -> includes * it -> them * update system test * remove use of promise constructor * Revert "remove use of promise constructor" This reverts commit 35ccc28. * log errors from Page.getFrameTree * test if anything breaks when removing optional chaining operator * remove vestigial file * handle queue ending in cross-origin driver * fix coordinates spec * improve chrome/firefox check in extension * improve secure cookie regex * use production mode for cross-origin driver bundle * adding remoteStates.getPrimary * catch and ignore queue errors * remove optional chaining in postMessage handler * removed unnecessary async * update frame tree on cri client reconnect * fix formatting * renaming remoteStates variable * prevent requests from being paused if experimentalSessionAndOrigin flag is off Co-authored-by: Matt Schile <mschile@cypress.io>
cypress-io · Apr 22, 2022 · 3e6d6bf · 3e6d6bf · cypress-bot · Apr 22, 2022
1 parent ebaaf18
commit 3e6d6bf
Show file tree

Hide file tree

Showing 46 changed files with 447 additions and 342 deletions.
diff --git a/cli/schema/cypress.schema.json b/cli/schema/cypress.schema.json
@@ -256,7 +256,7 @@
         "experimentalSessionAndOrigin": {
           "type": "boolean",
           "default": false,
-          "description": "Enables cross-origin and improved session support, including the `cy.origin` and `cy.session` commands."
+          "description": "Enables cross-origin and improved session support, including the `cy.origin` and `cy.session` commands. See https://on.cypress.io/origin and https://on.cypress.io/session."
         },
         "experimentalSourceRewriting": {
           "type": "boolean",

diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts
@@ -56,15 +56,6 @@ declare namespace Cypress {
     password: string
   }
 
-  interface RemoteState {
-    auth?: Auth
-    domainName: string
-    strategy: 'file' | 'http'
-    origin: string
-    fileServer: string | null
-    props: Record<string, any>
-  }
-
   interface Backend {
     /**
      * Firefox only: Force Cypress to run garbage collection routines.
@@ -1430,7 +1421,7 @@ declare namespace Cypress {
      *      cy.get('h1').should('equal', 'Example Domain')
      *    })
      */
-    origin(urlOrDomain: string, fn: () => void): Chainable
+    origin<T extends any>(urlOrDomain: string, fn: () => void): Chainable<T>
 
     /**
      * Enables running Cypress commands in a secondary origin.
@@ -1441,9 +1432,9 @@ declare namespace Cypress {
      *      expect(foo).to.equal('foo')
      *    })
      */
-    origin<T>(urlOrDomain: string, options: {
+    origin<T, S extends any>(urlOrDomain: string, options: {
       args: T
-    }, fn: (args: T) => void): Chainable
+    }, fn: (args: T) => void): Chainable<S>
 
     /**
      * Get the parent DOM element of a set of DOM elements.
@@ -2846,7 +2837,7 @@ declare namespace Cypress {
      */
     experimentalInteractiveRunEvents: boolean
     /**
-     * Enables cross-origin and improved session support, including the `cy.origin` and `cy.session` commands.
+     * Enables cross-origin and improved session support, including the `cy.origin` and `cy.session` commands. See https://on.cypress.io/origin and https://on.cypress.io/session.
      * @default false
      */
     experimentalSessionAndOrigin: boolean
@@ -2981,7 +2972,6 @@ declare namespace Cypress {
     projectName: string
     projectRoot: string
     proxyUrl: string
-    remote: RemoteState
     report: boolean
     reporterRoute: string
     reporterUrl: string
@@ -5766,7 +5756,8 @@ declare namespace Cypress {
   }
 
   interface LogConfig extends Timeoutable {
-    id: number
+    /** Unique id for the log, in the form of '<origin>-<number>' */
+    id: string
     /** The JQuery element for the command. This will highlight the command in the main window when debugging */
     $el: JQuery
     /** The scope of the log entry. If child, will appear nested below parents, prefixed with '-' */
@@ -5779,7 +5770,7 @@ declare namespace Cypress {
     message: any
     /** Set to false if you want to control the finishing of the command in the log yourself */
     autoEnd: boolean
-    /** Set to false if you want to control the finishing of the command in the log yourself */
+    /** Set to true to immediately finish the log  */
     end: boolean
     /** Return an object that will be printed in the dev tools console */
     consoleProps(): ObjectLike

diff --git a/packages/driver/cypress/fixtures/auth/index.html b/packages/driver/cypress/fixtures/auth/index.html
@@ -70,8 +70,6 @@
 
     } else {
       const token = JSON.parse(cypressAuthToken)
-      // ToDo, check for expiry maybe?
-
       // If the token exists, hooray, give them a logout button to destroy the token and refresh.
       const tag = document.createElement("p");
       const text = document.createTextNode(`Welcome ${token.body.username}`);

diff --git a/packages/driver/cypress/integration/commands/navigation_spec.js b/packages/driver/cypress/integration/commands/navigation_spec.js
@@ -2344,9 +2344,9 @@ describe('src/cy/commands/navigation', () => {
         }
 
         cy.on('command:queue:before:end', () => {
-        // force us to become unstable immediately
-        // else the beforeunload event fires at the end
-        // of the tick which is too late
+          // force us to become unstable immediately
+          // else the beforeunload event fires at the end
+          // of the tick which is too late
           cy.isStable(false, 'testing')
 
           win.location.href = '/timeout?ms=100'

diff --git a/packages/driver/cypress/integration/cypress/log_spec.js b/packages/driver/cypress/integration/cypress/log_spec.js
@@ -91,7 +91,7 @@ describe('src/cypress/log', function () {
       expect(LogUtils.countLogsByTests(tests)).to.equal(6)
     })
 
-    it('returns zero if there are no agents routes or commands', () => {
+    it('returns zero if there are no agents, routes, or commands', () => {
       const tests = {
         a: {
           notAThing: true,

diff --git a/packages/driver/cypress/integration/dom/coordinates_spec.ts b/packages/driver/cypress/integration/dom/coordinates_spec.ts
@@ -238,15 +238,18 @@ describe('src/dom/coordinates', () => {
     }
 
     it('returns true if parent is a window and not an iframe', () => {
-      const win = getWindowLikeObject()
+      const win = cy.state('window')
 
       expect(isAUTFrame(win)).to.be.true
     })
 
-    it('returns true if parent is a window and getting its frameElement property throws an error', () => {
+    it('returns true if parent is a window and getting its frameElement property throws a cross-origin error', () => {
       const win = getWindowLikeObject()
+      const err = new Error('cross-origin error')
+
+      err.name = 'SecurityError'
 
-      cy.stub($elements, 'getNativeProp').throws('cross-origin error')
+      cy.stub($elements, 'getNativeProp').throws(err)
 
       expect(isAUTFrame(win)).to.be.true
     })

diff --git a/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_log.spec.ts b/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_log.spec.ts
@@ -16,7 +16,7 @@ context('cy.origin log', () => {
   })
 
   it('logs in primary and secondary origins', () => {
-    cy.origin('http://foobar.com:3500', () => {
+    cy.origin<string>('http://foobar.com:3500', () => {
       const afterLogAdded = new Promise<void>((resolve) => {
         const listener = (attrs) => {
           if (attrs.message === 'test log in cy.origin') {

diff --git a/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_waiting.spec.ts b/packages/driver/cypress/integration/e2e/multi-domain/commands/multi_domain_waiting.spec.ts
@@ -8,7 +8,11 @@ context('cy.origin waiting', () => {
 
   it('.wait()', () => {
     cy.origin('http://foobar.com:3500', () => {
-      cy.wait(500)
+      const delay = cy.spy(Cypress.Promise, 'delay')
+
+      cy.wait(50).then(() => {
+        expect(delay).to.be.calledWith(50, 'wait')
+      })
     })
   })
 

diff --git a/packages/driver/cypress/integration/e2e/multi-domain/multi_domain_yield_spec.ts b/packages/driver/cypress/integration/e2e/multi-domain/multi_domain_yield_spec.ts
@@ -118,7 +118,7 @@ describe('cy.origin yields', () => {
       done()
     })
 
-    cy.origin('http://foobar.com:3500', () => {
+    cy.origin<JQuery>('http://foobar.com:3500', () => {
       cy.get('[data-cy="dom-check"]')
     })
     .then((subject) => subject.text())
@@ -134,7 +134,7 @@ describe('cy.origin yields', () => {
       done()
     })
 
-    cy.origin('http://foobar.com:3500', () => {
+    cy.origin<{ key: Function }>('http://foobar.com:3500', () => {
       cy.wrap({
         key: () => {
           return 'whoops'

diff --git a/packages/driver/src/cy/commands/navigation.ts b/packages/driver/src/cy/commands/navigation.ts
@@ -13,7 +13,7 @@ import debugFn from 'debug'
 const debug = debugFn('cypress:driver:navigation')
 
 let id = null
-let previousUrlVisited: LocationObject | undefined
+let previouslyVisitedLocation: LocationObject | undefined
 let hasVisitedAboutBlank: boolean = false
 let currentlyVisitingAboutBlank: boolean = false
 let knownCommandCausedInstability: boolean = false
@@ -30,7 +30,7 @@ const reset = (test: any = {}) => {
 
   // continuously reset this
   // before each test run!
-  previousUrlVisited = undefined
+  previouslyVisitedLocation = undefined
 
   // make sure we reset that we haven't
   // visited about blank again
@@ -53,33 +53,7 @@ const timedOutWaitingForPageLoad = (ms, log) => {
   const anticipatedCrossOriginHref = cy.state('anticipatingCrossOriginResponse')?.href
 
   // Were we anticipating a cross origin page when we timed out?
-  if (anticipatedCrossOriginHref) {
-    // We remain in an anticipating state until either a load even happens or a timeout.
-    cy.isAnticipatingCrossOriginResponseFor(undefined)
-
-    // By default origins is just this location.
-    let originPolicies = [$Location.create(location.href).originPolicy]
-
-    const currentCommand = cy.queue.state('current')
-
-    if (currentCommand?.get('name') === 'origin') {
-      // If the current command is a cy.origin command, we should have gotten a request on the origin it expects.
-      originPolicies = [cy.state('latestActiveOriginPolicy')]
-    } else if (Cypress.isCrossOriginSpecBridge && cy.queue.isOnLastCommand()) {
-      // If this is a cross origin spec bridge and we're on the last command, we should have gotten a request on the origin of one of the parents.
-      originPolicies = cy.state('parentOriginPolicies')
-    }
-
-    $errUtils.throwErrByPath('navigation.cross_origin_load_timed_out', {
-      args: {
-        configFile: Cypress.config('configFile'),
-        ms,
-        crossOriginUrl: $Location.create(anticipatedCrossOriginHref),
-        originPolicies,
-      },
-      onFail: log,
-    })
-  } else {
+  if (!anticipatedCrossOriginHref) {
     $errUtils.throwErrByPath('navigation.timed_out', {
       args: {
         configFile: Cypress.config('configFile'),
@@ -88,9 +62,35 @@ const timedOutWaitingForPageLoad = (ms, log) => {
       onFail: log,
     })
   }
+
+  // We remain in an anticipating state until either a load even happens or a timeout.
+  cy.isAnticipatingCrossOriginResponseFor(undefined)
+
+  // By default origins is just this location.
+  let originPolicies = [$Location.create(location.href).originPolicy]
+
+  const currentCommand = cy.queue.state('current')
+
+  if (currentCommand?.get('name') === 'origin') {
+    // If the current command is a cy.origin command, we should have gotten a request on the origin it expects.
+    originPolicies = [cy.state('latestActiveOriginPolicy')]
+  } else if (Cypress.isCrossOriginSpecBridge && cy.queue.isOnLastCommand()) {
+    // If this is a cross origin spec bridge and we're on the last command, we should have gotten a request on the origin of one of the parents.
+    originPolicies = cy.state('parentOriginPolicies')
+  }
+
+  $errUtils.throwErrByPath('navigation.cross_origin_load_timed_out', {
+    args: {
+      configFile: Cypress.config('configFile'),
+      ms,
+      crossOriginUrl: $Location.create(anticipatedCrossOriginHref),
+      originPolicies,
+    },
+    onFail: log,
+  })
 }
 
-const cannotVisitDifferentOrigin = ({ remote, existing, originalUrl, previousUrlVisited, log, isCrossOriginSpecBridge = false }) => {
+const cannotVisitDifferentOrigin = ({ remote, existing, originalUrl, previouslyVisitedLocation, log, isCrossOriginSpecBridge = false }) => {
   const differences: string[] = []
 
   if (remote.protocol !== existing.protocol) {
@@ -109,7 +109,7 @@ const cannotVisitDifferentOrigin = ({ remote, existing, originalUrl, previousUrl
     onFail: log,
     args: {
       differences: differences.join(', '),
-      previousUrl: previousUrlVisited,
+      previousUrl: previouslyVisitedLocation,
       attemptedUrl: remote,
       originalUrl,
       isCrossOriginSpecBridge,
@@ -123,12 +123,12 @@ const cannotVisitDifferentOrigin = ({ remote, existing, originalUrl, previousUrl
   $errUtils.throwErrByPath('visit.cannot_visit_different_origin', errOpts)
 }
 
-const cannotVisitPreviousOrigin = ({ remote, originalUrl, previousUrlVisited, log }) => {
+const cannotVisitPreviousOrigin = ({ remote, originalUrl, previouslyVisitedLocation, log }) => {
   const errOpts = {
     onFail: log,
     args: {
       attemptedUrl: remote,
-      previousUrl: previousUrlVisited,
+      previousUrl: previouslyVisitedLocation,
       originalUrl,
     },
     errProps: {
@@ -434,9 +434,7 @@ const stabilityChanged = (Cypress, state, config, stable) => {
       }
 
       const onCrossOriginFailure = (err) => {
-        options._log.set('message', '--page loaded--').snapshot().end()
-        options._log.set('state', 'failed')
-        options._log.set('error', err)
+        options._log.set('message', '--page loaded--').snapshot().error(err)
 
         resolve()
       }
@@ -526,6 +524,7 @@ type InvalidContentTypeError = Error & {
 
 interface InternalVisitOptions extends Partial<Cypress.VisitOptions> {
   _log?: Log
+  hasAlreadyVisitedUrl: boolean
 }
 
 export default (Commands, Cypress, cy, state, config) => {
@@ -852,7 +851,7 @@ export default (Commands, Cypress, cy, state, config) => {
         onLoad () {},
       })
 
-      options.hasAlreadyVisitedUrl = !!previousUrlVisited
+      options.hasAlreadyVisitedUrl = !!previouslyVisitedLocation
 
       if (!_.isUndefined(options.qs) && !_.isObject(options.qs)) {
         $errUtils.throwErrByPath('visit.invalid_qs', { args: { qs: String(options.qs) } })
@@ -1125,7 +1124,7 @@ export default (Commands, Cypress, cy, state, config) => {
           // if the origin currently matches
           // then go ahead and change the iframe's src
           if (remote.originPolicy === existing.originPolicy) {
-            previousUrlVisited = remote
+            previouslyVisitedLocation = remote
 
             url = $Location.fullyQualifyUrl(url)
 
@@ -1138,10 +1137,10 @@ export default (Commands, Cypress, cy, state, config) => {
           // if we've already cy.visit'ed in the test and we are visiting a new origin,
           // throw an error, else we'd be in a endless loop,
           // we also need to disable retries to prevent the endless loop
-          if (previousUrlVisited) {
+          if (previouslyVisitedLocation) {
             $utils.getTestFromRunnable(state('runnable'))._retries = 0
 
-            const params = { remote, existing, originalUrl, previousUrlVisited, log: options._log }
+            const params = { remote, existing, originalUrl, previouslyVisitedLocation, log: options._log }
 
             return cannotVisitDifferentOrigin(params)
           }
@@ -1151,7 +1150,7 @@ export default (Commands, Cypress, cy, state, config) => {
           // origin which isn't allowed within a cy.origin block
           if (Cypress.isCrossOriginSpecBridge) {
             const existingAutOrigin = win ? $Location.create(win.location.href) : $Location.create(Cypress.state('currentActiveOriginPolicy'))
-            const params = { remote, existing, originalUrl, previousUrlVisited: existingAutOrigin, log: options._log, isCrossOriginSpecBridge: true, isPrimaryOrigin }
+            const params = { remote, existing, originalUrl, previouslyVisitedLocation: existingAutOrigin, log: options._log, isCrossOriginSpecBridge: true, isPrimaryOrigin }
 
             return isPrimaryOrigin ? cannotVisitPreviousOrigin(params) : cannotVisitDifferentOrigin(params)
           }
@@ -1238,6 +1237,7 @@ export default (Commands, Cypress, cy, state, config) => {
           // not a network failure, and we should throw the original error
           if (err.isCallbackError || err.isCrossOrigin) {
             delete err.isCallbackError
+            delete err.isCrossOrigin
             throw err
           }
 

diff --git a/packages/driver/src/cy/location.ts b/packages/driver/src/cy/location.ts
@@ -15,7 +15,7 @@ export const create = (state) => ({
       return location
     } catch (e) {
       // it is possible we do not have access to the location
-      // for example, if the app has redirected to a 2nd origin
+      // for example, if the app has redirected to a different origin
       return ''
     }
   },

diff --git a/packages/driver/src/cypress/command_queue.ts b/packages/driver/src/cypress/command_queue.ts
@@ -262,15 +262,15 @@ export class CommandQueue extends Queue<Command> {
   // @ts-ignore
   run () {
     const next = () => {
-      // start at 0 index if one is not already set
-      let index = this.state('index') || this.state('index', 0)
-
       // bail if we've been told to abort in case
       // an old command continues to run after
       if (this.stopped) {
         return
       }
 
+      // start at 0 index if one is not already set
+      let index = this.state('index') || this.state('index', 0)
+
       const command = this.at(index)
 
       // if the command should be skipped, just bail and increment index