fix(web): hide Retry when blob unavailable; failed-state surfaces actionable error
Pending bubbles that survive a reload (or post-redeploy refresh) without a FileSystemFileHandle had no way to recover the original File bytes, so the Retry button silently no-op'd: resumeUpload set state back to 'paused' with no surfaced error, leaving the user stuck. - transferStore: add reactive hasInMemoryFile Set mirroring liveUploadFiles. setInMemoryRef helper keeps both in sync at every set/delete site (startUpload, resumeUpload, onSuccess, remove). Persisted shape is unchanged. - resumeUpload: when no blob is reachable (no in-memory File and no FS handle), call setError with an actionable "File no longer available — discard and re-upload" message instead of silently flipping back to 'paused'. - Message.tsx: compute canRetry reactively from transfersForRow + hasInMemoryFile; hide the Retry button when retry is infeasible. Discard remains. - AttachmentProgress: optional error prop surfaces transfer.error.message via title= on the failed-state ring for hover context. - Tests: existing 3 resumeUpload tests now assert state==='failed' with the actionable message; +2 new tests for hasInMemoryFile lifecycle (start/remove, abort retains).
This commit is contained in:
@@ -52,6 +52,12 @@ export interface CreateTransferInput {
|
||||
|
||||
interface TransferStoreState {
|
||||
transfers: Map<string, Transfer>;
|
||||
/**
|
||||
* Mirrors the keys of the module-scoped `liveUploadFiles` map so React components
|
||||
* can subscribe to "do we still hold the original File for this transfer?" reactively.
|
||||
* Session-scoped — never persisted (the underlying File refs vanish on reload).
|
||||
*/
|
||||
hasInMemoryFile: Set<string>;
|
||||
}
|
||||
|
||||
interface TransferStoreActions {
|
||||
@@ -129,8 +135,23 @@ const mapAwareStorage: PersistStorage<Pick<TransferStoreState, 'transfers'>> = {
|
||||
|
||||
export const useTransferStore = create<TransferStore>()(
|
||||
persist(
|
||||
(set, get) => ({
|
||||
(set, get) => {
|
||||
// DRY helper: keep the reactive `hasInMemoryFile` set in sync with the
|
||||
// module-scoped `liveUploadFiles` map. Both call-sites that mutate the map
|
||||
// immediately follow up with this so subscribers re-render.
|
||||
const setInMemoryRef = (id: string, present: boolean) => {
|
||||
set((s) => {
|
||||
const has = s.hasInMemoryFile.has(id);
|
||||
if (present === has) return s;
|
||||
const next = new Set(s.hasInMemoryFile);
|
||||
if (present) next.add(id); else next.delete(id);
|
||||
return { hasInMemoryFile: next };
|
||||
});
|
||||
};
|
||||
|
||||
return ({
|
||||
transfers: new Map<string, Transfer>(),
|
||||
hasInMemoryFile: new Set<string>(),
|
||||
|
||||
createTransfer: (input) => {
|
||||
const id = uuid();
|
||||
@@ -196,13 +217,16 @@ export const useTransferStore = create<TransferStore>()(
|
||||
return { transfers: next };
|
||||
}),
|
||||
|
||||
remove: (id) => set((s) => {
|
||||
if (!s.transfers.has(id)) return s;
|
||||
const next = new Map(s.transfers);
|
||||
next.delete(id);
|
||||
remove: (id) => {
|
||||
liveUploadFiles.delete(id);
|
||||
return { transfers: next };
|
||||
}),
|
||||
setInMemoryRef(id, false);
|
||||
set((s) => {
|
||||
if (!s.transfers.has(id)) return s;
|
||||
const next = new Map(s.transfers);
|
||||
next.delete(id);
|
||||
return { transfers: next };
|
||||
});
|
||||
},
|
||||
|
||||
startUpload: async (file, opts) => {
|
||||
const token = useAuthStore.getState().token;
|
||||
@@ -262,6 +286,7 @@ export const useTransferStore = create<TransferStore>()(
|
||||
} finally {
|
||||
liveUploads.delete(id);
|
||||
liveUploadFiles.delete(id);
|
||||
setInMemoryRef(id, false);
|
||||
}
|
||||
},
|
||||
onError: (err: Error) => {
|
||||
@@ -276,6 +301,7 @@ export const useTransferStore = create<TransferStore>()(
|
||||
const upload = new Upload(file as File, tusOpts);
|
||||
liveUploads.set(id, upload);
|
||||
liveUploadFiles.set(id, file);
|
||||
setInMemoryRef(id, true);
|
||||
upload.start();
|
||||
return id;
|
||||
},
|
||||
@@ -349,10 +375,14 @@ export const useTransferStore = create<TransferStore>()(
|
||||
}
|
||||
|
||||
if (!blob) {
|
||||
// No handle, no in-memory file — surface "re-pick to resume" to the user.
|
||||
// (Cross-reload on Firefox/Safari, or after MessageInput unmount cleared
|
||||
// the in-memory File and the bubble survives without an FS handle.)
|
||||
get().setState_(id, 'paused');
|
||||
// No in-memory File (post-reload) AND no FS handle to reacquire bytes from.
|
||||
// The user must discard and re-upload. Setting 'failed' lets the orchestrator's
|
||||
// mark-failed sweep keep the bubble in failed state and Message.tsx surfaces
|
||||
// the discard control. The Retry button is hidden in this case (canRetry gate).
|
||||
get().setError(id, {
|
||||
message: 'File no longer available — discard and re-upload',
|
||||
permanent: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -425,6 +455,7 @@ export const useTransferStore = create<TransferStore>()(
|
||||
} finally {
|
||||
liveUploads.delete(id);
|
||||
liveUploadFiles.delete(id);
|
||||
setInMemoryRef(id, false);
|
||||
}
|
||||
},
|
||||
onError: (err: Error) => {
|
||||
@@ -435,6 +466,11 @@ export const useTransferStore = create<TransferStore>()(
|
||||
},
|
||||
};
|
||||
|
||||
// Retain the resolved blob in the in-memory map so a subsequent retry-after-failure
|
||||
// (e.g., transient network error) can resume without going through the handle path.
|
||||
liveUploadFiles.set(id, blob);
|
||||
setInMemoryRef(id, true);
|
||||
|
||||
const upload = new Upload(blob as File, tusOpts);
|
||||
liveUploads.set(id, upload);
|
||||
upload.start();
|
||||
@@ -631,7 +667,8 @@ export const useTransferStore = create<TransferStore>()(
|
||||
listVisible: () => Array.from(get().transfers.values()).filter((t) => t.tray),
|
||||
listForChannel: (channelId) =>
|
||||
Array.from(get().transfers.values()).filter((t) => t.channelId === channelId),
|
||||
}),
|
||||
});
|
||||
},
|
||||
{
|
||||
name: 'transferStore@v1',
|
||||
storage: mapAwareStorage,
|
||||
|
||||
@@ -62,7 +62,7 @@ import { useTransferStore } from './transferStore';
|
||||
|
||||
describe('transferStore.startUpload', () => {
|
||||
beforeEach(() => {
|
||||
useTransferStore.setState({ transfers: new Map() });
|
||||
useTransferStore.setState({ transfers: new Map(), hasInMemoryFile: new Set() });
|
||||
startMock.mockClear();
|
||||
abortMock.mockClear();
|
||||
if (typeof localStorage !== 'undefined') localStorage.clear();
|
||||
@@ -118,12 +118,12 @@ describe('transferStore.startUpload', () => {
|
||||
|
||||
describe('transferStore.resumeUpload', () => {
|
||||
beforeEach(() => {
|
||||
useTransferStore.setState({ transfers: new Map() });
|
||||
useTransferStore.setState({ transfers: new Map(), hasInMemoryFile: new Set() });
|
||||
startMock.mockClear();
|
||||
abortMock.mockClear();
|
||||
});
|
||||
|
||||
it('stays paused when transfer has no tusUploadUrl and no available blob', async () => {
|
||||
it('marks failed when transfer has no available blob to resume from', async () => {
|
||||
const id = useTransferStore.getState().createTransfer({
|
||||
type: 'upload',
|
||||
file: { name: 'a.png', size: 100, mimetype: 'image/png' },
|
||||
@@ -132,12 +132,15 @@ describe('transferStore.resumeUpload', () => {
|
||||
useTransferStore.getState().setState_(id, 'paused');
|
||||
await useTransferStore.getState().resumeUpload(id);
|
||||
const t = useTransferStore.getState().get(id)!;
|
||||
// No URL + no in-memory file + no handle → can neither resume nor restart;
|
||||
// surface 'paused' to prompt the user to re-pick.
|
||||
expect(t.state).toBe('paused');
|
||||
// No URL + no in-memory file + no handle → cannot resume or restart.
|
||||
// Surface 'failed' with an actionable message so the UI shows Discard
|
||||
// and hides the Retry button (which would silently no-op).
|
||||
expect(t.state).toBe('failed');
|
||||
expect(t.error?.message ?? '').toMatch(/file no longer available/i);
|
||||
expect(t.error?.permanent).toBe(true);
|
||||
});
|
||||
|
||||
it('stays paused when tus URL has expired and no available blob', async () => {
|
||||
it('marks failed when tus URL has expired and no available blob', async () => {
|
||||
const id = useTransferStore.getState().createTransfer({
|
||||
type: 'upload',
|
||||
file: { name: 'a.png', size: 100, mimetype: 'image/png' },
|
||||
@@ -146,11 +149,13 @@ describe('transferStore.resumeUpload', () => {
|
||||
useTransferStore.getState().setTusUrl(id, '/api/files/expired', Date.now() - 1000);
|
||||
useTransferStore.getState().setState_(id, 'paused');
|
||||
await useTransferStore.getState().resumeUpload(id);
|
||||
// Expired URL with no blob to restart → stay paused, user re-picks.
|
||||
expect(useTransferStore.getState().get(id)!.state).toBe('paused');
|
||||
const t = useTransferStore.getState().get(id)!;
|
||||
// Expired URL with no blob to restart → failed, user must discard and re-upload.
|
||||
expect(t.state).toBe('failed');
|
||||
expect(t.error?.message ?? '').toMatch(/file no longer available/i);
|
||||
});
|
||||
|
||||
it('stays paused when no FS handle is available (re-pick required)', async () => {
|
||||
it('marks failed when no FS handle is available (re-pick required)', async () => {
|
||||
const id = useTransferStore.getState().createTransfer({
|
||||
type: 'upload',
|
||||
file: { name: 'a.png', size: 100, mimetype: 'image/png' },
|
||||
@@ -160,6 +165,54 @@ describe('transferStore.resumeUpload', () => {
|
||||
useTransferStore.getState().setTusUrl(id, '/api/files/abc', Date.now() + 60_000);
|
||||
useTransferStore.getState().setState_(id, 'paused');
|
||||
await useTransferStore.getState().resumeUpload(id);
|
||||
expect(useTransferStore.getState().get(id)!.state).toBe('paused');
|
||||
const t = useTransferStore.getState().get(id)!;
|
||||
expect(t.state).toBe('failed');
|
||||
expect(t.error?.message ?? '').toMatch(/file no longer available/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe('transferStore.hasInMemoryFile', () => {
|
||||
beforeEach(() => {
|
||||
useTransferStore.setState({ transfers: new Map(), hasInMemoryFile: new Set() });
|
||||
startMock.mockClear();
|
||||
abortMock.mockClear();
|
||||
if (typeof localStorage !== 'undefined') localStorage.clear();
|
||||
});
|
||||
|
||||
it('hasInMemoryFile tracks startUpload + remove lifecycle', async () => {
|
||||
const file = new File([new Uint8Array(100)], 'a.png', { type: 'image/png' });
|
||||
const id = await useTransferStore.getState().startUpload(file, { tray: true });
|
||||
// The mock's onSuccess fires synchronously inside start(), which clears the
|
||||
// in-memory ref. So here we should see it cleared (transfer is 'completed').
|
||||
expect(useTransferStore.getState().get(id)!.state).toBe('completed');
|
||||
expect(useTransferStore.getState().hasInMemoryFile.has(id)).toBe(false);
|
||||
useTransferStore.getState().remove(id);
|
||||
expect(useTransferStore.getState().hasInMemoryFile.has(id)).toBe(false);
|
||||
});
|
||||
|
||||
it('hasInMemoryFile survives abortUpload (file retained for retry)', async () => {
|
||||
// Simulate an in-flight transfer (pre-success) by manually populating the
|
||||
// store and the in-memory file ref via a fresh transfer that hasn't completed.
|
||||
// We do this by intercepting startUpload before the mock resolves: createTransfer
|
||||
// and prime hasInMemoryFile + state directly to mirror an in-flight upload.
|
||||
const id = useTransferStore.getState().createTransfer({
|
||||
type: 'upload',
|
||||
file: { name: 'a.png', size: 100, mimetype: 'image/png' },
|
||||
tray: true,
|
||||
});
|
||||
// Prime the reactive set to mirror a live upload.
|
||||
useTransferStore.setState((s) => {
|
||||
const next = new Set(s.hasInMemoryFile);
|
||||
next.add(id);
|
||||
return { hasInMemoryFile: next };
|
||||
});
|
||||
useTransferStore.getState().setState_(id, 'active');
|
||||
|
||||
expect(useTransferStore.getState().hasInMemoryFile.has(id)).toBe(true);
|
||||
useTransferStore.getState().abortUpload(id);
|
||||
// abortUpload must NOT clear the in-memory ref — the user can still retry.
|
||||
expect(useTransferStore.getState().hasInMemoryFile.has(id)).toBe(true);
|
||||
useTransferStore.getState().remove(id);
|
||||
expect(useTransferStore.getState().hasInMemoryFile.has(id)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user