diff --git a/src/storage/version/put.js b/src/storage/version/put.js index 978cd140..0150383c 100644 --- a/src/storage/version/put.js +++ b/src/storage/version/put.js @@ -295,18 +295,15 @@ export async function putObjectWithVersion( } catch (e) { const status = e.$metadata?.httpStatusCode || 500; if (status === 412) { - // Only retry if no client conditionals (internal operation) and under retry limit - if (!effectiveConditionals?.ifMatch) { - return putObjectWithVersion( - env, - daCtx, - update, - body, - guid, - clientConditionals, - ); + // Retry when no client conditional (internal operation) or when the client sent + // If-Match: * (wildcard — asserts existence only, already verified above; a concurrent + // write changed the ETag, so re-fetch and retry like the no-conditional path). + // A specific ETag means the client explicitly requires that version, so propagate 412. + const shouldRetry = !effectiveConditionals?.ifMatch + || effectiveConditionals.ifMatch === '*'; + if (shouldRetry) { + return putObjectWithVersion(env, daCtx, update, body, guid, clientConditionals); } - // Client conditional failed or max retries exceeded, return 412 return { status: 412, metadata: { id: ID } }; } diff --git a/test/storage/version/put.test.js b/test/storage/version/put.test.js index 4e184509..9583826c 100644 --- a/test/storage/version/put.test.js +++ b/test/storage/version/put.test.js @@ -231,6 +231,86 @@ describe('Version Put', () => { assert.strictEqual(resp.error, 'testing 123'); }); + it('putObjectWithVersion retries on ETag mismatch when If-Match: * is sent (existing document, concurrent write)', async () => { + let getObjectCallCount = 0; + const mockGetObject = async () => { + getObjectCallCount += 1; + return { status: 200, metadata: { id: 'existing-id' }, etag: `etag-${getObjectCallCount}` }; + }; + + let firstWrite = true; + const sendCalls = []; + const mockIfMatchClient = { + async send(cmd) { + sendCalls.push(cmd); + if (firstWrite) { + firstWrite = false; + const err = { $metadata: { httpStatusCode: 412 } }; + throw err; + } + return { $metadata: { httpStatusCode: 200 } }; + }, + }; + + const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', { + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/utils/version.js': { + ifMatch: () => mockIfMatchClient, + ifNoneMatch: () => mockIfMatchClient, + }, + }); + + const resp = await putObjectWithVersion( + { env: true }, + { users: [{ email: 'a@b.com' }] }, + { org: 'org', key: 'doc.html', type: 'text/html' }, + null, + null, + { ifMatch: '*' }, + ); + + assert.equal(200, resp.status, 'Should succeed after retry'); + assert.equal(2, getObjectCallCount, 'Should have fetched current state twice (initial + retry)'); + assert.equal(2, sendCalls.length, 'Should have attempted R2 write twice'); + }); + + it('putObjectWithVersion returns 412 without retry when client sends specific ETag and R2 rejects it', async () => { + const mockGetObject = async () => ({ + status: 200, + metadata: { id: 'existing-id' }, + etag: '"server-etag"', + }); + + const sendCalls = []; + const mockIfMatchClient = { + async send(cmd) { + sendCalls.push(cmd); + const err = { $metadata: { httpStatusCode: 412 } }; + throw err; + }, + }; + + const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', { + '../../../src/storage/object/get.js': { default: mockGetObject }, + '../../../src/storage/utils/version.js': { + ifMatch: () => mockIfMatchClient, + ifNoneMatch: () => mockIfMatchClient, + }, + }); + + const resp = await putObjectWithVersion( + { env: true }, + { users: [{ email: 'a@b.com' }] }, + { org: 'org', key: 'doc.html', type: 'text/html' }, + null, + null, + { ifMatch: '"client-specific-etag"' }, + ); + + assert.equal(412, resp.status, 'Should propagate 412 when client sent a specific ETag'); + assert.equal(1, sendCalls.length, 'Should not retry when client sent a specific ETag'); + }); + it('Put Object With Version store content', async () => { // eslint-disable-next-line consistent-return const mockGetObject = async (e, u, h) => {