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

[stable27] Fix sync errors after network issues #4486

Merged
merged 12 commits into from Jul 11, 2023
Merged
165 changes: 165 additions & 0 deletions cypress/e2e/api/SyncServiceProvider.spec.js
@@ -0,0 +1,165 @@
/*
* @copyright Copyright (c) 2023 Max <max@nextcloud.com>
*
* @author Max <max@nextcloud.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

import { randUser } from '../../utils/index.js'
import { SyncService } from '../../../src/services/SyncService.js'
import createSyncServiceProvider from '../../../src/services/SyncServiceProvider.js'
import { Doc } from 'yjs'

const user = randUser()

describe('Sync service provider', function() {
let fileId

before(function() {
cy.createUser(user)
window.OC = {
config: { modRewriteWorking: false },
webroot: '',
}
})

beforeEach(function() {
cy.login(user)
cy.prepareSessionApi()
cy.uploadTestFile('test.md')
.then(id => {
fileId = id
})
cy.wrap(new Doc()).as('source')
cy.wrap(new Doc()).as('target')
cy.get('@source').then(source => createProvider(source)).as('sourceProvider')
cy.get('@target').then(target => createProvider(target)).as('targetProvider')
})

afterEach(function() {
this.sourceProvider?.destroy()
this.targetProvider?.destroy()
})

/**
* @param {object} ydoc Yjs document
*/
function createProvider(ydoc) {
const syncService = new SyncService({
serialize: () => 'Serialized',
getDocumentState: () => null,
})
syncService.on('opened', () => syncService.startSync())
return createSyncServiceProvider({
ydoc,
syncService,
fileId,
initialSession: null,
disableBc: true,
})
}

it('recovers from a dropped message', function() {
const sourceMap = this.source.getMap()
const targetMap = this.target.getMap()
cy.intercept({ method: 'POST', url: '**/apps/text/session/push' })
.as('push')
cy.intercept({ method: 'POST', url: '**/apps/text/session/sync' })
.as('sync')
cy.wait('@push')
cy.then(() => {
sourceMap.set('keyA', 'valueA')
expect(targetMap.get('keyB')).to.be.eq(undefined)
})
cy.wait('@sync')
cy.wait('@sync')
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(1000)
cy.then(() => {
expect(targetMap.get('keyA')).to.be.eq('valueA')
})
cy.intercept({
method: 'POST',
url: '**/apps/text/session/push',
}, req => {
if (req.body.steps) {
req.reply({ forceNetworkError: true })
req.alias = 'dead'
} else {
req.continue()
}
})
cy.then(() => {
sourceMap.set('keyB', 'valueB')
expect(targetMap.get('keyB')).to.be.eq(undefined)
})
cy.wait('@dead')
cy.then(() => {
expect(targetMap.get('keyB')).to.be.eq(undefined)
})
cy.intercept({
method: 'POST',
url: '**/apps/text/session/push',
}, req => {
if (req.body.steps) {
req.alias = 'alive'
req.continue()
} else {
req.continue()
}
})
cy.then(() => {
sourceMap.set('keyC', 'valueC')
expect(targetMap.get('keyB')).to.be.eq(undefined)
})
cy.wait('@alive')
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(1000)
cy.then(() => {
expect(targetMap.get('keyC')).to.be.eq('valueC')
expect(targetMap.get('keyB')).to.be.eq('valueB')
})
})

/*
* Counts the amount of push and sync requests in one minute.
* Skipped per default, useful for comparison before/after changes to SyncProvider or PollingBackend.
*/
it.skip('is not too chatty', function() {
const sourceMap = this.source.getMap()
const targetMap = this.target.getMap()
cy.intercept({ method: 'POST', url: '**/apps/text/session/push' })
.as('push')
cy.intercept({ method: 'POST', url: '**/apps/text/session/sync' })
.as('sync')
cy.wait('@push')
cy.then(() => {
sourceMap.set('keyA', 'valueA')
expect(targetMap.get('keyB')).to.be.eq(undefined)
})
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(60000)
cy.then(() => {
expect(targetMap.get('keyA')).to.be.eq('valueA')
})
// 2 clients push awareness updates every 15 seconds -> 2*5 = 10. Actual 15.
cy.get('@push.all').its('length').should('be.lessThan', 30)
// 2 clients sync fast first and then every 5 seconds -> 2*12 = 24. Actual 32.
cy.get('@sync.all').its('length').should('be.lessThan', 60)
})
})
61 changes: 61 additions & 0 deletions cypress/e2e/api/Yjs.spec.js
@@ -0,0 +1,61 @@
/*
* @copyright Copyright (c) 2023 Jonas <jonas@nextcloud.com>
*
* @author Jonas <jonas@nextcloud.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

import { applyUpdate, Doc, encodeStateAsUpdate, encodeStateVector } from 'yjs'

describe('Yjs', function() {
// Only tests that Yjs allows to apply steps in wrong order
it('applies step in wrong order', function() {
const source = new Doc()
const target = new Doc()
const sourceMap = source.getMap()
const targetMap = target.getMap()

target.on('afterTransaction', (tr, doc) => {
// console.log('afterTransaction', tr)
})

// Add keyA to source and apply to target
sourceMap.set('keyA', 'valueA')
const update0A = encodeStateAsUpdate(source)
const sourceVectorA = encodeStateVector(source)
applyUpdate(target, update0A)
expect(targetMap.get('keyA')).to.be.eq('valueA')

// Add keyB to source, don't apply to target yet
sourceMap.set('keyB', 'valueB')
const updateAB = encodeStateAsUpdate(source, sourceVectorA)
const sourceVectorB = encodeStateVector(source)

// Add keyC to source, apply to target
sourceMap.set('keyC', 'valueC')
const updateBC = encodeStateAsUpdate(source, sourceVectorB)
applyUpdate(target, updateBC)
expect(targetMap.get('keyB')).to.be.eq(undefined)
expect(targetMap.get('keyC')).to.be.eq(undefined)

// Apply keyB to target
applyUpdate(target, updateAB)
expect(targetMap.get('keyB')).to.be.eq('valueB')
expect(targetMap.get('keyC')).to.be.eq('valueC')
})
})
2 changes: 1 addition & 1 deletion cypress/e2e/sync.spec.js
Expand Up @@ -77,7 +77,7 @@ describe('Sync', () => {
}
}).as('sessionRequests')
cy.wait('@dead', { timeout: 30000 })
cy.get('#editor-container .document-status', { timeout: 10000 })
cy.get('#editor-container .document-status', { timeout: 30000 })
.should('contain', 'File could not be loaded')
.then(() => {
count = 4
Expand Down
4 changes: 2 additions & 2 deletions js/editor.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/editor.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/files-modal.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion js/files-modal.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions js/text-editors.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-editors.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-files.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-files.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-public.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-public.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-text.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-text.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-viewer.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-viewer.js.map

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions src/components/Editor.vue
Expand Up @@ -586,11 +586,7 @@ export default {
}
if (type === ERROR_TYPE.CONNECTION_FAILED && !this.hasConnectionIssue) {
this.hasConnectionIssue = true
// FIXME: ideally we just try to reconnect in the service, so we don't loose steps
OC.Notification.showTemporary('Connection failed, reconnecting')
if (data.retry !== false) {
setTimeout(this.reconnect.bind(this), 5000)
}
OC.Notification.showTemporary('Connection failed.')
}
if (type === ERROR_TYPE.SOURCE_NOT_FOUND) {
this.hasConnectionIssue = true
Expand Down