From 00ef66bb95f298203b18990805a3206a8fe6f704 Mon Sep 17 00:00:00 2001 From: kptdobe Date: Thu, 30 Apr 2026 08:55:39 +0200 Subject: [PATCH] fix: treat putVersion 412 as version-already-exists, not failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple concurrent POST /versionsource requests target the same document (e.g. the localization pipeline processing many locales in parallel), the second caller's R2 PUT for the version object gets a 412 PreconditionFailed because the first caller already wrote it with If-None-Match:*. The version IS in R2 — returning 500 "Version was not created" is incorrect. Fixes ~860 spurious 500 errors per 24h on /versionsource/adobecom/da-cc. Co-Authored-By: Claude Sonnet 4.6 --- src/storage/version/put.js | 4 +++- test/storage/version/put.test.js | 17 ++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/storage/version/put.js b/src/storage/version/put.js index 92fc291..aa1d5de 100644 --- a/src/storage/version/put.js +++ b/src/storage/version/put.js @@ -242,7 +242,9 @@ export async function putObjectWithVersion( if (versionResp.status !== 200 && versionResp.status !== 412) { return { status: versionResp.status, metadata: { id: ID } }; } - versionCreated = versionResp.status === 200; + // 412 means the version key already exists — a concurrent request created it first. + // The version IS persisted in R2, so treat it as created. + versionCreated = versionResp.status === 200 || versionResp.status === 412; } // Audit: one entry per versionable PUT; versionLabel + versionId when labelled version created. diff --git a/test/storage/version/put.test.js b/test/storage/version/put.test.js index 8987843..7d16ef7 100644 --- a/test/storage/version/put.test.js +++ b/test/storage/version/put.test.js @@ -2228,7 +2228,7 @@ describe('Version Put', () => { assert.strictEqual(true, resp.versionCreated); }); - it('putObjectWithVersion sets versionCreated false when version already existed (putVersion 412)', async () => { + it('putObjectWithVersion sets versionCreated true when version already existed (putVersion 412 = concurrent race)', async () => { const mockGetObject = async () => ({ body: 'content', contentType: 'text/html', @@ -2261,10 +2261,10 @@ describe('Version Put', () => { org: 'o', key: 'a.html', body: 'new', label: 'My version', }, true); assert.equal(200, resp.status); - assert.strictEqual(false, resp.versionCreated); + assert.strictEqual(true, resp.versionCreated); }); - it('postObjectVersion returns 500 when version was not created (putVersion 412)', async () => { + it('postObjectVersion returns 201 when version already exists in R2 (putVersion 412 concurrent race)', async () => { const req = { json: async () => ({ label: 'my-label' }) }; const env = {}; const ctx = { @@ -2299,8 +2299,7 @@ describe('Version Put', () => { }); const resp = await postObjectVersion(req, env, ctx); - assert.equal(500, resp.status); - assert.strictEqual(resp.error, 'Version was not created'); + assert.equal(201, resp.status); }); it('postObjectVersion returns 201 when version is successfully created', async () => { @@ -3029,7 +3028,7 @@ describe('Version Put', () => { assert.equal(201, resp.status); }); - it('returns 500 when versionCreated is false (version already exists / 412)', async () => { + it('returns 201 when version PUT gets 412 (concurrent race — version already exists in R2)', async () => { const mockGetObject = async () => ({ body: 'doc content', contentType: 'text/html', @@ -3043,7 +3042,8 @@ describe('Version Put', () => { return { $metadata: { httpStatusCode: 200 } }; }, }; - // version put throws 412 → putVersion returns { status: 412 } → versionCreated stays false + // Concurrent request already created the version object; R2 returns 412 on our PUT. + // The version IS persisted — this request should still return 201, not 500. const versionClient = { async send() { const err = new Error('precondition failed'); @@ -3069,8 +3069,7 @@ describe('Version Put', () => { }; const resp = await postObjectVersionWithLabel('My Label', {}, daCtx); - assert.strictEqual(resp.status, 500, 'must return 500 when version was not created'); - assert.strictEqual(resp.error, 'Version was not created'); + assert.strictEqual(resp.status, 201, 'must return 201 when version already exists (concurrent 412)'); }); it('returns 201 when version client body is a ReadableStream that would be disturbed by SDK retry', async () => {