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: stop subscription race condition error on debug page #27134

Merged
merged 9 commits into from Jun 28, 2023
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ _Released 07/05/2023 (PENDING)_
**Bugfixes:**

- Fixed an issue where chrome was not recovering from browser crashes properly. Fixes [#24650](https://github.com/cypress-io/cypress/issues/24650).
- Fixed a race condition that was causing a GraphQL error to appear on the [Debug page](https://docs.cypress.io/guides/cloud/runs#Debug) when viewing a running Cypress Cloud build. Fixed in [#27134](https://github.com/cypress-io/cypress/pull/27134).

## 12.16.0

Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/debug/DebugTestingProgress.vue
Expand Up @@ -49,7 +49,7 @@ subscription DebugTestingProgress_Specs($id: ID!) {
relevantRunSpecChange(runId: $id) {
id
...DebugPendingRunCounts
scheduledToCompleteAt
scheduledToCompleteAt
}
}
`
Expand Down
6 changes: 4 additions & 2 deletions packages/data-context/src/polling/poller.ts
Expand Up @@ -30,7 +30,7 @@ export class Poller<E extends EventType, T = never, M = never> {

debug(`subscribing to ${this.event} with initial value %o and meta %o`, config?.initialValue, config?.meta)
this.#subscriptions[subscriptionId] = { meta: config.meta }
debug('subscriptions before subscribe', this.#subscriptions)
debug('subscriptions after subscribe', this.#subscriptions)

if (!this.#isPolling) {
this.#isPolling = true
Expand All @@ -57,13 +57,14 @@ export class Poller<E extends EventType, T = never, M = never> {
}

async #poll () {
debug(`polling for ${this.event}`)
if (!this.#isPolling) {
debug('terminating poll after being stopped')

return
}

debug(`polling for ${this.event} with interval of ${this.pollingInterval} seconds`)
debug(`calling poll callback for ${this.event}`)

await this.callback(this.subscriptions)

Expand All @@ -73,6 +74,7 @@ export class Poller<E extends EventType, T = never, M = never> {
return
}

debug(`setting timeout with interval of ${this.pollingInterval} second`)
this.#timeout = setTimeout(async () => {
this.#timeout = undefined
await this.#poll()
Expand Down
36 changes: 33 additions & 3 deletions packages/data-context/src/sources/RelevantRunSpecsDataSource.ts
Expand Up @@ -88,7 +88,8 @@ export class RelevantRunSpecsDataSource {
//TODO Get spec counts before poll starts
if (!this.#poller) {
this.#poller = new Poller(this.ctx, 'relevantRunSpecChange', this.#pollingInterval, async (subscriptions) => {
debug('subscriptions', subscriptions)
debug('subscriptions', subscriptions.length)

const runIds = uniq(compact(subscriptions?.map((sub) => sub.meta?.runId)))

debug('Polling for specs for runs: %o', runIds)
Expand Down Expand Up @@ -128,13 +129,30 @@ export class RelevantRunSpecsDataSource {
this.ctx.emitter.relevantRunSpecChange(run)
}
})

debug('processed runs')
})
}

const fields = getFields(info)

/**
* Checks runs as they are emitted for the subscription to make sure they match what is
* expected by the subscription they are attached to. First, verifies it is for the run
* being watched by comparing IDs. Then also verifies that the fields match. The field validation
* was needed to prevent a race condition when attaching different subscriptions for the same
* run that expect different fields.
*
* @param run check to see if it should be filtered out
*/
const filter = (run: PartialCloudRunWithId) => {
debug('calling filter', run.id, runId)
const runIdsMatch = run.id === runId
const runFields = Object.keys(run)
const hasAllFields = fields.every((field) => runFields.includes(field))

debug('Calling filter %o', { runId, runIdsMatch, hasAllFields }, runFields, fields)

return run.id === runId
return runIdsMatch && hasAllFields
}

return this.#poller.start({ meta: { runId, info }, filter })
Expand Down Expand Up @@ -279,3 +297,15 @@ const createCombinedFragment = (name: string, fragmentNames: string[], type: Gra
},
}
}

/**
* Get the field names for a given GraphQLResolveInfo
* @param info to extract field names from
*/
const getFields = (info: GraphQLResolveInfo) => {
const selections = info.fieldNodes[0]!.selectionSet?.selections!

const fields = selections.map((selection) => selection.kind === 'Field' && selection.name.value).filter((field): field is string => typeof field === 'string')

return fields
}
Expand Up @@ -2,7 +2,7 @@ import chai from 'chai'
import sinon from 'sinon'
import sinonChai from 'sinon-chai'
import debugLib from 'debug'
import { GraphQLString } from 'graphql'
import { GraphQLInt, GraphQLString } from 'graphql'

import { DataContext } from '../../../src'
import { createTestDataContext } from '../helper'
Expand Down Expand Up @@ -47,7 +47,7 @@ describe('RelevantRunSpecsDataSource', () => {
})

describe('polling', () => {
let clock
let clock: sinon.SinonFakeTimers

beforeEach(() => {
clock = sinon.useFakeTimers()
Expand All @@ -57,45 +57,51 @@ describe('RelevantRunSpecsDataSource', () => {
clock.restore()
})

const configureGraphQL = (cb: (info: Parameters<typeof dataSource.pollForSpecs>[1]) => any) => {
it('polls and emits changes', async () => {
const testData = FAKE_PROJECT_ONE_RUNNING_RUN_ONE_SPEC

//clone the fixture so it is not modified for future tests
const FAKE_PROJECT = JSON.parse(JSON.stringify(testData)) as typeof testData

const runId = testData.data.cloudNodesByIds[0].id

sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves(FAKE_PROJECT)

const query = `
query Test {
test {
value
value2
id
runNumber
completedInstanceCount
totalInstanceCount
totalTests
status
}
}
`

const fields = {
value: {
id: {
type: GraphQLString,
},
value2: {
runNumber: {
type: GraphQLString,
},
}

return createGraphQL(
query,
fields,
(source, args, context, info) => {
return cb(info)
completedInstanceCount: {
type: GraphQLInt,
},
)
}

it('polls and emits changes', async () => {
const testData = FAKE_PROJECT_ONE_RUNNING_RUN_ONE_SPEC

//clone the fixture so it is not modified for future tests
const FAKE_PROJECT = JSON.parse(JSON.stringify(testData)) as typeof testData

const runId = testData.data.cloudNodesByIds[0].id

sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves(FAKE_PROJECT)
totalInstanceCount: {
type: GraphQLInt,
},
totalTests: {
type: GraphQLInt,
},
status: {
type: GraphQLString,
},
}

return configureGraphQL(async (info) => {
return createGraphQL(query, fields, async (source, args, context, info) => {
const subscriptionIterator = dataSource.pollForSpecs(runId, info)

const firstEmit = await subscriptionIterator.next()
Expand Down Expand Up @@ -145,8 +151,12 @@ describe('RelevantRunSpecsDataSource', () => {
const expected = {
data: {
test: {
value: null,
value2: null,
completedInstanceCount: null,
id: null,
runNumber: null,
status: null,
totalInstanceCount: null,
totalTests: null,
},
},
}
Expand All @@ -155,14 +165,46 @@ describe('RelevantRunSpecsDataSource', () => {
})
})

it('should create query', () => {
const gqlStub = sinon.stub(ctx.cloud, 'executeRemoteGraphQL')
it('should create query', async () => {
const gqlStub = sinon.stub(ctx.cloud, 'executeRemoteGraphQL').resolves({ data: {} })

return configureGraphQL(async (info) => {
const subscriptionIterator = dataSource.pollForSpecs('runId', info)
const fields = {
value: {
type: GraphQLString,
},
value2: {
type: GraphQLString,
},
value3: {
type: GraphQLString,
},
}

subscriptionIterator.return(undefined)
}).then((result) => {
const query = `
query Test {
test {
value
value2
}
}
`

const query2 = `
query Test {
test {
value2
value3
}
}
`

let iterator1: ReturnType<RelevantRunSpecsDataSource['pollForSpecs']>
let iterator2: ReturnType<RelevantRunSpecsDataSource['pollForSpecs']>

return createGraphQL(query, fields, async (source, args, context, info) => {
iterator1 = dataSource.pollForSpecs('runId', info)
})
.then(() => {
const expected =
dedent`query RelevantRunSpecsDataSource_Specs($ids: [ID!]!) {
cloudNodesByIds(ids: $ids) {
Expand All @@ -187,7 +229,51 @@ describe('RelevantRunSpecsDataSource', () => {

expect(gqlStub).to.have.been.called
expect(gqlStub.firstCall.args[0]).to.haveOwnProperty('operation')
expect(gqlStub.firstCall.args[0].operation).to.eql(`${expected }\n`)
expect(gqlStub.firstCall.args[0].operation, 'should match initial query with one fragment').to.eql(`${expected }\n`)
})
.then(() => {
return createGraphQL(query2, fields, async (source, args, context, info) => {
iterator2 = dataSource.pollForSpecs('runId', info)

await clock.nextAsync()
})
})
.then(() => {
const expected =
dedent`query RelevantRunSpecsDataSource_Specs($ids: [ID!]!) {
cloudNodesByIds(ids: $ids) {
id
... on CloudRun {
...Subscriptions
}
}
pollingIntervals {
runByNumber
}
}

fragment Subscriptions on Test {
...Fragment0
...Fragment1
}

fragment Fragment0 on Test {
value
value2
}

fragment Fragment1 on Test {
value2
value3
}`

expect(gqlStub).to.have.been.calledTwice
expect(gqlStub.secondCall.args[0]).to.haveOwnProperty('operation')
expect(gqlStub.secondCall.args[0].operation, 'should match second query with two fragments').to.eql(`${expected }\n`)
})
.then(() => {
iterator1.return(undefined)
iterator2.return(undefined)
})
})
})
Expand Down