feat: real byte progress for attachment uploads, stock and plugin-offloaded

The composer chip only knew a boolean: `uploading` flipped on, an
indeterminate bar animated at 40% width, and nothing anywhere carried a
percentage, so a large attachment on a slow link looked frozen for the
whole transfer. The JMAP client has been able to report real progress
since #333 gave uploadBlob an XHR path with onProgress; the composer
just never passed it.

Stock path: pass onProgress and the existing AbortController's signal to
client.uploadBlob. The chip's bar becomes determinate as soon as bytes
are reported (falling back to the indeterminate animation otherwise),
and cancel now aborts the transfer itself instead of being checked only
once the upload has finished.

Plugin offload path (#751): a plugin that offloads an attachment moves
the bytes itself via api.http.post, which the composer cannot observe.
It also cannot be handed a callback: the sandbox bridge marshals
functions host-to-plugin only, and the File a plugin holds is a
structured clone, so object identity cannot correlate its request back
to an attachment. The staged file id can: the composer generated it and
handed it to onBeforeBlobUpload. A plugin echoes it back as the new
optional `progressFileId` on the post options; the host sends the Blob
via XMLHttpRequest (fetch exposes no upload progress events) and reports
bytes into a host-side registry keyed by that id, which the composer
subscribes to before running the hook. Progress never crosses into the
sandbox, and reports for an id the composer is not tracking are dropped,
so the plugin API grows exactly one optional field. Calls without
progressFileId keep the fetch path untouched.

Closes #754
This commit is contained in:
Roman Varlakov
2026-08-23 19:44:56 +06:00
parent 9a19ddbbac
commit 7928f72ee2
5 changed files with 393 additions and 51 deletions
+90 -50
View File
@@ -18,6 +18,7 @@ import { isEditableEventTarget } from "@/lib/keyboard";
import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html";
import { buildSignatureBlock, containsEmbeddedSignature, SIGNATURE_RANGE_MARKER } from "@/components/email/signature-block";
import { emailHooks, contactHooks, isExternalAttachmentResult } from "@/lib/plugin-hooks";
import { onUploadProgress } from "@/lib/upload-progress";
import type { AlmostSavedDraft, OutgoingEmail, RecipientSuggestion } from "@/lib/plugin-types";
import { useAuthStore } from "@/stores/auth-store";
import { useIdentityStore } from "@/stores/identity-store";
@@ -217,6 +218,14 @@ type ComposerAttachment = {
size: number;
blobId?: string;
uploading?: boolean;
/**
* Transferred share of the upload, 0-100. Set while `uploading` when the
* transport reports byte progress: the stock path via the JMAP client's
* `onProgress`, a plugin offload via the staged-file-id registry
* (`lib/upload-progress.ts`). Absent when nothing has reported yet, in
* which case the chip falls back to its indeterminate bar.
*/
progress?: number;
error?: boolean;
abortController?: AbortController;
// Carried through for parts hydrated from an existing draft so inline
@@ -1387,60 +1396,82 @@ export function EmailComposer({
const fileId = generateUUID();
await fileStorage.saveFile(fileId, file);
const transformed = await emailHooks.onBeforeBlobUpload.transform<unknown>(fileId);
// A handler can offload the file elsewhere and hand back replacement
// content instead of a file id. Drop the binary attachment and put the
// replacement in the body.
if (isExternalAttachmentResult(transformed)) {
// `transformed.fileId` when the handler re-saved the staged file
// under a new id; otherwise the id we handed it.
await fileStorage.deleteFile(transformed.fileId ?? fileId);
// The user may have removed the attachment while the handler was
// offloading it - don't drop a link for a file they cancelled.
if (controller?.signal.aborted) continue;
setAttachments(prev => prev.filter(att => att.file !== file));
if (plainTextMode) {
setBody(previous =>
previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text
);
} else {
// Never trust plugin markup in the document (or in the message the
// user then sends); insert through the editor so it lands at the
// caret rather than after the signature and quoted block.
const html = sanitizePluginBodyHtml(transformed.html);
if (!html) continue;
if (editorRef.current) {
editorRef.current.chain().focus().insertContent(html).run();
// Byte progress for the chip, from whichever transport moves the
// bytes: the JMAP client below reports directly, a plugin offloading
// via `api.http.post` reports through the staged-file-id registry
// (it echoes `fileId` as `progressFileId`, see doHttpPost).
const reportProgress = (loaded: number, total: number) => {
if (total <= 0 || controller?.signal.aborted) return;
const pct = Math.min(100, Math.floor((loaded / total) * 100));
setAttachments(prev =>
prev.map(att => (att.file === file ? { ...att, progress: pct } : att))
);
};
const stopProgress = onUploadProgress(fileId, reportProgress);
try {
const transformed = await emailHooks.onBeforeBlobUpload.transform<unknown>(fileId);
// A handler can offload the file elsewhere and hand back replacement
// content instead of a file id. Drop the binary attachment and put the
// replacement in the body.
if (isExternalAttachmentResult(transformed)) {
// `transformed.fileId` when the handler re-saved the staged file
// under a new id; otherwise the id we handed it.
await fileStorage.deleteFile(transformed.fileId ?? fileId);
// The user may have removed the attachment while the handler was
// offloading it - don't drop a link for a file they cancelled.
if (controller?.signal.aborted) continue;
setAttachments(prev => prev.filter(att => att.file !== file));
if (plainTextMode) {
setBody(previous =>
previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text
);
} else {
setBody(previous => previous + html);
// Never trust plugin markup in the document (or in the message the
// user then sends); insert through the editor so it lands at the
// caret rather than after the signature and quoted block.
const html = sanitizePluginBodyHtml(transformed.html);
if (!html) continue;
if (editorRef.current) {
editorRef.current.chain().focus().insertContent(html).run();
} else {
setBody(previous => previous + html);
}
}
continue;
}
continue;
const newFileId = typeof transformed === 'string' ? transformed : fileId;
const newFile = await fileStorage.getFile(newFileId) || file;
await fileStorage.deleteFile(newFileId);
// Passing the signal also makes cancel abort the transfer itself,
// instead of only being checked once the upload has finished.
const { blobId } = await client.uploadBlob(newFile, {
onProgress: reportProgress,
signal: controller?.signal,
});
if (controller?.signal.aborted) continue;
setAttachments(prev =>
prev.map(att =>
att.file === file
? { ...att, blobId, uploading: false, abortController: undefined }
: att
)
);
emailHooks.onAfterAttachmentUpload.emit({
name: file.name,
type: file.type || 'application/octet-stream',
size: file.size,
blobId,
});
} finally {
stopProgress();
}
const newFileId = typeof transformed === 'string' ? transformed : fileId;
const newFile = await fileStorage.getFile(newFileId) || file;
await fileStorage.deleteFile(newFileId);
const { blobId } = await client.uploadBlob(newFile);
if (controller?.signal.aborted) continue;
setAttachments(prev =>
prev.map(att =>
att.file === file
? { ...att, blobId, uploading: false, abortController: undefined }
: att
)
);
emailHooks.onAfterAttachmentUpload.emit({
name: file.name,
type: file.type || 'application/octet-stream',
size: file.size,
blobId,
});
} catch (error) {
if (controller?.signal.aborted) continue;
debug.error(`Failed to upload ${file.name}:`, error);
@@ -2891,7 +2922,16 @@ export function EmailComposer({
{att.uploading && (
<div className="absolute inset-0 pointer-events-none">
<div className="h-full bg-primary/10 animate-pulse" />
<div className="absolute bottom-0 left-0 h-0.5 bg-primary/40 animate-[indeterminate_1.5s_ease-in-out_infinite]" style={{ width: '40%' }} />
{typeof att.progress === 'number' ? (
// Real byte progress from the transport; see
// ComposerAttachment.progress for who reports it.
<div
className="absolute bottom-0 left-0 h-0.5 bg-primary/60 transition-[width] duration-300 ease-out"
style={{ width: `${att.progress}%` }}
/>
) : (
<div className="absolute bottom-0 left-0 h-0.5 bg-primary/40 animate-[indeterminate_1.5s_ease-in-out_infinite]" style={{ width: '40%' }} />
)}
</div>
)}
<div className="relative flex items-center gap-2">
@@ -0,0 +1,154 @@
import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest';
import type { InstalledPlugin } from '../plugin-types';
import { useAuthStore } from '@/stores/auth-store';
import { onUploadProgress } from '../upload-progress';
vi.mock('@/stores/toast-store', () => ({
toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() },
}));
import { dispatchApiCall } from '../plugin-sandbox/host-api';
function plugin(overrides: Partial<InstalledPlugin> = {}): InstalledPlugin {
return {
id: 'p1',
name: 'Test plugin',
version: '1.0.0',
author: 'test',
description: '',
type: 'hook',
permissions: ['http:post'],
grantedPermissions: ['http:post'],
entrypoint: 'index.js',
enabled: true,
status: 'running',
settings: {},
apiPostPaths: ['/api/upload'],
...overrides,
};
}
/**
* Stand-in for XMLHttpRequest that records the request and lets the test
* drive upload progress and completion by hand.
*/
class FakeXHR {
static instances: FakeXHR[] = [];
method = '';
url = '';
headers: Record<string, string> = {};
sentBody: unknown = null;
status = 200;
responseText = '{"url":"https://example.test/d/1"}';
upload: { onprogress: ((e: { lengthComputable: boolean; loaded: number; total: number }) => void) | null } = { onprogress: null };
onload: (() => void) | null = null;
onerror: (() => void) | null = null;
ontimeout: (() => void) | null = null;
open(method: string, url: string) { this.method = method; this.url = url; }
setRequestHeader(name: string, value: string) { this.headers[name] = value; }
send(body: unknown) { this.sentBody = body; FakeXHR.instances.push(this); }
}
describe('plugin binary upload progress', () => {
beforeEach(() => {
FakeXHR.instances = [];
vi.stubGlobal('XMLHttpRequest', FakeXHR);
useAuthStore.setState({
client: {
getAuthHeader: () => 'Basic dGVzdA==',
getUsername: () => 'tester',
} as never,
});
});
afterEach(() => {
vi.unstubAllGlobals();
useAuthStore.setState({ client: null });
});
it('reports byte progress into the registry keyed by progressFileId', async () => {
const seen: Array<[number, number]> = [];
const off = onUploadProgress('staged-1', (loaded, total) => seen.push([loaded, total]));
const call = dispatchApiCall(plugin(), 'http.post', [
'/api/upload',
new Blob(['0123456789']),
{ progressFileId: 'staged-1' },
]);
// send() is synchronous inside the promise executor, so the request is
// already captured even though the call promise is still pending.
expect(FakeXHR.instances).toHaveLength(1);
const xhr = FakeXHR.instances[0];
xhr.upload.onprogress?.({ lengthComputable: true, loaded: 4, total: 10 });
xhr.upload.onprogress?.({ lengthComputable: true, loaded: 10, total: 10 });
xhr.onload?.();
const result = await call as { ok: boolean; status: number; data: unknown };
expect(seen).toEqual([[4, 10], [10, 10]]);
expect(result.ok).toBe(true);
expect(result.data).toEqual({ url: 'https://example.test/d/1' });
off();
});
it('still applies credential headers after plugin headers on the XHR path', async () => {
const call = dispatchApiCall(plugin(), 'http.post', [
'/api/upload',
new Blob(['x'], { type: 'application/octet-stream' }),
{ headers: { 'X-Plugin-Name': 'a.bin' }, progressFileId: 'staged-2' },
]);
const xhr = FakeXHR.instances[0];
expect(xhr.headers['Authorization']).toBe('Basic dGVzdA==');
expect(xhr.headers['X-JMAP-Username']).toBe('tester');
expect(xhr.headers['X-Plugin-Name']).toBe('a.bin');
xhr.onload?.();
await call;
});
it('skips progress events whose length is not computable', async () => {
const listener = vi.fn();
const off = onUploadProgress('staged-3', listener);
const call = dispatchApiCall(plugin(), 'http.post', [
'/api/upload',
new Blob(['x']),
{ progressFileId: 'staged-3' },
]);
const xhr = FakeXHR.instances[0];
xhr.upload.onprogress?.({ lengthComputable: false, loaded: 0, total: 0 });
xhr.onload?.();
await call;
expect(listener).not.toHaveBeenCalled();
off();
});
it('rejects a malformed progressFileId', async () => {
await expect(
dispatchApiCall(plugin(), 'http.post', [
'/api/upload',
new Blob(['x']),
{ progressFileId: 'x'.repeat(129) },
]),
).rejects.toThrow(/progressFileId/);
expect(FakeXHR.instances).toHaveLength(0);
});
it('keeps the fetch path when no progressFileId is given', async () => {
const fetchSpy = vi.fn().mockResolvedValue(
new Response(JSON.stringify({ ok: 1 }), { status: 200 }),
);
vi.stubGlobal('fetch', fetchSpy);
await dispatchApiCall(plugin(), 'http.post', ['/api/upload', new Blob(['x']), {}]);
expect(fetchSpy).toHaveBeenCalledTimes(1);
expect(FakeXHR.instances).toHaveLength(0);
});
it('surfaces a network error as a rejection', async () => {
const call = dispatchApiCall(plugin(), 'http.post', [
'/api/upload',
new Blob(['x']),
{ progressFileId: 'staged-4' },
]);
FakeXHR.instances[0].onerror?.();
await expect(call).rejects.toThrow(/Network error/);
});
});
+50
View File
@@ -0,0 +1,50 @@
import { describe, it, expect, vi } from 'vitest';
import { onUploadProgress, reportUploadProgress } from '../upload-progress';
describe('upload progress registry', () => {
it('delivers reports to the listener registered for the id', () => {
const listener = vi.fn();
const off = onUploadProgress('file-1', listener);
reportUploadProgress('file-1', 512, 2048);
expect(listener).toHaveBeenCalledWith(512, 2048);
off();
});
it('drops reports for an id nobody listens to', () => {
const listener = vi.fn();
const off = onUploadProgress('file-1', listener);
reportUploadProgress('file-2', 1, 2);
expect(listener).not.toHaveBeenCalled();
off();
});
it('stops delivering after unsubscribe', () => {
const listener = vi.fn();
const off = onUploadProgress('file-1', listener);
off();
reportUploadProgress('file-1', 1, 2);
expect(listener).not.toHaveBeenCalled();
});
it('ignores non-finite byte counts', () => {
const listener = vi.fn();
const off = onUploadProgress('file-1', listener);
reportUploadProgress('file-1', NaN, 100);
reportUploadProgress('file-1', 50, Infinity);
expect(listener).not.toHaveBeenCalled();
off();
});
it("unsubscribing a stale registration does not drop the id's current listener", () => {
const first = vi.fn();
const second = vi.fn();
const offFirst = onUploadProgress('file-1', first);
const offSecond = onUploadProgress('file-1', second);
// The composer re-registered the id; the stale unsubscribe must be a no-op.
offFirst();
reportUploadProgress('file-1', 10, 100);
expect(second).toHaveBeenCalledWith(10, 100);
expect(first).not.toHaveBeenCalled();
offSecond();
});
});
+57 -1
View File
@@ -18,7 +18,8 @@ import {
type KeywordVisibility,
} from '@/stores/settings-store';
import type { MessageListTabsConfig } from '../plugin-types';
import { apiFetch } from '../browser-navigation';
import { apiFetch, getPathPrefix } from '../browser-navigation';
import { reportUploadProgress } from '../upload-progress';
import { DEFAULT_KEYWORD_SCAN_LIMIT } from '../jmap/client';
import { suggestKeywordColor } from '../keyword-discovery';
import { MAX_KEYWORD_LENGTH } from '../keyword-nesting';
@@ -286,6 +287,15 @@ function isApiPostPathAllowed(path: string, allowlist: readonly string[]): boole
interface PluginHttpPostOptions {
headers?: Record<string, string>;
/**
* Staged-attachment file id (the one `onBeforeBlobUpload` handed to the
* plugin) to report byte-level upload progress for. Only meaningful on a
* `Blob`/`File` body. Progress goes to the host-side registry the composer
* listens on (`lib/upload-progress.ts`), never back into the sandbox, so
* the attachment chip can show a real percentage while a plugin offloads
* the file. Reports for an id the composer isn't tracking are dropped.
*/
progressFileId?: string;
}
/**
@@ -356,6 +366,21 @@ async function doHttpPost(
headers['Authorization'] = client.getAuthHeader();
headers['X-JMAP-Username'] = client.getUsername();
}
// Progress requires XMLHttpRequest: fetch() exposes no upload progress
// events (request streaming is Chrome-only), and the JMAP client's blob
// upload already made the same trade for the same reason (#333). Callers
// that don't ask for progress keep the fetch path untouched.
if (body instanceof Blob && options?.progressFileId !== undefined) {
const fileId = options.progressFileId;
if (typeof fileId !== 'string' || fileId.length === 0 || fileId.length > 128) {
throw new Error('progressFileId must be a non-empty string of at most 128 characters');
}
return xhrPost(url.pathname + url.search, headers, body, (loaded, total) =>
reportUploadProgress(fileId, loaded, total),
);
}
const res = await apiFetch(url.pathname + url.search, {
method: 'POST',
headers,
@@ -365,6 +390,37 @@ async function doHttpPost(
return { ok: res.ok, status: res.status, data };
}
/**
* POST via XMLHttpRequest so `xhr.upload.onprogress` can report transferred
* bytes. Same shape of result as the fetch path in `doHttpPost`; the path is
* prefixed exactly like `apiFetch` does for a same-origin `/api/*` path.
*/
function xhrPost(
path: string,
headers: Record<string, string>,
body: Blob,
onProgress: (loaded: number, total: number) => void,
): Promise<{ ok: boolean; status: number; data: unknown }> {
return new Promise((resolve, reject) => {
const xhr = new XMLHttpRequest();
xhr.open('POST', getPathPrefix() + path);
for (const [name, value] of Object.entries(headers)) {
xhr.setRequestHeader(name, value);
}
xhr.upload.onprogress = (event) => {
if (event.lengthComputable) onProgress(event.loaded, event.total);
};
xhr.onload = () => {
let data: unknown = null;
try { data = JSON.parse(xhr.responseText); } catch { /* non-JSON body */ }
resolve({ ok: xhr.status >= 200 && xhr.status < 300, status: xhr.status, data });
};
xhr.onerror = () => reject(new Error('Network error during plugin upload'));
xhr.ontimeout = () => reject(new Error('Plugin upload timed out'));
xhr.send(body);
});
}
// ─── http.fetch (cross-origin, manifest-allowlisted) ──────────
interface PluginFetchInit {
+42
View File
@@ -0,0 +1,42 @@
/**
* Byte-level progress for staged-attachment uploads, keyed by the staged file
* id the composer hands to `onBeforeBlobUpload`.
*
* The composer cannot observe a plugin's own transfer directly: a plugin that
* offloads an attachment (see `isExternalAttachmentResult`) uploads the bytes
* itself via `api.http.post`, and the File it holds is a structured clone from
* the sandbox boundary, so object identity can't correlate that request back
* to an attachment. The staged file id can: the composer generated it, handed
* it to the hook, and the plugin passes it back via `progressFileId` on the
* post options. The host counts the bytes (see `doHttpPost`) and reports them
* here; the composer subscribes before running the hook.
*
* Progress never crosses into the sandbox. Reports for an id nobody listens to
* are dropped, so a plugin sending noise for ids it was never handed reaches
* nothing.
*/
export type UploadProgressListener = (loaded: number, total: number) => void;
const listeners = new Map<string, UploadProgressListener>();
/**
* Subscribe to progress reports for one staged file id. Returns the
* unsubscribe function. One listener per id: the composer owns the staged id,
* nothing else has a reason to listen.
*/
export function onUploadProgress(fileId: string, listener: UploadProgressListener): () => void {
listeners.set(fileId, listener);
return () => {
// Only clear our own registration, in case the id was re-registered.
if (listeners.get(fileId) === listener) listeners.delete(fileId);
};
}
/** Report transferred bytes for a staged file id. No listener, no effect. */
export function reportUploadProgress(fileId: string, loaded: number, total: number): void {
const listener = listeners.get(fileId);
if (!listener) return;
if (!Number.isFinite(loaded) || !Number.isFinite(total)) return;
listener(loaded, total);
}