Skip to content

Add defensive checks for corrupted queue items in persistent session #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcodejongh merged 5 commits into from
Feb 1, 2026
Merged

Add defensive checks for corrupted queue items in persistent session #619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
672a4cf
fix: Handle null/undefined queue items to prevent TypeError in comput…
claude Feb 1, 2026
e937b4c
chore: Update package-lock.json
claude Feb 1, 2026
21105a8
fix: Add cooldown to corruption-triggered resyncs to prevent infinite…
claude Feb 1, 2026
3a23035
fix: Improve corruption handling - prevent infinite loops and improve…
claude Feb 1, 2026
a393381
fix: Improve type safety, add tests, and document corruption detection
claude Feb 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion docs/websocket-implementation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,60 @@ sequenceDiagram
- Sequence numbers must increment by 1
- Hash updated after each delta event

### 6. Subscription Error / Complete
### 6. Queue Item Corruption Detection

The client detects and recovers from corrupted queue items (null/undefined entries) that may occur due to server bugs, network issues, or state corruption.

```mermaid
sequenceDiagram
participant C as Client
participant PS as PersistentSession
participant S as Server

Note over C: useEffect runs on queue change

C->>C: Check for null/undefined items

alt No corrupted items
C->>C: Compute and update state hash
else Corrupted items detected
C->>C: Check resync cooldown (30s)

alt Within cooldown
C->>C: Filter items locally
C->>C: Log error (Sentry)
Note over C: Prevents infinite loops
else Cooldown expired
C->>C: Log error (Sentry)
C->>S: Trigger resync
S->>C: FullSync event
C->>C: Apply clean state
end
end
```

**Corruption sources:**
- Server sends malformed queue data
- State corruption during delta sync
- Race conditions in event handling

**Detection points:**
1. **FullSync handler**: Filters null items when receiving initial/full state
2. **QueueItemAdded handler**: Skips events with null items
3. **State hash effect**: Detects corruption in current queue state

**Resync cooldown:**
- 30 second cooldown between corruption-triggered resyncs
- Prevents infinite loop if server keeps returning corrupted data
- During cooldown: filter corrupted items locally instead of resyncing
- All corruption events logged at `console.error` level for Sentry visibility

**Implementation:**
- `computeQueueStateHash()` defensively filters null/undefined items
- `isFilteringCorruptedItemsRef` prevents useEffect re-trigger loops
- `lastCorruptionResyncRef` tracks cooldown timing

### 7. Subscription Error / Complete

```mermaid
sequenceDiagram
Expand Down
15 changes: 6 additions & 9 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 49 additions & 1 deletion packages/web/app/components/persistent-session/persistent-session-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ const DEFAULT_BACKEND_URL = process.env.NEXT_PUBLIC_WS_URL || null;
// Board names to check if we're on a board route - use centralized constant
const BOARD_NAMES = SUPPORTED_BOARDS;

// Cooldown for corruption-triggered resyncs to prevent infinite loops
const CORRUPTION_RESYNC_COOLDOWN_MS = 30000; // 30 seconds

// Session type matching the GraphQL response
export interface Session {
id: string;
Expand Down Expand Up @@ -245,6 +248,10 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }>
const mountedRef = useRef(false);
// Ref to store reconnect handler for use by hash verification
const triggerResyncRef = useRef<(() => void) | null>(null);
// Cooldown tracking for corruption-triggered resyncs to prevent infinite loops
const lastCorruptionResyncRef = useRef<number>(0);
// Track if we're currently filtering corrupted items to prevent useEffect re-trigger loop
const isFilteringCorruptedItemsRef = useRef(false);

// Event subscribers
const queueEventSubscribersRef = useRef<Set<(event: SubscriptionQueueEvent) => void>>(new Set());
Expand Down Expand Up @@ -294,13 +301,20 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }>

switch (event.__typename) {
case 'FullSync':
setQueueState(event.state.queue as LocalClimbQueueItem[]);
// Filter out any undefined/null items that may have been introduced by state corruption
setQueueState((event.state.queue as LocalClimbQueueItem[]).filter(item => item != null));
setCurrentClimbQueueItem(event.state.currentClimbQueueItem as LocalClimbQueueItem | null);
// Reset sequence tracking on full sync
updateLastReceivedSequence(event.sequence);
setLastReceivedStateHash(event.state.stateHash);
break;
case 'QueueItemAdded':
// Skip if item is undefined/null to prevent state corruption
if (event.addedItem == null) {
console.error('[PersistentSession] Received QueueItemAdded with null/undefined item, skipping');
updateLastReceivedSequence(event.sequence);
break;
}
setQueueState((prev) => {
const newQueue = [...prev];
if (event.position !== undefined && event.position >= 0) {
Expand Down Expand Up @@ -350,8 +364,42 @@ export const PersistentSessionProvider: React.FC<{ children: React.ReactNode }>

// Keep state hash in sync with local state after delta events
// This ensures hash verification compares against current state, not stale FullSync hash
// Also detects corrupted items and triggers resync if found
useEffect(() => {
if (!session) return; // Only update hash when connected

// Skip if we're currently filtering corrupted items to prevent re-trigger loop
if (isFilteringCorruptedItemsRef.current) {
isFilteringCorruptedItemsRef.current = false;
return;
}

// Check for corrupted (null/undefined) items in the queue
const hasCorruptedItems = queue.some(item => item == null);
if (hasCorruptedItems) {
const now = Date.now();
const timeSinceLastResync = now - lastCorruptionResyncRef.current;

if (timeSinceLastResync < CORRUPTION_RESYNC_COOLDOWN_MS) {
// Still in cooldown - filter corrupted items locally instead of resyncing
console.error(
`[PersistentSession] Detected null/undefined items in queue, but resync on cooldown ` +
`(${Math.round((CORRUPTION_RESYNC_COOLDOWN_MS - timeSinceLastResync) / 1000)}s remaining). ` +
`Filtering locally.`
);
isFilteringCorruptedItemsRef.current = true;
setQueueState(prev => prev.filter(item => item != null));
return;
}

console.error('[PersistentSession] Detected null/undefined items in queue, triggering resync');
lastCorruptionResyncRef.current = now;
if (triggerResyncRef.current) {
triggerResyncRef.current();
}
return; // Skip hash update - resync will provide correct state
}

const newHash = computeQueueStateHash(queue, currentClimbQueueItem?.uuid || null);
setLastReceivedStateHash(newHash);
}, [session, queue, currentClimbQueueItem]);
Expand Down
173 changes: 173 additions & 0 deletions packages/web/app/utils/__tests__/hash.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import { describe, it, expect } from 'vitest';
import { fnv1aHash, computeQueueStateHash } from '../hash';

describe('hash utilities', () => {
describe('fnv1aHash', () => {
it('should return consistent hash for same input', () => {
const input = 'test-string';
const hash1 = fnv1aHash(input);
const hash2 = fnv1aHash(input);

expect(hash1).toBe(hash2);
});

it('should return different hashes for different inputs', () => {
const hash1 = fnv1aHash('input-1');
const hash2 = fnv1aHash('input-2');

expect(hash1).not.toBe(hash2);
});

it('should return 8-character hex string', () => {
const hash = fnv1aHash('test');

expect(hash).toMatch(/^[0-9a-f]{8}$/);
});

it('should handle empty string', () => {
const hash = fnv1aHash('');

expect(hash).toMatch(/^[0-9a-f]{8}$/);
});
});

describe('computeQueueStateHash', () => {
it('should return consistent hash for same queue state', () => {
const queue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];
const currentItemUuid = 'item-1';

const hash1 = computeQueueStateHash(queue, currentItemUuid);
const hash2 = computeQueueStateHash(queue, currentItemUuid);

expect(hash1).toBe(hash2);
});

it('should return different hash when queue changes', () => {
const queue1 = [{ uuid: 'item-1' }, { uuid: 'item-2' }];
const queue2 = [{ uuid: 'item-1' }, { uuid: 'item-3' }];
const currentItemUuid = 'item-1';

const hash1 = computeQueueStateHash(queue1, currentItemUuid);
const hash2 = computeQueueStateHash(queue2, currentItemUuid);

expect(hash1).not.toBe(hash2);
});

it('should return different hash when currentItemUuid changes', () => {
const queue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];

const hash1 = computeQueueStateHash(queue, 'item-1');
const hash2 = computeQueueStateHash(queue, 'item-2');

expect(hash1).not.toBe(hash2);
});

it('should return same hash regardless of queue order (sorted internally)', () => {
const queue1 = [{ uuid: 'item-1' }, { uuid: 'item-2' }];
const queue2 = [{ uuid: 'item-2' }, { uuid: 'item-1' }];
const currentItemUuid = 'item-1';

const hash1 = computeQueueStateHash(queue1, currentItemUuid);
const hash2 = computeQueueStateHash(queue2, currentItemUuid);

expect(hash1).toBe(hash2);
});

it('should handle null currentItemUuid', () => {
const queue = [{ uuid: 'item-1' }];

const hash = computeQueueStateHash(queue, null);

expect(hash).toMatch(/^[0-9a-f]{8}$/);
});

it('should handle empty queue', () => {
const hash = computeQueueStateHash([], 'item-1');

expect(hash).toMatch(/^[0-9a-f]{8}$/);
});

it('should handle empty queue with null currentItemUuid', () => {
const hash = computeQueueStateHash([], null);

expect(hash).toMatch(/^[0-9a-f]{8}$/);
});

// Corruption handling tests
describe('corruption handling', () => {
it('should filter out null items from queue', () => {
const queue = [{ uuid: 'item-1' }, null, { uuid: 'item-2' }];
const cleanQueue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];

const hashWithNull = computeQueueStateHash(queue, 'item-1');
const hashClean = computeQueueStateHash(cleanQueue, 'item-1');

expect(hashWithNull).toBe(hashClean);
});

it('should filter out undefined items from queue', () => {
const queue = [{ uuid: 'item-1' }, undefined, { uuid: 'item-2' }];
const cleanQueue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];

const hashWithUndefined = computeQueueStateHash(queue, 'item-1');
const hashClean = computeQueueStateHash(cleanQueue, 'item-1');

expect(hashWithUndefined).toBe(hashClean);
});

it('should filter out items with null uuid', () => {
const queue = [
{ uuid: 'item-1' },
{ uuid: null as unknown as string },
{ uuid: 'item-2' },
];
const cleanQueue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];

const hashWithNullUuid = computeQueueStateHash(queue, 'item-1');
const hashClean = computeQueueStateHash(cleanQueue, 'item-1');

expect(hashWithNullUuid).toBe(hashClean);
});

it('should filter out items with undefined uuid', () => {
const queue = [
{ uuid: 'item-1' },
{ uuid: undefined as unknown as string },
{ uuid: 'item-2' },
];
const cleanQueue = [{ uuid: 'item-1' }, { uuid: 'item-2' }];

const hashWithUndefinedUuid = computeQueueStateHash(queue, 'item-1');
const hashClean = computeQueueStateHash(cleanQueue, 'item-1');

expect(hashWithUndefinedUuid).toBe(hashClean);
});

it('should handle queue with all corrupted items', () => {
const queue = [null, undefined, { uuid: null as unknown as string }];

const hash = computeQueueStateHash(queue, 'item-1');

// Should be equivalent to empty queue
const emptyHash = computeQueueStateHash([], 'item-1');
expect(hash).toBe(emptyHash);
});

it('should not crash with mixed valid and corrupted items', () => {
const queue = [
null,
{ uuid: 'item-1' },
undefined,
{ uuid: 'item-2' },
{ uuid: null as unknown as string },
{ uuid: 'item-3' },
];

expect(() => computeQueueStateHash(queue, 'item-1')).not.toThrow();

const hash = computeQueueStateHash(queue, 'item-1');
expect(hash).toMatch(/^[0-9a-f]{8}$/);
});
});
});
});
Loading
Loading