mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-03 00:13:09 +00:00
fix: keep failed messages for the next import run and stop an import that can't go on
A message that failed for a passing reason (a dropped connection, a server error, an expired token) used to count as done, so continuing the import skipped it for good, and a slow upload followed by a cancel imported the messages after it twice. - The checkpoint keeps every message that is done, also past a gap, and the next run of the same import tries only the rest again. - Ten failures in a row, a full mailbox (overQuota) or a refusal (forbidden) stop the import with a message instead of going through the whole file. - An upload without an answer pauses the import instead of failing the message: 30 seconds, then until the upload window ends. - Email/import, the export's Blob/get and folder listing pass the client's own 429 pause by, and a 429 on them doesn't pause the rest of the app. - Signing out stops the job of that account only, writes no checkpoint and imports nothing more afterwards; another account's sign-out leaves the job alone. - One tab imports a file at a time (Web Locks), and Import waits until the dialog knows whether an earlier run can be continued. The import dialog tests now wait for the Import button to be enabled before clicking it, since it stays off during that lookup.
This commit is contained in:
@@ -104,6 +104,20 @@ describe('MailExportDialog', () => {
|
||||
expect(discard).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('another account signing out leaves the job alone; its own sign-out stops it with the sign-out reason', async () => {
|
||||
const { runBeforeSignOut } = await import('@/lib/sign-out-cleanup');
|
||||
const controller = new AbortController();
|
||||
render(<MailTransferHost />);
|
||||
act(() => {
|
||||
openMailExport({ mailboxId: 'inbox' });
|
||||
useMailTransferStore.getState().startJob({ kind: 'export', controller, scope: 'bulwark-a', finished: Promise.resolve(), progress: { phase: 'running', exported: 1, failed: 0, failures: [], total: 4, fraction: 0.25 } });
|
||||
});
|
||||
await act(() => runBeforeSignOut('bulwark-b'));
|
||||
expect(controller.signal.aborted).toBe(false);
|
||||
await act(() => runBeforeSignOut('bulwark-a'));
|
||||
expect(controller.signal.reason).toBe('signout');
|
||||
});
|
||||
|
||||
it('runs on in the background and the chip names the export', () => {
|
||||
const controller = new AbortController();
|
||||
render(<MailTransferHost />);
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { render, screen, fireEvent, act, waitFor } from '@testing-library/react';
|
||||
import { MailTransferHost } from '@/components/mail-transfer/mail-transfer-host';
|
||||
import { openMailImport } from '@/stores/mail-transfer-store';
|
||||
import { useEmailStore } from '@/stores/email-store';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
import type { Mailbox } from '@/lib/jmap/types';
|
||||
|
||||
// The lookup of an earlier run never answers here.
|
||||
vi.mock('@/lib/mail-import/checkpoints', () => ({
|
||||
checkpointKey: () => 'k',
|
||||
readCheckpoint: () => new Promise(() => {}),
|
||||
clearCheckpointsFor: async () => {},
|
||||
}));
|
||||
|
||||
describe('MailImportDialog and an earlier run', () => {
|
||||
it('keeps Import off until it knows whether the file can be continued', async () => {
|
||||
useEmailStore.setState({ mailboxes: [{ id: 'inbox', name: 'Inbox', role: 'inbox', myRights: { mayAddItems: true } } as Mailbox] } as never);
|
||||
useAuthStore.setState({ client: { getAccountId: () => 'acc1' } as never, activeAccountId: 'b1' } as never);
|
||||
render(<MailTransferHost />);
|
||||
act(() => openMailImport());
|
||||
const input = screen.getByTestId('mail-import-file') as HTMLInputElement;
|
||||
Object.defineProperty(input, 'files', { value: [new File(['From a Tue May 5 10:11:12 2020\n\nb\n'], 'a.mbox')], configurable: true });
|
||||
fireEvent.change(input);
|
||||
await screen.findByText('a.mbox');
|
||||
const start = screen.getByTestId('mail-import-start') as HTMLButtonElement;
|
||||
await waitFor(() => expect(start.disabled).toBe(true));
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
expect(start.disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -48,6 +48,8 @@ describe('MailImportDialog', () => {
|
||||
pick([new File(['From a Tue May 5 10:11:12 2020\nSubject: x\n\nb\n'], 'old.mbox')]);
|
||||
await screen.findByText('old.mbox');
|
||||
expect(screen.getByText(/format_mbox/)).toBeTruthy();
|
||||
// Import waits until it knows whether an earlier run can be continued.
|
||||
await waitFor(() => expect((screen.getByTestId('mail-import-start') as HTMLButtonElement).disabled).toBe(false));
|
||||
fireEvent.click(screen.getByTestId('mail-import-start'));
|
||||
expect(startImport).toHaveBeenCalledWith(expect.objectContaining({
|
||||
accountId: 'acc2', targetMailboxId: 'shared', keepFolders: false, resume: false,
|
||||
@@ -67,6 +69,8 @@ describe('MailImportDialog', () => {
|
||||
const keep = await screen.findByRole('checkbox', { name: /keep_folders/ });
|
||||
expect((keep as HTMLInputElement).checked).toBe(true);
|
||||
fireEvent.change(screen.getByTestId('mail-import-target'), { target: { value: '\u0000top' } });
|
||||
// Import waits until it knows whether an earlier run can be continued.
|
||||
await waitFor(() => expect((screen.getByTestId('mail-import-start') as HTMLButtonElement).disabled).toBe(false));
|
||||
fireEvent.click(screen.getByTestId('mail-import-start'));
|
||||
expect(startImport).toHaveBeenCalledWith(expect.objectContaining({ accountId: 'acc1', targetMailboxId: null, keepFolders: true }));
|
||||
});
|
||||
|
||||
@@ -96,7 +96,7 @@ export function MailExportDialog({ request }: { request: ExportRequest }) {
|
||||
footer={active ? (
|
||||
<>
|
||||
<Button variant="outline" onClick={hide}>{t("background")}</Button>
|
||||
<Button variant="destructive" onClick={cancelJob} aria-describedby={`${id}-status`}>{t("cancel_export")}</Button>
|
||||
<Button variant="destructive" onClick={() => cancelJob()} aria-describedby={`${id}-status`}>{t("cancel_export")}</Button>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
|
||||
@@ -65,6 +65,7 @@ export function MailImportDialog({ request }: { request: ImportRequest }) {
|
||||
const [keepFolders, setKeepFolders] = useState(true);
|
||||
const [resume, setResume] = useState(true);
|
||||
const [resumable, setResumable] = useState(0);
|
||||
const [checking, setChecking] = useState(false);
|
||||
|
||||
const anyFolders = picked.some((p) => p.folders);
|
||||
const targetMailbox = targets.find((x) => x.mailbox.id === target)?.mailbox;
|
||||
@@ -86,12 +87,16 @@ export function MailImportDialog({ request }: { request: ImportRequest }) {
|
||||
// An earlier import of the same file into the same place that stopped part way.
|
||||
useEffect(() => {
|
||||
let current = true;
|
||||
if (picked.length === 0 || !accountId) { setResumable(0); return; }
|
||||
if (picked.length === 0 || !accountId) { setResumable(0); setChecking(false); return; }
|
||||
// Import waits for this: starting before it knows would throw the checkpoint away.
|
||||
setChecking(true);
|
||||
void Promise.all(picked.map((p) => readCheckpoint(checkpointKey({
|
||||
scope: activeAccountId ?? "default", accountId, target: targetId, keepFolders: effectiveKeep, file: p.file,
|
||||
})))).then((found) => {
|
||||
if (current) setResumable(found.reduce((n, c) => n + (c?.done ?? 0), 0));
|
||||
});
|
||||
if (!current) return;
|
||||
setResumable(found.reduce((n, c) => n + (c?.done ?? 0) + (c?.ahead?.length ?? 0), 0));
|
||||
setChecking(false);
|
||||
}, () => { if (current) setChecking(false); });
|
||||
return () => { current = false; };
|
||||
}, [picked, accountId, targetId, effectiveKeep, activeAccountId]);
|
||||
|
||||
@@ -136,7 +141,7 @@ export function MailImportDialog({ request }: { request: ImportRequest }) {
|
||||
footer={active ? (
|
||||
<>
|
||||
<Button variant="outline" onClick={hide}>{t("background")}</Button>
|
||||
<Button variant="destructive" onClick={cancelJob} aria-describedby={`${id}-status`}>{t("cancel_import")}</Button>
|
||||
<Button variant="destructive" onClick={() => cancelJob()} aria-describedby={`${id}-status`}>{t("cancel_import")}</Button>
|
||||
</>
|
||||
) : (
|
||||
<Button onClick={close}>{t("close")}</Button>
|
||||
@@ -147,7 +152,7 @@ export function MailImportDialog({ request }: { request: ImportRequest }) {
|
||||
);
|
||||
}
|
||||
|
||||
const canStart = picked.length > 0 && !!accountId && !(target === TOP && !effectiveKeep);
|
||||
const canStart = picked.length > 0 && !!accountId && !checking && !(target === TOP && !effectiveKeep);
|
||||
|
||||
return (
|
||||
<Dialog
|
||||
@@ -253,11 +258,15 @@ export function MailImportDialog({ request }: { request: ImportRequest }) {
|
||||
type T = ReturnType<typeof useTranslations>;
|
||||
|
||||
const PST_ERRORS = ["encrypted", "not_pst", "unreadable", "worker"];
|
||||
const IMPORT_ERRORS = ["over_quota", "forbidden", "failures", "network", "busy"];
|
||||
|
||||
/** The PST reader's errors in words; anything else as the server or browser said it. */
|
||||
/** Why an import stopped, in words; anything else as the server or browser said it. */
|
||||
function errorText(t: T, error: string): string {
|
||||
const code = /^pst:(\w+):/.exec(error)?.[1];
|
||||
return code && PST_ERRORS.includes(code) ? t(`pst_error_${code}`) : error;
|
||||
const pst = /^pst:(\w+):/.exec(error)?.[1];
|
||||
if (pst && PST_ERRORS.includes(pst)) return t(`pst_error_${pst}`);
|
||||
const own = /^import:(\w+)$/.exec(error)?.[1];
|
||||
if (own && IMPORT_ERRORS.includes(own)) return t(`error_${own}`);
|
||||
return error;
|
||||
}
|
||||
|
||||
function progressTitle(t: T, p: ImportProgress): string {
|
||||
@@ -312,6 +321,7 @@ function ProgressBody({ progress: p, t, locale, statusId }: { progress: ImportPr
|
||||
</div>
|
||||
)}
|
||||
{p.phase === "cancelled" && <p className="text-muted-foreground">{t("cancelled_hint")}</p>}
|
||||
{p.phase === "done" && (p.retryable ?? 0) > 0 && <p className="text-muted-foreground">{t("retry_hint")}</p>}
|
||||
{p.phase === "failed" && p.error && <p className="text-destructive break-words">{errorText(t, p.error)}</p>}
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -10,6 +10,9 @@ import { MailImportDialog } from "./mail-import-dialog";
|
||||
import { MailExportDialog } from "./mail-export-dialog";
|
||||
import { formatClock } from "./transfer-progress";
|
||||
|
||||
/** How long sign-out waits for a cancelled job to stop (its budget is 2 s). */
|
||||
const SIGN_OUT_WAIT_MS = 1500;
|
||||
|
||||
/**
|
||||
* Draws the mail import and export dialogs, and while it is hidden with a job running,
|
||||
* a chip in the corner that shows the progress and brings it back. Mounted
|
||||
@@ -22,10 +25,18 @@ export function MailTransferHost() {
|
||||
|
||||
// An account that signs out takes its running job, a finished export's
|
||||
// file in the browser's private storage, and its checkpoints with it.
|
||||
// Another account signing out leaves the job alone. The job's checkpoint
|
||||
// goes after the job has stopped, so a late write can't bring it back.
|
||||
useEffect(() => onBeforeSignOut(async (accountId) => {
|
||||
const store = useMailTransferStore.getState();
|
||||
if (isJobActive(store.job)) store.cancelJob();
|
||||
else store.close();
|
||||
const job = store.job;
|
||||
if (job && (accountId === "all" || !job.scope || job.scope === accountId)) {
|
||||
if (isJobActive(job)) {
|
||||
store.cancelJob("signout");
|
||||
await Promise.race([job.finished, new Promise((resolve) => setTimeout(resolve, SIGN_OUT_WAIT_MS))]);
|
||||
}
|
||||
useMailTransferStore.getState().close();
|
||||
}
|
||||
if (accountId !== "all") await clearCheckpointsFor(accountId);
|
||||
}), []);
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { runExport, folderTree, type ExportFolder, type ExportFormat } from "@/lib/mail-export/engine";
|
||||
import type { ExportSink } from "@/lib/mail-export/sinks";
|
||||
import type { IJMAPClient } from "@/lib/jmap/client-interface";
|
||||
import { useAuthStore } from "@/stores/auth-store";
|
||||
import { useMailTransferStore } from "@/stores/mail-transfer-store";
|
||||
|
||||
@@ -14,15 +15,27 @@ export interface StartExportInput {
|
||||
|
||||
/** Runs an export as the dialog's job. */
|
||||
export function startExport(input: StartExportInput): void {
|
||||
const client = useAuthStore.getState().client;
|
||||
if (!client) return;
|
||||
const auth = useAuthStore.getState();
|
||||
if (!auth.client) return;
|
||||
const client: IJMAPClient = auth.client;
|
||||
const controller = new AbortController();
|
||||
let ended: () => void = () => {};
|
||||
const finished = new Promise<void>((resolve) => { ended = resolve; });
|
||||
useMailTransferStore.getState().startJob({
|
||||
kind: "export",
|
||||
controller,
|
||||
scope: auth.activeAccountId ?? "default",
|
||||
finished,
|
||||
progress: { phase: "running", exported: 0, failed: 0, failures: [], total: 0, fraction: 0 },
|
||||
});
|
||||
void (async () => {
|
||||
try {
|
||||
await run();
|
||||
} finally {
|
||||
ended();
|
||||
}
|
||||
})();
|
||||
async function run() {
|
||||
let folders: ExportFolder[];
|
||||
try {
|
||||
folders = input.mailboxId
|
||||
@@ -46,5 +59,5 @@ export function startExport(input: StartExportInput): void {
|
||||
onProgress: (progress) => useMailTransferStore.getState().updateProgress(progress),
|
||||
});
|
||||
useMailTransferStore.getState().updateProgress(result);
|
||||
})();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { runImport, type ImportFile } from "@/lib/mail-import/engine";
|
||||
import type { IJMAPClient } from "@/lib/jmap/client-interface";
|
||||
import { useAuthStore } from "@/stores/auth-store";
|
||||
import { useEmailStore } from "@/stores/email-store";
|
||||
import { useMailTransferStore } from "@/stores/mail-transfer-store";
|
||||
@@ -16,17 +17,29 @@ export interface StartImportInput {
|
||||
/** Runs an import as the dialog's job and refreshes the mail lists when it ends. */
|
||||
export function startImport(input: StartImportInput): void {
|
||||
const auth = useAuthStore.getState();
|
||||
const client = auth.client;
|
||||
if (!client) return;
|
||||
if (!auth.client) return;
|
||||
const client: IJMAPClient = auth.client;
|
||||
const store = useMailTransferStore.getState();
|
||||
const controller = new AbortController();
|
||||
const scope = auth.activeAccountId ?? "default";
|
||||
let ended: () => void = () => {};
|
||||
const finished = new Promise<void>((resolve) => { ended = resolve; });
|
||||
store.startJob({
|
||||
kind: "import",
|
||||
controller,
|
||||
scope,
|
||||
finished,
|
||||
progress: { phase: "running", imported: 0, failed: 0, skipped: {}, failures: [], fraction: 0, foldersCreated: 0 },
|
||||
});
|
||||
const hasPst = input.files.some((f) => f.format === "pst");
|
||||
void (async () => {
|
||||
try {
|
||||
await run();
|
||||
} finally {
|
||||
ended();
|
||||
}
|
||||
})();
|
||||
async function run() {
|
||||
// The PST reader (and its worker) load only for an Outlook file.
|
||||
const openPst = hasPst ? (await import("@/lib/mail-import/pst-source")).openPstSource : undefined;
|
||||
const result = await runImport({
|
||||
@@ -35,7 +48,7 @@ export function startImport(input: StartImportInput): void {
|
||||
targetMailboxId: input.targetMailboxId,
|
||||
keepFolders: input.keepFolders,
|
||||
files: input.files,
|
||||
scope: auth.activeAccountId ?? "default",
|
||||
scope,
|
||||
resume: input.resume,
|
||||
signal: controller.signal,
|
||||
onProgress: (progress) => useMailTransferStore.getState().updateProgress(progress),
|
||||
@@ -51,5 +64,5 @@ export function startImport(input: StartImportInput): void {
|
||||
} catch {
|
||||
/* the next refresh shows them */
|
||||
}
|
||||
})();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user