Merge pull request #32712 from overleaf/mg-recreate-corrupted-blob
Recover from corrupted blobs during hard resync GitOrigin-RevId: 7cc764e3bcc8557689c040c8f042991d97f897bc
This commit is contained in:
@@ -11,3 +11,4 @@ export class UpdateWithUnknownFormatError extends OError {}
|
||||
export class UnexpectedOpTypeError extends OError {}
|
||||
export class TooManyRequestsError extends OError {}
|
||||
export class NeedFullProjectStructureResyncError extends OError {}
|
||||
export class FileContentEmptyError extends OError {}
|
||||
|
||||
@@ -645,9 +645,11 @@ const resyncProjectSchema = z.object({
|
||||
}),
|
||||
query: z.object({
|
||||
force: z.stringbool().default(false),
|
||||
recoverCorruptedFiles: z.stringbool().default(false),
|
||||
}),
|
||||
body: z.object({
|
||||
force: z.boolean().default(false),
|
||||
recoverCorruptedFiles: z.boolean().default(false),
|
||||
origin: z
|
||||
.object({
|
||||
kind: z.string(),
|
||||
@@ -668,6 +670,9 @@ export function resyncProject(req, res, next) {
|
||||
options.historyRangesMigration = body.historyRangesMigration
|
||||
}
|
||||
if (query.force || body.force) {
|
||||
if (query.recoverCorruptedFiles || body.recoverCorruptedFiles) {
|
||||
options.recoverCorruptedFiles = true
|
||||
}
|
||||
// this will delete the queue and clear the sync state
|
||||
// use if the project is completely broken
|
||||
SyncManager.startHardResync(projectId, options, error => {
|
||||
|
||||
@@ -7,8 +7,13 @@ import Settings from '@overleaf/settings'
|
||||
import logger from '@overleaf/logger'
|
||||
import Metrics from '@overleaf/metrics'
|
||||
import OError from '@overleaf/o-error'
|
||||
import { File, Range } from 'overleaf-editor-core'
|
||||
import { File, Range, TextOperation } from 'overleaf-editor-core'
|
||||
import {
|
||||
TooLongError,
|
||||
UnprocessableError,
|
||||
} from 'overleaf-editor-core/lib/errors.js'
|
||||
import {
|
||||
FileContentEmptyError,
|
||||
NeedFullProjectStructureResyncError,
|
||||
SYNC_ONGOING_ERROR_MESSAGE,
|
||||
SyncError,
|
||||
@@ -81,7 +86,7 @@ async function startHardResync(projectId, options = {}) {
|
||||
await clearResyncState(projectId)
|
||||
await RedisManager.promises.clearFirstOpTimestamp(projectId)
|
||||
await RedisManager.promises.destroyDocUpdatesQueue(projectId)
|
||||
await startResyncWithoutLock(projectId, options)
|
||||
await startResyncWithoutLock(projectId, { ...options, hard: true })
|
||||
}
|
||||
)
|
||||
} catch (error) {
|
||||
@@ -138,6 +143,8 @@ async function startResyncWithoutLock(projectId, options) {
|
||||
}
|
||||
syncState.setOrigin(options.origin || { kind: 'history-resync' })
|
||||
syncState.startProjectStructureSync()
|
||||
syncState.hardResync = options.hard === true
|
||||
syncState.recoverCorruptedFiles = options.recoverCorruptedFiles === true
|
||||
|
||||
const webOpts = {}
|
||||
if (options.historyRangesMigration) {
|
||||
@@ -345,7 +352,9 @@ async function expandSyncUpdates(
|
||||
const expander = new SyncUpdateExpander(
|
||||
projectId,
|
||||
snapshotFiles,
|
||||
syncState.origin
|
||||
syncState.origin,
|
||||
syncState.hardResync,
|
||||
syncState.recoverCorruptedFiles
|
||||
)
|
||||
|
||||
// expand updates asynchronously to avoid blocking
|
||||
@@ -369,7 +378,9 @@ class SyncState {
|
||||
history,
|
||||
stuckClearCount,
|
||||
lastStuckClearAt,
|
||||
lastStuckDocPaths
|
||||
lastStuckDocPaths,
|
||||
hardResync,
|
||||
recoverCorruptedFiles
|
||||
) {
|
||||
this.projectId = projectId
|
||||
this.resyncProjectStructure = resyncProjectStructure
|
||||
@@ -382,6 +393,8 @@ class SyncState {
|
||||
this.stuckClearCount = stuckClearCount
|
||||
this.lastStuckClearAt = lastStuckClearAt
|
||||
this.lastStuckDocPaths = lastStuckDocPaths
|
||||
this.hardResync = hardResync || false
|
||||
this.recoverCorruptedFiles = recoverCorruptedFiles || false
|
||||
}
|
||||
|
||||
static fromRaw(projectId, rawSyncState) {
|
||||
@@ -414,6 +427,8 @@ class SyncState {
|
||||
const stuckClearCount = rawSyncState.stuckClearCount ?? 0
|
||||
const lastStuckClearAt = rawSyncState.lastStuckClearAt
|
||||
const lastStuckDocPaths = rawSyncState.lastStuckDocPaths
|
||||
const hardResync = rawSyncState.hardResync || false
|
||||
const recoverCorruptedFiles = rawSyncState.recoverCorruptedFiles || false
|
||||
return new SyncState(
|
||||
projectId,
|
||||
resyncProjectStructure,
|
||||
@@ -425,7 +440,9 @@ class SyncState {
|
||||
history,
|
||||
stuckClearCount,
|
||||
lastStuckClearAt,
|
||||
lastStuckDocPaths
|
||||
lastStuckDocPaths,
|
||||
hardResync,
|
||||
recoverCorruptedFiles
|
||||
)
|
||||
}
|
||||
|
||||
@@ -434,6 +451,8 @@ class SyncState {
|
||||
resyncProjectStructure: this.resyncProjectStructure,
|
||||
resyncDocContents: Array.from(this.resyncDocContents),
|
||||
origin: this.origin,
|
||||
hardResync: this.hardResync,
|
||||
recoverCorruptedFiles: this.recoverCorruptedFiles,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -562,13 +581,23 @@ class SyncUpdateExpander {
|
||||
*
|
||||
* @param {string} projectId
|
||||
* @param {Record<string, File>} snapshotFiles
|
||||
* @param {string} origin
|
||||
* @param {import('overleaf-editor-core/lib/types.js').RawOrigin} origin
|
||||
* @param {boolean} hardResync
|
||||
* @param {boolean} recoverCorruptedFiles
|
||||
*/
|
||||
constructor(projectId, snapshotFiles, origin) {
|
||||
constructor(
|
||||
projectId,
|
||||
snapshotFiles,
|
||||
origin,
|
||||
hardResync,
|
||||
recoverCorruptedFiles
|
||||
) {
|
||||
this.projectId = projectId
|
||||
this.files = snapshotFiles
|
||||
this.expandedUpdates = /** @type ProjectStructureUpdate[] */ []
|
||||
this.origin = origin
|
||||
this.hardResync = hardResync || false
|
||||
this.recoverCorruptedFiles = recoverCorruptedFiles || false
|
||||
}
|
||||
|
||||
// If there's an expected *file* with the same path and either the same hash
|
||||
@@ -888,18 +917,81 @@ class SyncUpdateExpander {
|
||||
|
||||
// compute the difference between the expected and persisted content
|
||||
const historyId = await WebApiManager.promises.getHistoryId(this.projectId)
|
||||
const file = await snapshotFile.load(
|
||||
'eager',
|
||||
HistoryStoreManager.getBlobStore(historyId)
|
||||
)
|
||||
const persistedContent = file.getContent()
|
||||
if (persistedContent == null) {
|
||||
// This should not happen given that we loaded the file eagerly. We could
|
||||
// probably refine the types in overleaf-editor-core so that this check
|
||||
// wouldn't be necessary.
|
||||
throw new Error('File was not properly loaded')
|
||||
|
||||
let file
|
||||
try {
|
||||
file = await snapshotFile.load(
|
||||
'eager',
|
||||
HistoryStoreManager.getBlobStore(historyId)
|
||||
)
|
||||
const persistedContent = file.getContent()
|
||||
if (persistedContent == null) {
|
||||
throw new FileContentEmptyError('File was not properly loaded')
|
||||
}
|
||||
} catch (err) {
|
||||
// When the recoverCorruptedFiles flag is set (requires hard resync),
|
||||
// recover from known data corruption errors by removing the file and
|
||||
// re-adding it from docstore. For soft resyncs, hard resyncs without
|
||||
// the flag, or transient errors, re-throw so the operation can be retried.
|
||||
// Also bail out if the expected content exceeds the max string length,
|
||||
// as the re-add would fail when applying text operations. This check
|
||||
// must happen here (not in isDataCorruptionError) because the original
|
||||
// error may be a different corruption type — excluding TooLongError
|
||||
// from detection would cause the file to be removed but not re-added.
|
||||
if (!this.recoverCorruptedFiles || !isDataCorruptionError(err)) {
|
||||
logger.error({
|
||||
name: 'failed to load file from history during resync',
|
||||
projectId: this.projectId,
|
||||
pathname,
|
||||
err,
|
||||
})
|
||||
throw err
|
||||
}
|
||||
if (expectedContent.length > TextOperation.MAX_STRING_LENGTH) {
|
||||
throw new TooLongError(null, expectedContent.length)
|
||||
.withInfo({
|
||||
projectId: this.projectId,
|
||||
pathname,
|
||||
maxLength: TextOperation.MAX_STRING_LENGTH,
|
||||
})
|
||||
.withCause(err)
|
||||
}
|
||||
|
||||
logger.warn(
|
||||
{ projectId: this.projectId, pathname, err },
|
||||
'failed to load file from history during hard resync, removing and re-adding from docstore'
|
||||
)
|
||||
Metrics.inc('project_history_resync_operation', 1, {
|
||||
status: 'recover corrupted file',
|
||||
})
|
||||
|
||||
this.expandedUpdates.push({
|
||||
pathname,
|
||||
new_pathname: '',
|
||||
meta: {
|
||||
resync: true,
|
||||
origin: this.origin,
|
||||
ts: update.meta.ts,
|
||||
},
|
||||
})
|
||||
this.expandedUpdates.push({
|
||||
pathname,
|
||||
doc: update.doc,
|
||||
docLines: expectedContent,
|
||||
meta: {
|
||||
resync: true,
|
||||
origin: this.origin,
|
||||
ts: update.meta.ts,
|
||||
},
|
||||
})
|
||||
// Replace in-memory file so the range sync below diffs the empty
|
||||
// persisted state against docstore ranges and restores them.
|
||||
this.files[pathname] = File.fromString(expectedContent)
|
||||
file = this.files[pathname]
|
||||
}
|
||||
|
||||
const persistedContent = /** @type {string} */ (file.getContent())
|
||||
|
||||
if (!hashesMatch) {
|
||||
const expandedUpdate = await this.queueUpdateForOutOfSyncContent(
|
||||
update,
|
||||
@@ -1439,6 +1531,44 @@ function trackingDirectivesEqual(a, b) {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines whether an error from loading a file's blob indicates data
|
||||
* corruption (safe to recover from by removing and re-adding the file) as
|
||||
* opposed to a transient infrastructure failure (which should be retried).
|
||||
*
|
||||
* Known corruption indicators:
|
||||
* - UnprocessableError (and subclasses ApplyError, InvalidInsertionError,
|
||||
* TooLongError): the stored operations are inconsistent with the blob content
|
||||
* - SyntaxError: the ranges blob contains invalid JSON
|
||||
* - FileContentEmptyError: blob loaded but returned null content
|
||||
*
|
||||
* @param {unknown} err
|
||||
* @returns {boolean}
|
||||
*/
|
||||
function isDataCorruptionError(err) {
|
||||
if (!(err instanceof Error)) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Operation apply failures (op/content mismatch). This covers ApplyError,
|
||||
// InvalidInsertionError, and TooLongError from overleaf-editor-core.
|
||||
if (err instanceof UnprocessableError) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Corrupted ranges blob (invalid JSON from BlobStore.getObject)
|
||||
if (err instanceof SyntaxError) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Null content after loading
|
||||
if (err instanceof FileContentEmptyError) {
|
||||
return true
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// EXPORTS
|
||||
|
||||
const cloneResyncStateCb = callbackify(cloneResyncState)
|
||||
|
||||
@@ -169,7 +169,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -263,7 +263,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
it('should skip HEAD on blob without hash', async function () {
|
||||
@@ -353,7 +353,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
it('should record error when checking blob fails with 500', async function () {
|
||||
@@ -453,7 +453,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
!addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been skipped`
|
||||
`/api/projects/${historyId}/legacy_changes should have been skipped`
|
||||
)
|
||||
})
|
||||
it('should skip blob write when blob exists', async function () {
|
||||
@@ -544,7 +544,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
it('should add file w/o url', async function () {
|
||||
@@ -635,7 +635,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
describe('with filestore disabled', function () {
|
||||
@@ -746,7 +746,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
!addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been skipped`
|
||||
`/api/projects/${historyId}/legacy_changes should have been skipped`
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -852,7 +852,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
)
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -921,7 +921,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
deleteFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -1008,7 +1008,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
|
||||
const project = await db.projects.findOne({
|
||||
@@ -1073,7 +1073,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1150,7 +1150,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1222,7 +1222,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1324,7 +1324,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
fixTrackedChange.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1419,11 +1419,360 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
fixTrackedChange.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe("when a doc's blob content is corrupted", function () {
|
||||
beforeEach(function () {
|
||||
MockHistoryStore()
|
||||
.get(`/api/projects/${historyId}/latest/history`)
|
||||
.reply(200, {
|
||||
chunk: {
|
||||
history: {
|
||||
snapshot: {
|
||||
files: {
|
||||
'main.tex': {
|
||||
hash: '0a207c060e61f3b88eaee0a8cd0696f46fb155eb',
|
||||
stringLength: 10,
|
||||
},
|
||||
},
|
||||
},
|
||||
changes: [
|
||||
{
|
||||
operations: [
|
||||
{
|
||||
pathname: 'main.tex',
|
||||
// Retain 10 chars, but blob content is only 3 chars.
|
||||
// This simulates a corrupted history where the
|
||||
// operation doesn't match the blob content, causing
|
||||
// an ApplyError when the file is loaded eagerly.
|
||||
textOperation: [10],
|
||||
},
|
||||
],
|
||||
timestamp: '2026-01-01T00:00:00.000Z',
|
||||
authors: [],
|
||||
v2Authors: [],
|
||||
},
|
||||
],
|
||||
},
|
||||
startVersion: 0,
|
||||
},
|
||||
})
|
||||
|
||||
// Blob returns valid content, but it doesn't match the operation
|
||||
MockHistoryStore()
|
||||
.get(
|
||||
`/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155eb`
|
||||
)
|
||||
.reply(200, 'a\nb')
|
||||
})
|
||||
|
||||
it('should remove and re-add the file during hard resync', async function () {
|
||||
const addFile = MockHistoryStore()
|
||||
.post(`/api/projects/${historyId}/legacy_changes`, body => {
|
||||
// The corrupted file is removed and re-added as two changes
|
||||
expect(body).to.have.length(2)
|
||||
// First change: remove the corrupted file
|
||||
expect(body[0].operations).to.have.length(1)
|
||||
expect(body[0].operations[0]).to.deep.include({
|
||||
pathname: 'main.tex',
|
||||
newPathname: '',
|
||||
})
|
||||
// Second change: re-add with new blob from docstore
|
||||
expect(body[1].operations).to.have.length(1)
|
||||
expect(body[1].operations[0].pathname).to.equal('main.tex')
|
||||
expect(body[1].operations[0].file).to.have.property('hash')
|
||||
return true
|
||||
})
|
||||
.query({ end_version: 1 })
|
||||
.reply(204)
|
||||
|
||||
// The re-added file needs its blob to be stored
|
||||
MockHistoryStore()
|
||||
.put(new RegExp(`/api/projects/${historyId}/blobs/`))
|
||||
.reply(201)
|
||||
|
||||
await ProjectHistoryClient.hardResyncHistory(this.project_id, {
|
||||
recoverCorruptedFiles: true,
|
||||
})
|
||||
|
||||
const update1 = {
|
||||
projectHistoryId: historyId,
|
||||
resyncProjectStructure: {
|
||||
docs: [{ path: '/main.tex' }],
|
||||
files: [],
|
||||
},
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update1)
|
||||
|
||||
const update2 = {
|
||||
path: '/main.tex',
|
||||
projectHistoryId: historyId,
|
||||
resyncDocContent: {
|
||||
content: 'hello world',
|
||||
},
|
||||
doc: this.doc_id,
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update2)
|
||||
|
||||
await ProjectHistoryClient.flushProject(this.project_id)
|
||||
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
sinon.assert.calledWithMatch(
|
||||
loggerWarn,
|
||||
sinon.match({
|
||||
projectId: this.project_id,
|
||||
pathname: 'main.tex',
|
||||
}),
|
||||
'failed to load file from history during hard resync, removing and re-adding from docstore'
|
||||
)
|
||||
})
|
||||
|
||||
it('should restore comments and tracked changes from docstore after hard resync re-add', async function () {
|
||||
const commentId = 'comment-id-1'
|
||||
|
||||
const addFile = MockHistoryStore()
|
||||
.post(`/api/projects/${historyId}/legacy_changes`, body => {
|
||||
// Expect three changes: remove, re-add, then comment sync
|
||||
expect(body).to.have.length(3)
|
||||
// First change: remove the corrupted file
|
||||
expect(body[0].operations).to.have.length(1)
|
||||
expect(body[0].operations[0]).to.deep.include({
|
||||
pathname: 'main.tex',
|
||||
newPathname: '',
|
||||
})
|
||||
// Second change: re-add with new blob from docstore
|
||||
expect(body[1].operations).to.have.length(1)
|
||||
expect(body[1].operations[0].pathname).to.equal('main.tex')
|
||||
expect(body[1].operations[0].file).to.have.property('hash')
|
||||
// Third change: restore comment from docstore ranges
|
||||
expect(body[2].operations).to.have.length(1)
|
||||
expect(body[2].operations[0]).to.deep.include({
|
||||
pathname: 'main.tex',
|
||||
commentId,
|
||||
})
|
||||
expect(body[2].operations[0].ranges).to.deep.equal([
|
||||
{ pos: 0, length: 5 },
|
||||
])
|
||||
return true
|
||||
})
|
||||
.query({ end_version: 1 })
|
||||
.reply(204)
|
||||
|
||||
// The re-added file needs its blob to be stored
|
||||
MockHistoryStore()
|
||||
.put(new RegExp(`/api/projects/${historyId}/blobs/`))
|
||||
.reply(201)
|
||||
|
||||
await ProjectHistoryClient.hardResyncHistory(this.project_id, {
|
||||
recoverCorruptedFiles: true,
|
||||
})
|
||||
|
||||
const update1 = {
|
||||
projectHistoryId: historyId,
|
||||
resyncProjectStructure: {
|
||||
docs: [{ path: '/main.tex' }],
|
||||
files: [],
|
||||
},
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update1)
|
||||
|
||||
const update2 = {
|
||||
path: '/main.tex',
|
||||
projectHistoryId: historyId,
|
||||
resyncDocContent: {
|
||||
content: 'hello world',
|
||||
ranges: {
|
||||
comments: [
|
||||
{
|
||||
id: commentId,
|
||||
op: {
|
||||
c: 'hello',
|
||||
p: 0,
|
||||
hpos: 0,
|
||||
hlen: 5,
|
||||
t: commentId,
|
||||
},
|
||||
meta: {
|
||||
user_id: 'user-id',
|
||||
ts: this.timestamp,
|
||||
},
|
||||
},
|
||||
],
|
||||
changes: [],
|
||||
},
|
||||
},
|
||||
doc: this.doc_id,
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update2)
|
||||
|
||||
await ProjectHistoryClient.flushProject(this.project_id)
|
||||
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
it('should propagate the error during soft resync', async function () {
|
||||
await ProjectHistoryClient.resyncHistory(this.project_id)
|
||||
|
||||
const update1 = {
|
||||
projectHistoryId: historyId,
|
||||
resyncProjectStructure: {
|
||||
docs: [{ path: '/main.tex' }],
|
||||
files: [],
|
||||
},
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update1)
|
||||
|
||||
const update2 = {
|
||||
path: '/main.tex',
|
||||
projectHistoryId: historyId,
|
||||
resyncDocContent: {
|
||||
content: 'hello world',
|
||||
},
|
||||
doc: this.doc_id,
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update2)
|
||||
|
||||
// Soft resync should fail — the error is propagated so it can be
|
||||
// retried later (the blob store may be temporarily unavailable).
|
||||
const { statusCode } = await ProjectHistoryClient.flushProject(
|
||||
this.project_id,
|
||||
{ allowErrors: true }
|
||||
)
|
||||
expect(statusCode).to.equal(500)
|
||||
})
|
||||
|
||||
it('should propagate the error during hard resync without recoverCorruptedFiles flag', async function () {
|
||||
await ProjectHistoryClient.hardResyncHistory(this.project_id)
|
||||
|
||||
const update1 = {
|
||||
projectHistoryId: historyId,
|
||||
resyncProjectStructure: {
|
||||
docs: [{ path: '/main.tex' }],
|
||||
files: [],
|
||||
},
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update1)
|
||||
|
||||
const update2 = {
|
||||
path: '/main.tex',
|
||||
projectHistoryId: historyId,
|
||||
resyncDocContent: {
|
||||
content: 'hello world',
|
||||
},
|
||||
doc: this.doc_id,
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update2)
|
||||
|
||||
// Hard resync without the recoverCorruptedFiles flag should not
|
||||
// recover — recovery is too destructive to be the default behaviour.
|
||||
const { statusCode } = await ProjectHistoryClient.flushProject(
|
||||
this.project_id,
|
||||
{ allowErrors: true }
|
||||
)
|
||||
expect(statusCode).to.equal(500)
|
||||
})
|
||||
})
|
||||
|
||||
describe("when a doc's blob store is temporarily unavailable", function () {
|
||||
beforeEach(function () {
|
||||
MockHistoryStore()
|
||||
.get(`/api/projects/${historyId}/latest/history`)
|
||||
.reply(200, {
|
||||
chunk: {
|
||||
history: {
|
||||
snapshot: {
|
||||
files: {
|
||||
'main.tex': {
|
||||
hash: '0a207c060e61f3b88eaee0a8cd0696f46fb155eb',
|
||||
stringLength: 3,
|
||||
},
|
||||
},
|
||||
},
|
||||
changes: [],
|
||||
},
|
||||
startVersion: 0,
|
||||
},
|
||||
})
|
||||
|
||||
// Blob returns a 500 simulating a transient server error
|
||||
MockHistoryStore()
|
||||
.get(
|
||||
`/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155eb`
|
||||
)
|
||||
.reply(500, 'Internal Server Error')
|
||||
})
|
||||
|
||||
it('should propagate transient errors even during hard resync', async function () {
|
||||
await ProjectHistoryClient.hardResyncHistory(this.project_id)
|
||||
|
||||
const update1 = {
|
||||
projectHistoryId: historyId,
|
||||
resyncProjectStructure: {
|
||||
docs: [{ path: '/main.tex' }],
|
||||
files: [],
|
||||
},
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update1)
|
||||
|
||||
const update2 = {
|
||||
path: '/main.tex',
|
||||
projectHistoryId: historyId,
|
||||
resyncDocContent: {
|
||||
content: 'hello world',
|
||||
},
|
||||
doc: this.doc_id,
|
||||
meta: {
|
||||
ts: this.timestamp,
|
||||
},
|
||||
}
|
||||
await ProjectHistoryClient.pushRawUpdate(this.project_id, update2)
|
||||
|
||||
// Even hard resync should not recover from transient errors (5xx).
|
||||
// The error should propagate so it can be retried.
|
||||
const { statusCode } = await ProjectHistoryClient.flushProject(
|
||||
this.project_id,
|
||||
{ allowErrors: true }
|
||||
)
|
||||
expect(statusCode).to.equal(500)
|
||||
})
|
||||
})
|
||||
|
||||
describe("when a doc's ranges are out of sync", function () {
|
||||
const commentId = 'comment-id'
|
||||
beforeEach(function () {
|
||||
@@ -1580,7 +1929,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1682,7 +2031,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1771,7 +2120,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1861,7 +2210,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addComment.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1977,7 +2326,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
fixTrackedChange.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -2081,7 +2430,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
fixTrackedChange.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
|
||||
@@ -2199,7 +2548,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
fixTrackedChange.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -2292,7 +2641,7 @@ describe('Syncing with web and doc-updater', function () {
|
||||
|
||||
assert(
|
||||
addFile.isDone(),
|
||||
`/api/projects/${historyId}/changes should have been called`
|
||||
`/api/projects/${historyId}/legacy_changes should have been called`
|
||||
)
|
||||
assert(
|
||||
!docContentRequest.isDone(),
|
||||
|
||||
@@ -164,6 +164,24 @@ export async function resyncHistory(projectId) {
|
||||
expect(response.status).to.equal(204)
|
||||
}
|
||||
|
||||
export async function hardResyncHistory(
|
||||
projectId,
|
||||
{ recoverCorruptedFiles } = {}
|
||||
) {
|
||||
const params = new URLSearchParams({ force: 'true' })
|
||||
if (recoverCorruptedFiles) {
|
||||
params.set('recoverCorruptedFiles', 'true')
|
||||
}
|
||||
const response = await fetchNothing(
|
||||
`http://127.0.0.1:3054/project/${projectId}/resync?${params}`,
|
||||
{
|
||||
method: 'POST',
|
||||
json: { origin: { kind: 'test-origin' } },
|
||||
}
|
||||
)
|
||||
expect(response.status).to.equal(204)
|
||||
}
|
||||
|
||||
export async function createLabel(
|
||||
projectId,
|
||||
userId,
|
||||
|
||||
@@ -3,7 +3,9 @@ import { expect } from 'chai'
|
||||
import mongodb from 'mongodb-legacy'
|
||||
import tk from 'timekeeper'
|
||||
import { File, Comment, TrackedChange, Range } from 'overleaf-editor-core'
|
||||
import { UnprocessableError } from 'overleaf-editor-core/lib/errors.js'
|
||||
import { strict as esmock } from 'esmock'
|
||||
import { FileContentEmptyError } from '../../../../app/js/Errors.js'
|
||||
const { ObjectId } = mongodb
|
||||
|
||||
const MODULE_PATH = '../../../../app/js/SyncManager.js'
|
||||
@@ -144,20 +146,29 @@ describe('SyncManager', function () {
|
||||
},
|
||||
}
|
||||
|
||||
this.SyncManager = await esmock(MODULE_PATH, {
|
||||
'../../../../app/js/LockManager.js': this.LockManager,
|
||||
'../../../../app/js/UpdateCompressor.js': this.UpdateCompressor,
|
||||
'../../../../app/js/UpdateTranslator.js': this.UpdateTranslator,
|
||||
'../../../../app/js/mongodb.js': { ObjectId, db: this.db },
|
||||
'../../../../app/js/WebApiManager.js': this.WebApiManager,
|
||||
'../../../../app/js/ErrorRecorder.js': this.ErrorRecorder,
|
||||
'../../../../app/js/RedisManager.js': this.RedisManager,
|
||||
'../../../../app/js/SnapshotManager.js': this.SnapshotManager,
|
||||
'../../../../app/js/HistoryStoreManager.js': this.HistoryStoreManager,
|
||||
'../../../../app/js/HashManager.js': this.HashManager,
|
||||
'@overleaf/metrics': this.Metrics,
|
||||
'@overleaf/settings': this.Settings,
|
||||
})
|
||||
this.SyncManager = await esmock(
|
||||
MODULE_PATH,
|
||||
{
|
||||
'../../../../app/js/LockManager.js': this.LockManager,
|
||||
'../../../../app/js/UpdateCompressor.js': this.UpdateCompressor,
|
||||
'../../../../app/js/UpdateTranslator.js': this.UpdateTranslator,
|
||||
'../../../../app/js/mongodb.js': { ObjectId, db: this.db },
|
||||
'../../../../app/js/WebApiManager.js': this.WebApiManager,
|
||||
'../../../../app/js/ErrorRecorder.js': this.ErrorRecorder,
|
||||
'../../../../app/js/RedisManager.js': this.RedisManager,
|
||||
'../../../../app/js/SnapshotManager.js': this.SnapshotManager,
|
||||
'../../../../app/js/HistoryStoreManager.js': this.HistoryStoreManager,
|
||||
'../../../../app/js/HashManager.js': this.HashManager,
|
||||
'@overleaf/metrics': this.Metrics,
|
||||
'@overleaf/settings': this.Settings,
|
||||
},
|
||||
{
|
||||
'overleaf-editor-core/lib/errors.js':
|
||||
await import('overleaf-editor-core/lib/errors.js'),
|
||||
'../../../../app/js/Errors.js':
|
||||
await import('../../../../app/js/Errors.js'),
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
afterEach(function () {
|
||||
@@ -533,6 +544,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: ['new.tex'],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -549,6 +562,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: ['new.tex'],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -566,6 +581,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: [],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -585,6 +602,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: ['new.tex'],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -608,6 +627,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: [],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
|
||||
@@ -632,6 +653,8 @@ describe('SyncManager', function () {
|
||||
resyncProjectStructure: false,
|
||||
resyncDocContents: [],
|
||||
origin: { kind: 'history-resync' },
|
||||
hardResync: false,
|
||||
recoverCorruptedFiles: false,
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1571,6 +1594,116 @@ describe('SyncManager', function () {
|
||||
expect(this.extendLock).to.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('when the blob content is corrupted during hard resync', function () {
|
||||
describe('and the recoverCorruptedFiles flag is true', function () {
|
||||
beforeEach(function () {
|
||||
this.syncState.recoverCorruptedFiles = true
|
||||
})
|
||||
|
||||
async function assertRecovery(ctx, err) {
|
||||
ctx.fileMap['main.tex'].load.rejects(err)
|
||||
const updates = [
|
||||
resyncProjectStructureUpdate(
|
||||
[ctx.persistedDoc],
|
||||
[ctx.persistedFile]
|
||||
),
|
||||
docContentSyncUpdate(ctx.persistedDoc, ctx.persistedDocContent),
|
||||
]
|
||||
const expandedUpdates =
|
||||
await ctx.SyncManager.promises.expandSyncUpdates(
|
||||
ctx.projectId,
|
||||
ctx.historyId,
|
||||
ctx.mostRecentChunk,
|
||||
updates,
|
||||
ctx.extendLock
|
||||
)
|
||||
expect(expandedUpdates).to.deep.equal([
|
||||
{
|
||||
pathname: 'main.tex',
|
||||
new_pathname: '',
|
||||
meta: {
|
||||
resync: true,
|
||||
origin: { kind: 'history-resync' },
|
||||
ts: TIMESTAMP,
|
||||
},
|
||||
},
|
||||
{
|
||||
pathname: 'main.tex',
|
||||
doc: ctx.persistedDoc.doc,
|
||||
docLines: ctx.persistedDocContent,
|
||||
meta: {
|
||||
resync: true,
|
||||
origin: { kind: 'history-resync' },
|
||||
ts: TIMESTAMP,
|
||||
},
|
||||
},
|
||||
])
|
||||
}
|
||||
|
||||
it('queues REMOVE + ADD for UnprocessableError', async function () {
|
||||
await assertRecovery(this, new UnprocessableError('apply failed'))
|
||||
})
|
||||
|
||||
it('queues REMOVE + ADD for SyntaxError', async function () {
|
||||
await assertRecovery(this, new SyntaxError('bad JSON'))
|
||||
})
|
||||
|
||||
it('queues REMOVE + ADD for FileContentEmptyError', async function () {
|
||||
await assertRecovery(
|
||||
this,
|
||||
new FileContentEmptyError('null content')
|
||||
)
|
||||
})
|
||||
|
||||
it('propagates non-corruption errors during hard resync', async function () {
|
||||
this.fileMap['main.tex'].load.rejects(
|
||||
new Error('connection timeout')
|
||||
)
|
||||
const updates = [
|
||||
resyncProjectStructureUpdate(
|
||||
[this.persistedDoc],
|
||||
[this.persistedFile]
|
||||
),
|
||||
docContentSyncUpdate(this.persistedDoc, this.persistedDocContent),
|
||||
]
|
||||
await expect(
|
||||
this.SyncManager.promises.expandSyncUpdates(
|
||||
this.projectId,
|
||||
this.historyId,
|
||||
this.mostRecentChunk,
|
||||
updates,
|
||||
this.extendLock
|
||||
)
|
||||
).to.be.rejectedWith('connection timeout')
|
||||
})
|
||||
})
|
||||
|
||||
describe('and the recoverCorruptedFiles flag is false', function () {
|
||||
it('propagates corruption errors', async function () {
|
||||
this.syncState.recoverCorruptedFiles = false
|
||||
this.fileMap['main.tex'].load.rejects(
|
||||
new UnprocessableError('apply failed')
|
||||
)
|
||||
const updates = [
|
||||
resyncProjectStructureUpdate(
|
||||
[this.persistedDoc],
|
||||
[this.persistedFile]
|
||||
),
|
||||
docContentSyncUpdate(this.persistedDoc, this.persistedDocContent),
|
||||
]
|
||||
await expect(
|
||||
this.SyncManager.promises.expandSyncUpdates(
|
||||
this.projectId,
|
||||
this.historyId,
|
||||
this.mostRecentChunk,
|
||||
updates,
|
||||
this.extendLock
|
||||
)
|
||||
).to.be.rejectedWith('apply failed')
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('syncing comments', function () {
|
||||
|
||||
Reference in New Issue
Block a user