Skip to content

Commit e8502fa

Browse files
robertjbassPatrikKozakJarrodMFlesch
authored
fix: handle concurrent autosave write conflicts gracefully (#16216)
### What? Two-part fix for intermittent 500s during autosave on serverless environments. 1. `updateLatestVersion` catches errors from `updateVersion`/`updateGlobalVersion` and checks whether a concurrent write already succeeded before falling back to `createVersion` 2. `saveVersion` re-throws instead of `return undefined!` so genuine failures still surface as real errors ### Why? On serverless, concurrent autosave requests race to update the same version row. The losing request gets a DB error. Previously `saveVersion` caught it and returned `undefined!`, which was assigned to `result`. Downstream field hooks then accessed properties on `undefined`: ``` TypeError: Cannot read properties of undefined (reading '_status') ``` ### How? When `updateVersion` fails, we re-query the row and check if `updatedAt` has moved — if it has, a concurrent request already committed and we return that result. If not, we fall back to `createVersion`. If that also fails, `saveVersion` re-throws. Fixes #16217 --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1213985147428664 --------- Co-authored-by: Patrik Kozak <35232443+PatrikKozak@users.noreply.github.com> Co-authored-by: Jarrod Flesch <jarrodmflesch@gmail.com>
1 parent b332bff commit e8502fa

File tree

3 files changed

+139
-10
lines changed

3 files changed

+139
-10
lines changed

packages/payload/src/versions/saveVersion.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ export async function saveVersion<TData extends JsonObject = JsonObject>({
134134
errorMessage = `There was an error while saving a version for the global ${typeof global.label === 'string' ? global.label : global.slug}.`
135135
}
136136
payload.logger.error({ err, msg: errorMessage })
137-
return undefined!
137+
throw err
138138
}
139139

140140
const max = getVersionsMax(collection || global!)

packages/payload/src/versions/updateLatestVersion.ts

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,64 @@ export async function updateLatestVersion<TData extends JsonObject>({
7676
},
7777
}
7878

79-
if (collection) {
80-
return await payload.db.updateVersion<TData>({
79+
let versionUpdateFailed: boolean | undefined = undefined
80+
81+
try {
82+
if (collection) {
83+
return await payload.db.updateVersion<TData>({
84+
...updateVersionArgs,
85+
collection: collection.slug,
86+
req,
87+
})
88+
}
89+
90+
return await payload.db.updateGlobalVersion<TData>({
8191
...updateVersionArgs,
82-
collection: collection.slug,
92+
global: global!.slug,
8393
req,
8494
})
95+
} catch (err) {
96+
versionUpdateFailed = true
97+
payload.logger.warn({
98+
err,
99+
msg: `Failed to update latest version — checking if a concurrent write already succeeded.`,
100+
})
85101
}
86102

87-
return await payload.db.updateGlobalVersion<TData>({
88-
...updateVersionArgs,
89-
global: global!.slug,
90-
req,
91-
})
103+
if (versionUpdateFailed) {
104+
// If a concurrent request already committed, return its result to avoid a duplicate version.
105+
// If updatedAt is unchanged, the update genuinely failed — fall back to createVersion.
106+
try {
107+
let freshDocs: JsonObject[]
108+
109+
if (collection) {
110+
;({ docs: freshDocs } = await payload.db.findVersions<TData>({
111+
collection: collection.slug,
112+
limit: 1,
113+
pagination: false,
114+
req,
115+
sort: '-updatedAt',
116+
where: { parent: { equals: id } },
117+
}))
118+
} else {
119+
;({ docs: freshDocs } = await payload.db.findGlobalVersions<TData>({
120+
global: global!.slug,
121+
limit: 1,
122+
pagination: false,
123+
req,
124+
sort: '-updatedAt',
125+
}))
126+
}
127+
128+
const [freshVersion] = freshDocs
129+
130+
if (freshVersion && new Date(freshVersion.updatedAt) > new Date(latestVersion.updatedAt)) {
131+
return freshVersion
132+
}
133+
} catch {
134+
// If the follow-up query also fails, fall through to createVersion
135+
}
136+
}
137+
138+
return undefined
92139
}

test/versions/int.spec.ts

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { createLocalReq, saveVersion, ValidationError } from 'payload'
66
import { wait } from 'payload/shared'
77
import * as qs from 'qs-esm'
88
import { fileURLToPath } from 'url'
9-
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'
9+
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
1010

1111
import type { NextRESTClient } from '../__helpers/shared/NextRESTClient.js'
1212
import type { AutosaveMultiSelectPost, DraftPost } from './payload-types.js'
@@ -1738,6 +1738,88 @@ describe('Versions', () => {
17381738
payload,
17391739
})
17401740
})
1741+
1742+
it('should fall back to creating a new version when updateVersion fails due to a concurrent write', async () => {
1743+
const doc = await payload.create({
1744+
collection: autosaveCollectionSlug,
1745+
data: { title: 'original', _status: 'draft' },
1746+
draft: true,
1747+
})
1748+
1749+
// Establish an existing autosave version so updateLatestVersion has something to update
1750+
await payload.update({
1751+
id: doc.id,
1752+
autosave: true,
1753+
collection: autosaveCollectionSlug,
1754+
data: { title: 'first autosave' },
1755+
draft: true,
1756+
})
1757+
1758+
const spy = vi
1759+
.spyOn(payload.db, 'updateVersion')
1760+
.mockRejectedValueOnce(new Error('concurrent update conflict'))
1761+
1762+
// Should not throw — updateLatestVersion catches the error and saveVersion falls back to createVersion
1763+
const result = await payload.update({
1764+
id: doc.id,
1765+
autosave: true,
1766+
collection: autosaveCollectionSlug,
1767+
data: { title: 'second autosave' },
1768+
draft: true,
1769+
})
1770+
1771+
spy.mockRestore()
1772+
1773+
expect(result.title).toBe('second autosave')
1774+
1775+
// A new version was created as fallback instead of the in-place update
1776+
const { totalDocs } = await payload.countVersions({
1777+
collection: autosaveCollectionSlug,
1778+
where: { parent: { equals: doc.id } },
1779+
})
1780+
1781+
// create → 1 version, first autosave updates in place → still 1 version on autosave collection (it creates a new autosave),
1782+
// second autosave failed update → fell back to create → one extra version
1783+
expect(totalDocs).toBeGreaterThan(1)
1784+
1785+
await cleanupDocuments({
1786+
collectionSlugs: [autosaveCollectionSlug],
1787+
payload,
1788+
})
1789+
})
1790+
1791+
it('should propagate the error when createVersion also fails', async () => {
1792+
const doc = await payload.create({
1793+
collection: autosaveCollectionSlug,
1794+
data: { title: 'original', _status: 'draft' },
1795+
draft: true,
1796+
})
1797+
1798+
const updateVersionSpy = vi
1799+
.spyOn(payload.db, 'updateVersion')
1800+
.mockRejectedValueOnce(new Error('concurrent update conflict'))
1801+
const createVersionSpy = vi
1802+
.spyOn(payload.db, 'createVersion')
1803+
.mockRejectedValueOnce(new Error('database connection lost'))
1804+
1805+
await expect(
1806+
payload.update({
1807+
id: doc.id,
1808+
autosave: true,
1809+
collection: autosaveCollectionSlug,
1810+
data: { title: 'will fail' },
1811+
draft: true,
1812+
}),
1813+
).rejects.toThrow('database connection lost')
1814+
1815+
updateVersionSpy.mockRestore()
1816+
createVersionSpy.mockRestore()
1817+
1818+
await cleanupDocuments({
1819+
collectionSlugs: [autosaveCollectionSlug],
1820+
payload,
1821+
})
1822+
})
17411823
})
17421824
})
17431825

0 commit comments

Comments
 (0)