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

[stable30] fix: Ensure WebsocketPolyfill always has the latest session state and version #6241

Merged
merged 1 commit into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ export default {
},

onSync({ steps, document }) {
this.hasConnectionIssue = !this.$providers[0].wsconnected || this.$syncService.pushError > 0
this.hasConnectionIssue = this.$syncService.backend.fetcher === 0 || !this.$providers[0].wsconnected || this.$syncService.pushError > 0
this.$nextTick(() => {
this.emit('sync-service:sync')
})
Expand Down
27 changes: 16 additions & 11 deletions src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,19 @@ class SyncService {
return this.#connection && !this.#connection.isClosed
}

get connectionState() {
if (!this.#connection || this.version === undefined) {
return null
}
return {
...this.#connection.state,
version: this.version,
}
}

async open({ fileId, initialSession }) {
if (this.hasActiveConnection) {
// We're already connected.
return
return this.connectionState
}
const connect = initialSession
? Promise.resolve(new Connection({ data: initialSession }, {}))
Expand All @@ -106,19 +115,15 @@ class SyncService {
this.#connection = await connect
if (!this.#connection) {
// Error was already emitted in connect
return
return null
}
this.backend = new PollingBackend(this, this.#connection)
this.version = this.#connection.docStateVersion
this.baseVersionEtag = this.#connection.document.baseVersionEtag
this.emit('opened', {
...this.#connection.state,
version: this.version,
})
this.emit('loaded', {
...this.#connection.state,
version: this.version,
})
this.emit('opened', this.connectionState)
this.emit('loaded', this.connectionState)

return this.connectionState
}

startSync() {
Expand Down
17 changes: 11 additions & 6 deletions src/services/WebSocketPolyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,11 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
this.#notifyPushBus?.on('notify_push', this.#onNotifyPush.bind(this))
this.url = url
logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession })
if (syncService.hasActiveConnection) {
setTimeout(() => this.onopen?.(), 0)
}
this.#registerHandlers({
opened: ({ version, session }) => {
this.#version = version
logger.debug('opened ', { version, session })
this.#version = version
this.#session = session
this.onopen?.()
},
loaded: ({ version, session, content }) => {
logger.debug('loaded ', { version, session })
Expand All @@ -60,7 +56,16 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
}
},
})
syncService.open({ fileId, initialSession })

syncService.open({ fileId, initialSession }).then((data) => {
if (syncService.hasActiveConnection) {
const { version, session } = data
this.#version = version
this.#session = session

this.onopen?.()
}
})
}

#registerHandlers(handlers) {
Expand Down
17 changes: 10 additions & 7 deletions src/tests/services/WebsocketPolyfill.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@ import initWebSocketPolyfill from '../../services/WebSocketPolyfill.js'
describe('Init function', () => {

it('returns a websocket polyfill class', () => {
const syncService = { on: jest.fn(), open: jest.fn() }
const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) }
const Polyfill = initWebSocketPolyfill(syncService)
const websocket = new Polyfill('url')
expect(websocket).toBeInstanceOf(Polyfill)
})

it('registers handlers', () => {
const syncService = { on: jest.fn(), open: jest.fn() }
const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) }
const Polyfill = initWebSocketPolyfill(syncService)
const websocket = new Polyfill('url')
expect(syncService.on).toHaveBeenCalled()
})

it('opens sync service', () => {
const syncService = { on: jest.fn(), open: jest.fn() }
const syncService = { on: jest.fn(), open: jest.fn(() => Promise.resolve({ version: 123, session: {} })) }
const fileId = 123
const initialSession = { }
const Polyfill = initWebSocketPolyfill(syncService, fileId, initialSession)
Expand All @@ -33,7 +33,7 @@ describe('Init function', () => {
it('sends steps to sync service', async () => {
const syncService = {
on: jest.fn(),
open: jest.fn(),
open: jest.fn(() => Promise.resolve({ version: 123, session: {} })),
sendSteps: async getData => getData(),
}
const queue = [ 'initial' ]
Expand All @@ -51,9 +51,10 @@ describe('Init function', () => {
})

it('handles early reject', async () => {
jest.spyOn(console, 'error').mockImplementation(() => {})
const syncService = {
on: jest.fn(),
open: jest.fn(),
open: jest.fn(() => Promise.resolve({ version: 123, session: {} })),
sendSteps: jest.fn().mockRejectedValue('error before reading steps in sync service'),
}
const queue = [ 'initial' ]
Expand All @@ -69,9 +70,10 @@ describe('Init function', () => {
})

it('handles reject after reading data', async () => {
jest.spyOn(console, 'error').mockImplementation(() => {})
const syncService = {
on: jest.fn(),
open: jest.fn(),
open: jest.fn(() => Promise.resolve({ version: 123, session: {} })),
sendSteps: jest.fn().mockImplementation( async getData => {
getData()
throw 'error when sending in sync service'
Expand All @@ -90,9 +92,10 @@ describe('Init function', () => {
})

it('queue survives a close', async () => {
jest.spyOn(console, 'error').mockImplementation(() => {})
const syncService = {
on: jest.fn(),
open: jest.fn(),
open: jest.fn(() => Promise.resolve({ version: 123, session: {} })),
sendSteps: jest.fn().mockImplementation( async getData => {
getData()
throw 'error when sending in sync service'
Expand Down
Loading