Skip to content

Commit 346d6a2

Browse files
Programcodeclaude
andcommitted
A save that never ran for its account is not reported as kept
A queue save whose account changed before it ran wrote nothing but still resolved as a success: its caller heard the change was kept on the phone, and the storage-failure flag was cleared. It now rejects those callers with the account-changed error (the switch removes that account's queue, as before) and leaves the flag alone (tested; the test fails without it). A concurrency test pins that a change whose retirement save fails is sent once, however often replay is asked: the sent change has left the queue before that save, and replay is single-flight, so clearing inFlight there cannot cause a second send. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 4638ab5 commit 346d6a2

2 files changed

Lines changed: 46 additions & 3 deletions

File tree

‎apps/mobile/src/lib/offline-queue.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
33
import type { Source } from './sources';
44

55
const storage = vi.hoisted(() => new Map<string, string>());
6-
const net = vi.hoisted(() => ({ offline: false, user: 'user-a' as string | null, uuids: 0, failGet: 0, failSet: 0 }));
6+
const net = vi.hoisted(() => ({ offline: false, user: 'user-a' as string | null, uuids: 0, failGet: 0, failSet: 0, holdSet: null as Promise<void> | null }));
77
// supabase-js answers a request that never reached the server with status 0; readFailed marks it unreachable.
88
// The saved copies, kept in memory so a test can see what an offline relaunch would show.
99
const copies = vi.hoisted(() => new Map<string, unknown>());
@@ -43,6 +43,7 @@ vi.mock('@react-native-async-storage/async-storage', () => ({
4343
return storage.get(key) ?? null;
4444
},
4545
setItem: async (key: string, value: string) => {
46+
if (net.holdSet) await net.holdSet;
4647
if (net.failSet > 0) {
4748
net.failSet -= 1;
4849
throw new Error('storage full');
@@ -98,6 +99,7 @@ beforeEach(async () => {
9899
net.uuids = 0;
99100
net.failGet = 0;
100101
net.failSet = 0;
102+
net.holdSet = null;
101103
vi.clearAllMocks();
102104
sources.addSource.mockImplementation(async (_userId: string, input: { id: string; title: string; content: string; debateId: string }) => row(input.id, input));
103105
sources.updateSource.mockResolvedValue(undefined);
@@ -208,6 +210,39 @@ describe('offline queue', () => {
208210
expect(sources.updateSource).not.toHaveBeenCalled(); net.failSet = 0;
209211
});
210212

213+
it('never tells a caller a change is kept when the account changed before its save ran', async () => {
214+
const queue = await load();
215+
net.offline = true;
216+
let release: () => void = () => undefined;
217+
net.holdSet = new Promise<void>((resolve) => { release = resolve; });
218+
// The first save waits in storage; the second is queued behind it.
219+
const first = queue.queueAddSource('user-a', { title: 'First', content: 'Text', type: 'text', debateId: 'd1' });
220+
await new Promise((resolve) => setTimeout(resolve, 20));
221+
const second = queue.queueAddSource('user-a', { title: 'Second', content: 'Text', type: 'text', debateId: 'd1' });
222+
await new Promise((resolve) => setTimeout(resolve, 20));
223+
queue.setQueueOwner('user-b');
224+
net.holdSet = null;
225+
release();
226+
// The second save never ran for its account, so it is not reported as kept.
227+
await expect(second).rejects.toThrow('account changed');
228+
await first.catch(() => undefined);
229+
});
230+
231+
it('sends a change once when the save that retires it fails, however often replay is asked', async () => {
232+
const queue = await load();
233+
net.offline = true;
234+
await queue.queueUpdateSource('user-a', 'd1', 's1', { title: 'One' });
235+
await queue.queueUpdateSource('user-a', 'd1', 's2', { title: 'Two' });
236+
// Back online, every save fails: "One" is sent, the save that retires it fails, and replay stops.
237+
net.failSet = 1000;
238+
net.offline = false;
239+
await Promise.all([queue.replayOfflineQueue(), queue.replayOfflineQueue()]);
240+
await queue.replayOfflineQueue();
241+
expect(sent()).toEqual(['edit s1 {"title":"One"}']);
242+
expect(queue.getQueueSnapshot().items.map((item) => (item.kind === 'edit' ? item.sourceId : ''))).toEqual(['s2']);
243+
net.failSet = 0;
244+
});
245+
211246
it('never writes over a stored queue it could not read, and keeps it once a read works', async () => {
212247
let queue = await load();
213248
net.offline = true;

‎apps/mobile/src/lib/offline-queue.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,8 @@ function persist(): Promise<boolean> {
184184
awaitingSave = [];
185185
if (!userId) return Promise.resolve(true);
186186
return storage(async () => {
187-
if (gen !== generation) return;
187+
// The account changed before this save ran: nothing is written, so nothing may count as kept.
188+
if (gen !== generation) return false;
188189
// A stored queue that could not be read is never written over: it may hold unsynced work.
189190
if (!hydrated) throw new Error('The offline changes on this phone could not be read.');
190191
const write = () =>
@@ -196,8 +197,15 @@ function persist(): Promise<boolean> {
196197
await new Promise((resolve) => setTimeout(resolve, 200));
197198
await write();
198199
});
200+
return true;
199201
}).then(
200-
() => {
202+
(wrote) => {
203+
if (!wrote) {
204+
// The changes belonged to the account that was replaced; it removed them, as a sign-out does.
205+
const error = new Error('The account changed before this change was saved.');
206+
confirming.forEach(({ waiter }) => waiter.reject(error));
207+
return false;
208+
}
201209
const recovered = storageTroubled;
202210
storageTroubled = false;
203211
confirming.forEach(({ waiter }) => waiter.resolve('queued'));

0 commit comments

Comments
 (0)