fix(import): preserve committed document on lost response (AI-assisted) (#1864)

* fix(import): preserve committed document on lost response (AI-assisted)

A rejected save response can arrive after the server has committed the imported aggregate. Do not compensate by deleting the stage, because that turns an unknown write outcome into permanent data loss. Keep disposable local-cache cleanup and cover the response-loss path with a stateful regression test.

* fix(import): verify rejected document saves

Read the new document back when saveDocument rejects. Treat a found document as committed, while confirmed absence and read failures keep the import on the error path without destructive cleanup. Cover all three outcomes in the stateful import test.
This commit is contained in:
DDL authored and GitHub committed 2026-10-12 17:14:42 +08:00
1 parent 7ca6077cdd
commit 9d2eface53
2 files changed
+75 -11

No files matched your search

+15 -9
View File
@@ -543,7 +543,18 @@ export function useImportClassroom(onSuccess?: (importedStageId: string) => void
// replaces.
await mutateDocument(
newStageId,
async (_existing, store) => store.saveDocument(document),
async (_existing, store) => {
try {
await store.saveDocument(document);
} catch (saveError) {
try {
if (await store.loadDocument(newStageId)) return;
} catch (verificationError) {
log.error('Failed to verify rejected document write:', verificationError);
}
throw saveError;
}
},
{},
{ mode: 'replace' },
);
@@ -560,9 +571,9 @@ export function useImportClassroom(onSuccess?: (importedStageId: string) => void
: t('import.error.invalidZip');
toast.error(message, { id: toastId });
} finally {
// Local rows cannot join the document transaction. Server allocations
// are left to the pending-asset collector: the browser cannot safely
// delete them after a document write with an ambiguous outcome.
// A rejected response does not prove the atomic document write failed.
// Never delete by stage id here. Local rows are disposable caches, and
// the pending collector reclaims unreferenced server assets.
const cleanup = async (label: string, operation: () => Promise<unknown>) => {
try {
await operation();
@@ -572,11 +583,6 @@ export function useImportClassroom(onSuccess?: (importedStageId: string) => void
};
if (!importCommitted && importedStageId) {
const stageId = importedStageId;
await cleanup('document', async () => {
await mutateDocument(stageId, async (_document, store) =>
store.deleteDocument(stageId),
);
});
await cleanup('generated media', () =>
db.mediaFiles.where('stageId').equals(stageId).delete(),
);
+60 -2
View File
@@ -9,11 +9,14 @@ const mocks = vi.hoisted(() => ({
mediaDelete: vi.fn(),
audioDelete: vi.fn(),
save: vi.fn(),
load: vi.fn(),
remove: vi.fn(),
toastError: vi.fn(),
toastSuccess: vi.fn(),
}));
const documents = new Map<string, unknown>();
// Exercise the asynchronous import callback without mounting its file-input UI.
vi.mock('react', async (importOriginal) => ({
...(await importOriginal<typeof import('react')>()),
@@ -29,7 +32,11 @@ vi.mock('@/lib/media/asset-pool', () => ({ putAsset: mocks.poolPut }));
vi.mock('@/lib/document-store', () => ({
canonicalizeLegacyScene: (scene: unknown) => scene,
mutateDocument: (_id: string, work: (document: null, store: unknown) => unknown) =>
work(null, { saveDocument: mocks.save, deleteDocument: mocks.remove }),
work(null, {
saveDocument: mocks.save,
loadDocument: mocks.load,
deleteDocument: mocks.remove,
}),
}));
vi.mock('@/lib/device-storage/database', () => ({
mediaFileKey: (stageId: string, ref: string) => `${stageId}:${ref}`,
@@ -86,6 +93,14 @@ async function importArchive(onSuccess = vi.fn()) {
describe('server-backed import commit boundary', () => {
beforeEach(() => {
for (const mock of Object.values(mocks)) mock.mockReset();
documents.clear();
mocks.save.mockImplementation(async (document) => {
documents.set(document.stage.id, document);
});
mocks.load.mockImplementation(async (stageId) => documents.get(stageId) ?? null);
mocks.remove.mockImplementation(async (stageId) => {
documents.delete(stageId);
});
mocks.poolPut.mockResolvedValueOnce('ast_audio').mockResolvedValueOnce('ast_image');
});
@@ -99,6 +114,7 @@ describe('server-backed import commit boundary', () => {
expect(mocks.poolPut.mock.invocationCallOrder[1]).toBeLessThan(
mocks.save.mock.invocationCallOrder[0],
);
expect(mocks.load).not.toHaveBeenCalled();
expect(onSuccess).toHaveBeenCalledWith(document.stage.id);
expect(mocks.remove).not.toHaveBeenCalled();
});
@@ -112,6 +128,7 @@ describe('server-backed import commit boundary', () => {
await importArchive(onSuccess);
expect(mocks.save).not.toHaveBeenCalled();
expect(onSuccess).not.toHaveBeenCalled();
expect(mocks.remove).not.toHaveBeenCalled();
expect(mocks.audioDelete).toHaveBeenCalledOnce();
expect(mocks.mediaDelete).toHaveBeenCalledOnce();
expect(mocks.toastError).toHaveBeenCalledOnce();
@@ -138,13 +155,54 @@ describe('server-backed import commit boundary', () => {
expect(mocks.remove).not.toHaveBeenCalled();
});
it('reports a failed document commit without claiming import success', async () => {
it('reports failure when read-back confirms the document was not committed', async () => {
mocks.save.mockRejectedValue(new Error('document unavailable'));
const onSuccess = vi.fn();
await importArchive(onSuccess);
const document = mocks.save.mock.calls[0][0];
expect(mocks.load).toHaveBeenCalledExactlyOnceWith(document.stage.id);
expect(onSuccess).not.toHaveBeenCalled();
expect(mocks.toastSuccess).not.toHaveBeenCalled();
expect(mocks.toastError).toHaveBeenCalledOnce();
expect(mocks.remove).not.toHaveBeenCalled();
expect(mocks.audioDelete).toHaveBeenCalledOnce();
expect(mocks.mediaDelete).toHaveBeenCalledOnce();
});
it('reports failure without deleting when the rejected write cannot be verified', async () => {
mocks.save.mockRejectedValue(new Error('document unavailable'));
mocks.load.mockRejectedValue(new Error('read unavailable'));
const onSuccess = vi.fn();
await importArchive(onSuccess);
const document = mocks.save.mock.calls[0][0];
expect(mocks.load).toHaveBeenCalledExactlyOnceWith(document.stage.id);
expect(onSuccess).not.toHaveBeenCalled();
expect(mocks.toastSuccess).not.toHaveBeenCalled();
expect(mocks.toastError).toHaveBeenCalledOnce();
expect(mocks.remove).not.toHaveBeenCalled();
expect(mocks.audioDelete).toHaveBeenCalledOnce();
expect(mocks.mediaDelete).toHaveBeenCalledOnce();
});
it('treats a document found after a rejected write as committed', async () => {
mocks.save.mockImplementation(async (document) => {
documents.set(document.stage.id, document);
throw new Error('response lost');
});
const onSuccess = vi.fn();
await importArchive(onSuccess);
const document = mocks.save.mock.calls[0][0];
expect(documents.get(document.stage.id)).toBe(document);
expect(mocks.load).toHaveBeenCalledExactlyOnceWith(document.stage.id);
expect(mocks.remove).not.toHaveBeenCalled();
expect(mocks.audioDelete).not.toHaveBeenCalled();
expect(mocks.mediaDelete).not.toHaveBeenCalled();
expect(onSuccess).toHaveBeenCalledWith(document.stage.id);
expect(mocks.toastSuccess).toHaveBeenCalledOnce();
expect(mocks.toastError).not.toHaveBeenCalled();
});
});