Make pagination requests abortable

This commit is contained in:
Jack Kingsman
2026-03-16 15:18:38 -07:00
parent 4277e0c924
commit 58b34a6a2f
34 changed files with 170 additions and 16 deletions
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
+46 -14
View File
@@ -161,6 +161,8 @@ export function useConversationMessages(
const [loadingNewer, setLoadingNewer] = useState(false);
const abortControllerRef = useRef<AbortController | null>(null);
const olderAbortControllerRef = useRef<AbortController | null>(null);
const newerAbortControllerRef = useRef<AbortController | null>(null);
const fetchingConversationIdRef = useRef<string | null>(null);
const latestReconcileRequestIdRef = useRef(0);
const messagesRef = useRef<Message[]>([]);
@@ -306,14 +308,19 @@ export function useConversationMessages(
loadingOlderRef.current = true;
setLoadingOlder(true);
const controller = new AbortController();
olderAbortControllerRef.current = controller;
try {
const data = await api.getMessages({
type: activeConversation.type === 'channel' ? 'CHAN' : 'PRIV',
conversation_key: conversationId,
limit: MESSAGE_PAGE_SIZE,
before: oldestMessage.received_at,
before_id: oldestMessage.id,
});
const data = await api.getMessages(
{
type: activeConversation.type === 'channel' ? 'CHAN' : 'PRIV',
conversation_key: conversationId,
limit: MESSAGE_PAGE_SIZE,
before: oldestMessage.received_at,
before_id: oldestMessage.id,
},
controller.signal
);
if (fetchingConversationIdRef.current !== conversationId) return;
@@ -335,11 +342,17 @@ export function useConversationMessages(
}
setHasOlderMessages(dataWithPendingAck.length >= MESSAGE_PAGE_SIZE);
} catch (err) {
if (isAbortError(err)) {
return;
}
console.error('Failed to fetch older messages:', err);
toast.error('Failed to load older messages', {
description: err instanceof Error ? err.message : 'Check your connection',
});
} finally {
if (olderAbortControllerRef.current === controller) {
olderAbortControllerRef.current = null;
}
loadingOlderRef.current = false;
setLoadingOlder(false);
}
@@ -361,14 +374,19 @@ export function useConversationMessages(
if (!newestMessage) return;
setLoadingNewer(true);
const controller = new AbortController();
newerAbortControllerRef.current = controller;
try {
const data = await api.getMessages({
type: activeConversation.type === 'channel' ? 'CHAN' : 'PRIV',
conversation_key: conversationId,
limit: MESSAGE_PAGE_SIZE,
after: newestMessage.received_at,
after_id: newestMessage.id,
});
const data = await api.getMessages(
{
type: activeConversation.type === 'channel' ? 'CHAN' : 'PRIV',
conversation_key: conversationId,
limit: MESSAGE_PAGE_SIZE,
after: newestMessage.received_at,
after_id: newestMessage.id,
},
controller.signal
);
if (fetchingConversationIdRef.current !== conversationId) return;
@@ -385,11 +403,17 @@ export function useConversationMessages(
}
setHasNewerMessages(dataWithPendingAck.length >= MESSAGE_PAGE_SIZE);
} catch (err) {
if (isAbortError(err)) {
return;
}
console.error('Failed to fetch newer messages:', err);
toast.error('Failed to load newer messages', {
description: err instanceof Error ? err.message : 'Check your connection',
});
} finally {
if (newerAbortControllerRef.current === controller) {
newerAbortControllerRef.current = null;
}
setLoadingNewer(false);
}
}, [activeConversation, applyPendingAck, hasNewerMessages, loadingNewer, messages]);
@@ -420,6 +444,14 @@ export function useConversationMessages(
if (abortControllerRef.current) {
abortControllerRef.current.abort();
}
if (olderAbortControllerRef.current) {
olderAbortControllerRef.current.abort();
olderAbortControllerRef.current = null;
}
if (newerAbortControllerRef.current) {
newerAbortControllerRef.current.abort();
newerAbortControllerRef.current = null;
}
const prevId = prevConversationIdRef.current;
const newId = activeConversation?.id ?? null;
@@ -2,20 +2,28 @@ import { act, renderHook, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi, type Mock } from 'vitest';
import * as messageCache from '../messageCache';
import { api } from '../api';
import { useConversationMessages } from '../hooks/useConversationMessages';
import type { Conversation, Message } from '../types';
const mockGetMessages = vi.fn<(...args: unknown[]) => Promise<Message[]>>();
const mockGetMessages = vi.fn<typeof api.getMessages>();
const mockGetMessagesAround = vi.fn();
vi.mock('../api', () => ({
api: {
getMessages: (...args: unknown[]) => mockGetMessages(...args),
getMessages: (...args: Parameters<typeof api.getMessages>) => mockGetMessages(...args),
getMessagesAround: (...args: unknown[]) => mockGetMessagesAround(...args),
},
isAbortError: (err: unknown) => err instanceof DOMException && err.name === 'AbortError',
}));
const mockToastError = vi.fn();
vi.mock('../components/ui/sonner', () => ({
toast: {
error: (...args: unknown[]) => mockToastError(...args),
},
}));
function createConversation(): Conversation {
return {
type: 'contact',
@@ -55,6 +63,7 @@ describe('useConversationMessages ACK ordering', () => {
beforeEach(() => {
mockGetMessages.mockReset();
messageCache.clear();
mockToastError.mockReset();
});
it('applies buffered ACK when message is added after ACK event', async () => {
@@ -447,6 +456,55 @@ describe('useConversationMessages older-page dedup and reentry', () => {
expect(result.current.messages.filter((msg) => msg.id === 0)).toHaveLength(1);
expect(result.current.messages).toHaveLength(201);
});
it('aborts stale older-page requests on conversation switch without toasting', async () => {
const convA: Conversation = { type: 'contact', id: 'conv_a', name: 'Contact A' };
const convB: Conversation = { type: 'contact', id: 'conv_b', name: 'Contact B' };
const fullPage = Array.from({ length: 200 }, (_, i) =>
createMessage({
id: i + 1,
conversation_key: 'conv_a',
text: `msg-${i + 1}`,
sender_timestamp: 1700000000 + i,
received_at: 1700000000 + i,
})
);
mockGetMessages.mockResolvedValueOnce(fullPage);
const olderDeferred = createDeferred<Message[]>();
let olderSignal: AbortSignal | undefined;
mockGetMessages.mockImplementationOnce((_, signal?: AbortSignal) => {
olderSignal = signal;
signal?.addEventListener('abort', () => {
olderDeferred.resolve([]);
});
return new Promise<Message[]>((_, reject) => {
signal?.addEventListener('abort', () => {
reject(new DOMException('The operation was aborted', 'AbortError'));
});
});
});
const { result, rerender } = renderHook(
({ conv }: { conv: Conversation }) => useConversationMessages(conv),
{ initialProps: { conv: convA } }
);
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
act(() => {
void result.current.fetchOlderMessages();
});
await waitFor(() => expect(result.current.loadingOlder).toBe(true));
mockGetMessages.mockResolvedValueOnce([createMessage({ id: 999, conversation_key: 'conv_b' })]);
rerender({ conv: convB });
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
expect(olderSignal?.aborted).toBe(true);
expect(mockToastError).not.toHaveBeenCalled();
});
});
describe('useConversationMessages forward pagination', () => {
@@ -454,6 +512,7 @@ describe('useConversationMessages forward pagination', () => {
mockGetMessages.mockReset();
mockGetMessagesAround.mockReset();
messageCache.clear();
mockToastError.mockReset();
});
it('fetchNewerMessages loads newer messages and appends them', async () => {
@@ -618,6 +677,69 @@ describe('useConversationMessages forward pagination', () => {
expect(result.current.messages[0].text).toBe('latest-msg');
});
it('aborts stale newer-page requests on conversation switch without toasting', async () => {
const convA: Conversation = { type: 'channel', id: 'ch1', name: 'Channel A' };
const convB: Conversation = { type: 'channel', id: 'ch2', name: 'Channel B' };
mockGetMessagesAround.mockResolvedValueOnce({
messages: [
createMessage({
id: 1,
type: 'CHAN',
conversation_key: 'ch1',
text: 'msg-0',
sender_timestamp: 1700000000,
received_at: 1700000000,
}),
],
has_older: false,
has_newer: true,
});
let newerSignal: AbortSignal | undefined;
mockGetMessages.mockImplementationOnce((_, signal?: AbortSignal) => {
newerSignal = signal;
return new Promise<Message[]>((_, reject) => {
signal?.addEventListener('abort', () => {
reject(new DOMException('The operation was aborted', 'AbortError'));
});
});
});
const initialProps: { conv: Conversation; target: number | null } = {
conv: convA,
target: 1,
};
const { result, rerender } = renderHook(
({ conv, target }: { conv: Conversation; target: number | null }) =>
useConversationMessages(conv, target),
{ initialProps }
);
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
act(() => {
void result.current.fetchNewerMessages();
});
await waitFor(() => expect(result.current.loadingNewer).toBe(true));
mockGetMessages.mockResolvedValueOnce([
createMessage({
id: 999,
type: 'CHAN',
conversation_key: 'ch2',
text: 'conv-b',
}),
]);
rerender({ conv: convB, target: null });
await waitFor(() => expect(result.current.messagesLoading).toBe(false));
expect(newerSignal?.aborted).toBe(true);
expect(mockToastError).not.toHaveBeenCalled();
});
it('preserves around-loaded messages when the jump target is cleared in the same conversation', async () => {
const conv: Conversation = { type: 'channel', id: 'ch1', name: 'Channel' };