refactor(data): improve error handling and rollback for document and TTS segment operations

Enhance robustness of document deletion and TTS segment transfer by improving
error logging, partial rollback, and degraded state reporting. Add best-effort
cleanup and recovery mechanisms to prevent orphaned data and ensure diagnostic
information is captured for unexpected failures. Update user data cleanup to
log and swallow restoration errors without masking primary failures. Refine
claim logic to handle unmapped TTS audio keys safely.
This commit is contained in:
Richard R 2026-06-06 20:03:34 -06:00
parent 9a8bc060d9
commit 0df57b62b3
6 changed files with 104 additions and 37 deletions

View file

@ -111,10 +111,6 @@ export async function DELETE(req: NextRequest) {
const targetUserIds = [storageUserId]; const targetUserIds = [storageUserId];
if (targetUserIds.length === 0) {
return NextResponse.json({ success: true, deleted: 0 });
}
let targetIds: string[] = []; let targetIds: string[] = [];
if (idsParam) { if (idsParam) {
targetIds = idsParam targetIds = idsParam

View file

@ -52,8 +52,8 @@ export default function ClaimDataModal({
toast.success( toast.success(
`Successfully claimed ${claimed.documents} documents, ` `Successfully claimed ${claimed.documents} documents, `
+ `${claimed.audiobooks} audiobooks, ` + `${claimed.audiobooks} audiobooks, `
+ `${claimed.preferences} preference set(s), and ` + `${claimed.preferences} preference set(s), `
+ `${claimed.progress} reading progress record(s) and ` + `${claimed.progress} reading progress record(s), and `
+ `${claimed.documentSettings} document setting(s)!`, + `${claimed.documentSettings} document setting(s)!`,
); );
onClaimed(); onClaimed();

View file

@ -10,6 +10,8 @@ import {
import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
import { PDF_PARSER_VERSION, encodeParserVersion } from '@openreader/compute-core/api-contracts'; import { PDF_PARSER_VERSION, encodeParserVersion } from '@openreader/compute-core/api-contracts';
import { getS3Client, getS3Config, getS3ProxyClient } from '@/lib/server/storage/s3'; import { getS3Client, getS3Config, getS3ProxyClient } from '@/lib/server/storage/s3';
import { serverLogger } from '@/lib/server/logger';
import { logDegraded } from '@/lib/server/errors/logging';
const DOCUMENT_ID_REGEX = /^[a-f0-9]{64}$/i; const DOCUMENT_ID_REGEX = /^[a-f0-9]{64}$/i;
const SAFE_NAMESPACE_REGEX = /^[a-zA-Z0-9._-]{1,128}$/; const SAFE_NAMESPACE_REGEX = /^[a-zA-Z0-9._-]{1,128}$/;
@ -513,7 +515,20 @@ export async function deleteDocumentBlob(id: string, namespace: string | null):
// Delete the source after the initial derived-artifact cleanup, then sweep // Delete the source after the initial derived-artifact cleanup, then sweep
// parsed output once more to catch a worker that finished during deletion. // parsed output once more to catch a worker that finished during deletion.
await client.send(new DeleteObjectCommand({ Bucket: cfg.bucket, Key: key })); await client.send(new DeleteObjectCommand({ Bucket: cfg.bucket, Key: key }));
await deleteDocumentPrefix(parsedPrefix); // The source blob is already gone at this point. Treat the final sweep as a
// best-effort cleanup: if it throws, rethrowing would make callers roll back
// the document row even though the source is deleted, so log and swallow.
try {
await deleteDocumentPrefix(parsedPrefix);
} catch (error) {
logDegraded(serverLogger, {
event: 'documents.blob_delete.final_parsed_sweep.failed',
msg: 'Failed final parsed-output sweep after document deletion',
step: 'delete_document_parsed_prefix_final',
context: { parsedPrefix },
error,
});
}
} }
export async function deleteTempDocumentUpload(token: string, userId: string, namespace: string | null): Promise<void> { export async function deleteTempDocumentUpload(token: string, userId: string, namespace: string | null): Promise<void> {
@ -569,7 +584,12 @@ export async function deleteDocumentPrefix(prefix: string): Promise<number> {
); );
const errors = deleteRes.Errors ?? []; const errors = deleteRes.Errors ?? [];
if (errors.length > 0) { if (errors.length > 0) {
throw new Error(`Failed deleting ${errors.length} document storage objects`); const details = errors
.map((e) => `${e.Key ?? '?'} (${e.Code ?? 'Unknown'}: ${e.Message ?? 'no message'})`)
.join('; ');
throw new Error(
`Failed deleting ${errors.length} document storage object(s) under prefix "${cleanedPrefix}": ${details}`,
);
} }
deleted += keys.length; deleted += keys.length;
} }

View file

@ -268,28 +268,41 @@ export async function copyTtsSegmentPrefix(sourcePrefix: string, destinationPref
let copied = 0; let copied = 0;
let continuationToken: string | undefined; let continuationToken: string | undefined;
do { // Track destination keys we have written so a mid-copy failure can be rolled
const listRes = await client.send(new ListObjectsV2Command({ // back, leaving no orphaned objects behind at the destination prefix.
Bucket: cfg.bucket, const copiedKeys: string[] = [];
Prefix: source, try {
ContinuationToken: continuationToken, do {
})); const listRes = await client.send(new ListObjectsV2Command({
const keys = (listRes.Contents ?? [])
.map((item) => item.Key)
.filter((value): value is string => typeof value === 'string' && value.startsWith(source));
for (const key of keys) {
await client.send(new CopyObjectCommand({
Bucket: cfg.bucket, Bucket: cfg.bucket,
Key: `${destination}${key.slice(source.length)}`, Prefix: source,
CopySource: `${cfg.bucket}/${key}`, ContinuationToken: continuationToken,
ServerSideEncryption: 'AES256',
})); }));
copied += 1; const keys = (listRes.Contents ?? [])
} .map((item) => item.Key)
.filter((value): value is string => typeof value === 'string' && value.startsWith(source));
continuationToken = listRes.IsTruncated ? listRes.NextContinuationToken : undefined; for (const key of keys) {
} while (continuationToken); const destinationKey = `${destination}${key.slice(source.length)}`;
await client.send(new CopyObjectCommand({
Bucket: cfg.bucket,
Key: destinationKey,
CopySource: `${cfg.bucket}/${key}`,
ServerSideEncryption: 'AES256',
}));
copiedKeys.push(destinationKey);
copied += 1;
}
continuationToken = listRes.IsTruncated ? listRes.NextContinuationToken : undefined;
} while (continuationToken);
} catch (error) {
// Best-effort rollback of the partial copy; surface the original error.
if (copiedKeys.length > 0) {
await deleteTtsSegmentAudioObjects(copiedKeys).catch(() => {});
}
throw error;
}
return copied; return copied;
} }

View file

@ -301,16 +301,39 @@ async function transferDocumentTtsSegments(input: {
const encodedFrom = encodeURIComponent(input.fromUserId); const encodedFrom = encodeURIComponent(input.fromUserId);
const encodedTo = encodeURIComponent(input.toUserId); const encodedTo = encodeURIComponent(input.toUserId);
const sourceAudioKeyPrefix = `/users/${encodedFrom}/docs/${input.documentId}/`;
const destAudioKeyPrefix = `/users/${encodedTo}/docs/${input.documentId}/`;
if (variants.length > 0) { if (variants.length > 0) {
await input.database.insert(ttsSegmentVariants) await input.database.insert(ttsSegmentVariants)
.values(variants.map((variant: typeof ttsSegmentVariants.$inferSelect) => ({ .values(variants.map((variant: typeof ttsSegmentVariants.$inferSelect) => {
...variant, const audioKey = variant.audioKey ?? null;
userId: input.toUserId, if (!audioKey || audioKey.includes(sourceAudioKeyPrefix)) {
audioKey: variant.audioKey?.replace( return {
`/users/${encodedFrom}/docs/${input.documentId}/`, ...variant,
`/users/${encodedTo}/docs/${input.documentId}/`, userId: input.toUserId,
) ?? null, audioKey: audioKey?.replace(sourceAudioKeyPrefix, destAudioKeyPrefix) ?? null,
}))) };
}
// The key did not contain the expected source path, so it cannot be
// safely remapped. Leaving it would point the new owner at the source
// user's (soon-deleted) audio, so null it out and log for investigation.
logDegraded(serverLogger, {
event: 'user.claim.tts_variant_audio_key.unmapped',
msg: 'TTS segment variant audioKey did not match expected source path during claim',
step: 'remap_tts_variant_audio_key',
context: {
originalAudioKey: audioKey,
fromUserIdHash: hashForLog(input.fromUserId),
toUserIdHash: hashForLog(input.toUserId),
documentId: input.documentId,
},
});
return {
...variant,
userId: input.toUserId,
audioKey: null,
};
}))
.onConflictDoNothing(); .onConflictDoNothing();
} }

View file

@ -61,6 +61,21 @@ export async function deleteUserStorageData(
await database.insert(documents).values(docsToRestore).onConflictDoNothing(); await database.insert(documents).values(docsToRestore).onConflictDoNothing();
removedDocs.length = 0; removedDocs.length = 0;
}; };
// Restore before throwing the diagnostic AggregateError, but never let a
// restore failure replace the original cleanup failures we want to surface.
const tryRestoreRemovedDocs = async () => {
try {
await restoreRemovedDocs();
} catch (error) {
logDegraded(serverLogger, {
event: 'user.data_cleanup.restore_removed_docs.failed',
msg: 'Failed to restore document rows after cleanup failure',
step: 'restore_removed_docs',
context: { userIdHash: hashForLog(userId) },
error,
});
}
};
const removeOwnershipAndCheckLastOwner = async (doc: DocumentRow) => { const removeOwnershipAndCheckLastOwner = async (doc: DocumentRow) => {
const run = async (tx: typeof database) => { const run = async (tx: typeof database) => {
// Lock every ownership row for this document so concurrent account // Lock every ownership row for this document so concurrent account
@ -248,7 +263,7 @@ export async function deleteUserStorageData(
} }
if (failures.length > 0) { if (failures.length > 0) {
await restoreRemovedDocs(); await tryRestoreRemovedDocs();
throw new AggregateError(failures, `User storage cleanup failed in ${failures.length} operation(s)`); throw new AggregateError(failures, `User storage cleanup failed in ${failures.length} operation(s)`);
} }
@ -289,7 +304,7 @@ export async function deleteUserStorageData(
} }
if (failures.length > 0) { if (failures.length > 0) {
await restoreRemovedDocs(); await tryRestoreRemovedDocs();
throw new AggregateError(failures, `User database cleanup failed in ${failures.length} operation(s)`); throw new AggregateError(failures, `User database cleanup failed in ${failures.length} operation(s)`);
} }
} }