Skip to content
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: prevent serialization issues with retries using multi-domain #20628

Merged
merged 9 commits into from
Mar 16, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@
// @ts-ignore
const isErrorObj = Cypress[fnName](USED_KEYS.error)

expect(isErrorObj).to.be.an.instanceof(Error)
// We preserve the error structure, but on preprocessing to the spec bridge, the error is converted to a flat object
expect(isErrorObj).to.be.an.instanceof(Object)
expect(isErrorObj.message).to.eq('error')
})
})
})
Expand Down
20 changes: 18 additions & 2 deletions packages/driver/src/multi-domain/communicator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,37 @@ export class PrimaryDomainCommunicator extends EventEmitter {
*/
toAllSpecBridges (event: string, data?: any) {
debug('=> to all spec bridges', event, data)

const preprocessedData = preprocessForSerialization<any>(data)

// if user defined data is passed in, do NOT sanitize it.
if (data?.data) {
preprocessedData.data = data.data
}

// If there is no crossDomainDriverWindow, there is no need to send the message.
Object.values(this.crossDomainDriverWindows).forEach((win: Window) => {
win.postMessage({
event,
data,
data: preprocessedData,
}, '*')
})
}

toSpecBridge (domain: string, event: string, data?: any) {
debug('=> to spec bridge', domain, event, data)

const preprocessedData = preprocessForSerialization<any>(data)

// if user defined data is passed in, do NOT sanitize it.
if (data?.data) {
preprocessedData.data = data.data
}

// If there is no crossDomainDriverWindow, there is no need to send the message.
this.crossDomainDriverWindows[domain]?.postMessage({
event,
data,
data: preprocessedData,
}, '*')
}
}
Expand Down
11 changes: 8 additions & 3 deletions packages/driver/src/util/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,19 @@ export const preprocessForSerialization = <T>(valueToSanitize: { [key: string]:
// because of this, we preprocess native errors to objects and postprocess them once they come back to the primary domain

if (_.isArray(valueToSanitize)) {
return _.filter(valueToSanitize, preprocessForSerialization) as unknown as T
return _.map(valueToSanitize, preprocessForSerialization) as unknown as T
}

if (_.isObject(valueToSanitize)) {
try {
const sanitizedValue = convertObjectToSerializableLiteral(valueToSanitize) as T
const sanitizedValueAsLiteral = convertObjectToSerializableLiteral(valueToSanitize) as T

return sanitizedValue
// convert any nested structures as well, if objects or arrays, to literals. This is needed in the case of Proxy objects
_.forEach(sanitizedValueAsLiteral as any, (value, key) => {
sanitizedValueAsLiteral[key] = preprocessForSerialization(value)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we technically only have to do this for Objects/Arrays, but figured this is easier to maintain to do full recursion in which the property is just returned. This can be optimized a bit more if we find this highly important

})

return sanitizedValueAsLiteral
} catch (err) {
// if its not serializable, tell the primary to inform the user that the value thrown could not be serialized
throw UNSERIALIZABLE
Expand Down
84 changes: 84 additions & 0 deletions system-tests/__snapshots__/multi_domain_retries_spec.ts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
exports['e2e multi domain retries / Appropriately displays test retry errors without other side effects'] = `

====================================================================================================

(Run Starting)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (multi_domain_retries_spec.ts) │
│ Searched: cypress/integration/multi_domain_retries_spec.ts │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: multi_domain_retries_spec.ts (1 of 1)


multi-domain test retries
(Attempt 1 of 3) appropriately retries test within "switchToDomain" without propagating other errors errors
(Attempt 2 of 3) appropriately retries test within "switchToDomain" without propagating other errors errors
1) appropriately retries test within "switchToDomain" without propagating other errors errors


0 passing
1 failing

1) multi-domain test retries
appropriately retries test within "switchToDomain" without propagating other errors errors:
AssertionError: expected true to be false
[stack trace lines]




(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 1 │
│ Passing: 0 │
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 3 │
│ Video: true │
│ Duration: X seconds │
│ Spec Ran: multi_domain_retries_spec.ts │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/multi_domain_retries_spec.ts/multi-domain test (1280x720)
retries -- appropriately retries test within switchToDomain without propagating
other errors errors (failed).png
- /XXX/XXX/XXX/cypress/screenshots/multi_domain_retries_spec.ts/multi-domain test (1280x720)
retries -- appropriately retries test within switchToDomain without propagating
other errors errors (failed) (attempt 2).png
- /XXX/XXX/XXX/cypress/screenshots/multi_domain_retries_spec.ts/multi-domain test (1280x720)
retries -- appropriately retries test within switchToDomain without propagating
other errors errors (failed) (attempt 3).png


(Video)

- Started processing: Compressing to 32 CRF
- Finished processing: /XXX/XXX/XXX/cypress/videos/multi_domain_retries_spec.ts.mp (X second)
4


====================================================================================================

(Run Finished)


Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ multi_domain_retries_spec.ts XX:XX 1 - 1 - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✖ 1 of 1 failed (100%) XX:XX 1 - 1 - -


`
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// @ts-ignore / session support is needed for visiting about:blank between tests
describe('multi-domain test retries', { experimentalSessionSupport: true }, () => {
beforeEach(() => {
cy.visit('/multi_domain.html')
cy.get('a[data-cy="multi_domain_secondary_link"]').click()
})

it('appropriately retries test within "switchToDomain" without propagating other errors errors', function () {
cy.switchToDomain('http://foobar.com:4466', () => {
cy.then(() => {
expect(true).to.be.false
})
})
})
})
49 changes: 49 additions & 0 deletions system-tests/test/multi_domain_retries_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import path from 'path'
import systemTests, { expect } from '../lib/system-tests'
import Fixtures from '../lib/fixtures'

const e2ePath = Fixtures.projectPath('e2e')

const PORT = 3500
const onServer = function (app) {
app.get('/multi_domain_secondary.html', (_, res) => {
res.sendFile(path.join(e2ePath, `multi_domain_secondary.html`))
})
}

describe('e2e multi domain retries', () => {
systemTests.setup({
servers: [{
port: 4466,
onServer,
}],
settings: {
hosts: {
'*.foobar.com': '127.0.0.1',
},
},
})

systemTests.it('Appropriately displays test retry errors without other side effects', {
// keep the port the same to prevent issues with the snapshot
port: PORT,
spec: 'multi_domain_retries_spec.ts',
snapshot: true,
expectedExitCode: 1,
config: {
experimentalMultiDomain: true,
experimentalSessionSupport: true,
retries: 2,
},
async onRun (exec) {
const res = await exec()

// verify that retrying tests with multi domain doesn't cause serialization problems to spec bridges on test:before:run:async
expect(res.stdout).to.not.contain('TypeError')
expect(res.stdout).to.not.contain('Cannot set property message of which has only a getter')

expect(res.stdout).to.contain('AssertionError')
expect(res.stdout).to.contain('expected true to be false')
},
})
})