Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions src/storage/version/put.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import getObject from '../object/get.js';
import { writeAuditEntry } from './audit.js';
import { versionKeyNew, versionKeyLegacy } from './paths.js';

const AUDIT_WRITE_RETRIES = 3;

export function getContentLength(body) {
if (body === undefined) {
return undefined;
Expand Down Expand Up @@ -246,22 +248,29 @@ export async function putObjectWithVersion(
// Store path without repo prefix and versionId without extension for readability.
if (createVersion) {
if (usesAuditFile) {
try {
const versionId = versionCreated ? Version : undefined;
const versionLabel = versionCreated ? (Label ?? '') : undefined;
const pathForAudit = (daCtx.site && Path.startsWith(`${daCtx.site}/`))
? Path.slice(daCtx.site.length)
: Path;
await writeAuditEntry(env, { bucket: input.Bucket, org: daCtx.org }, daCtx.site, ID, {
timestamp: Timestamp,
users: Users,
path: pathForAudit,
versionLabel,
versionId,
});
} catch (e) {
const versionId = versionCreated ? Version : undefined;
const versionLabel = versionCreated ? (Label ?? '') : undefined;
const pathForAudit = (daCtx.site && Path.startsWith(`${daCtx.site}/`))
? Path.slice(daCtx.site.length)
: Path;
let auditErr;
for (let i = 0; i < AUDIT_WRITE_RETRIES; i += 1) {
try {
// eslint-disable-next-line no-await-in-loop
await writeAuditEntry(env, { bucket: input.Bucket, org: daCtx.org }, daCtx.site, ID, {
timestamp: Timestamp,
users: Users,
path: pathForAudit,
versionLabel,
versionId,
});
auditErr = null;
break;
} catch (e) { auditErr = e; }
}
if (auditErr) {
// eslint-disable-next-line no-console
console.error('Failed to write audit entry', e);
console.error(`Failed to write audit entry after ${AUDIT_WRITE_RETRIES} retries`, auditErr);
}
} else if (!shouldCreateVersionObject) {
// Legacy path: write an empty version object so listFromLegacyStructure can find it.
Expand Down
90 changes: 90 additions & 0 deletions test/storage/version/put.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2534,6 +2534,96 @@ describe('Version Put', () => {
);
});

it('retries writeAuditEntry on transient failure and succeeds on second attempt', async () => {
let callCount = 0;
const mockWriteAuditEntry = async () => {
callCount += 1;
if (callCount < 2) throw new Error('transient R2 error');
};

const mockS3Client = { send: () => ({ $metadata: { httpStatusCode: 200 } }) };
const mockGetObject = async () => ({
body: 'content',
contentType: 'text/html',
metadata: { id: 'doc-id', version: 'v1' },
status: 200,
});

const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', {
'../../../src/storage/object/get.js': { default: mockGetObject },
'../../../src/storage/utils/version.js': {
ifNoneMatch: () => mockS3Client,
ifMatch: () => mockS3Client,
},
'../../../src/storage/version/audit.js': { writeAuditEntry: mockWriteAuditEntry },
});

const resp = await putObjectWithVersion(
{ VERSIONS_AUDIT_FILE_ORGS: 'o' },
{
org: 'o', ext: 'html', site: 'repo', users: [],
},
{
org: 'o', key: 'repo/p.html', body: 'edit', type: 'text/html',
},
true,
);

assert.strictEqual(resp.status, 200, 'document write must succeed despite transient audit error');
assert.strictEqual(callCount, 2, 'writeAuditEntry must be retried once after first failure');
});

it('logs error and continues when writeAuditEntry fails all retries', async () => {
let callCount = 0;
const mockWriteAuditEntry = async () => {
callCount += 1;
throw new Error('persistent R2 error');
};

const mockS3Client = { send: () => ({ $metadata: { httpStatusCode: 200 } }) };
const mockGetObject = async () => ({
body: 'content',
contentType: 'text/html',
metadata: { id: 'doc-id', version: 'v1' },
status: 200,
});

const { putObjectWithVersion } = await esmock('../../../src/storage/version/put.js', {
'../../../src/storage/object/get.js': { default: mockGetObject },
'../../../src/storage/utils/version.js': {
ifNoneMatch: () => mockS3Client,
ifMatch: () => mockS3Client,
},
'../../../src/storage/version/audit.js': { writeAuditEntry: mockWriteAuditEntry },
});

const errors = [];
const origError = console.error;
console.error = (...args) => errors.push(args.map(String).join(' '));
let resp;
try {
resp = await putObjectWithVersion(
{ VERSIONS_AUDIT_FILE_ORGS: 'o' },
{
org: 'o', ext: 'html', site: 'repo', users: [],
},
{
org: 'o', key: 'repo/p.html', body: 'edit', type: 'text/html',
},
true,
);
} finally {
console.error = origError;
}

assert.strictEqual(resp.status, 200, 'document write must succeed even when audit write is permanently failing');
assert.strictEqual(callCount, 3, 'writeAuditEntry must be attempted 3 times before giving up');
assert.ok(
errors.some((e) => e.includes('after 3 retries')),
'error after all retries exhausted must be logged with retry count',
);
});

it('writes legacy empty version object when org is not in VERSIONS_AUDIT_FILE_ORGS', async () => {
const auditCalls = [];
const legacyPutCalls = [];
Expand Down
Loading