diff --git a/.changeset/fix-infinite-loop-orderby-limit.md b/.changeset/fix-infinite-loop-orderby-limit.md new file mode 100644 index 000000000..5d9f07afe --- /dev/null +++ b/.changeset/fix-infinite-loop-orderby-limit.md @@ -0,0 +1,23 @@ +--- +'@tanstack/db': patch +'@tanstack/db-ivm': patch +--- + +Add safety limits to prevent app freezes from excessive iterations in ORDER BY + LIMIT queries. + +**The problem**: ORDER BY + LIMIT queries can cause excessive iterations when WHERE filters out most data - the TopK keeps asking for more data that doesn't exist. + +**The fix**: Added iteration safety limits that gracefully break out of loops and continue with available data: + +- D2 graph: 100,000 iterations +- maybeRunGraph: 10,000 iterations +- requestLimitedSnapshot: 10,000 iterations + +When limits are hit, a warning is logged with: + +- **Iteration breakdown**: Shows where the loop spent time (e.g., "iterations 1-5: [TopK, Filter], 6-10000: [TopK]") +- Diagnostic info: collection IDs, query structure, cursor position, etc. + +The query **continues normally** with the data it has - no error state, no app breakage. + +The iteration breakdown makes it easy to see the stuck pattern in the state machine. Please report any warnings to https://github.com/TanStack/db/issues diff --git a/packages/db-ivm/src/d2.ts b/packages/db-ivm/src/d2.ts index 8451b2aff..506e39128 100644 --- a/packages/db-ivm/src/d2.ts +++ b/packages/db-ivm/src/d2.ts @@ -1,4 +1,5 @@ import { DifferenceStreamWriter } from './graph.js' +import { createIterationLimitChecker } from './iteration-tracker.js' import type { BinaryOperator, DifferenceStreamReader, @@ -57,7 +58,37 @@ export class D2 implements ID2 { } run(): void { + // Safety limit to prevent infinite loops in case of circular data flow + // or other bugs that cause operators to perpetually produce output. + const checkLimit = createIterationLimitChecker({ + maxSameState: 10000, + maxTotal: 100000, + }) + while (this.pendingWork()) { + // Use count of operators with pending work as state key + const operatorsWithWorkCount = this.#operators.filter((op) => + op.hasPendingWork(), + ).length + + if ( + checkLimit(() => { + // Only compute diagnostics when limit is exceeded (lazy) + const operatorsWithWork = this.#operators + .filter((op) => op.hasPendingWork()) + .map((op) => ({ id: op.id, type: op.constructor.name })) + return { + context: `D2 graph execution`, + diagnostics: { + operatorsWithPendingWork: operatorsWithWork, + totalOperators: this.#operators.length, + }, + } + }, operatorsWithWorkCount) + ) { + break + } + this.step() } } diff --git a/packages/db-ivm/src/index.ts b/packages/db-ivm/src/index.ts index cae148b46..e4a21408c 100644 --- a/packages/db-ivm/src/index.ts +++ b/packages/db-ivm/src/index.ts @@ -1,4 +1,5 @@ export * from './d2.js' +export * from './iteration-tracker.js' export * from './multiset.js' export * from './operators/index.js' export * from './types.js' diff --git a/packages/db-ivm/src/iteration-tracker.ts b/packages/db-ivm/src/iteration-tracker.ts new file mode 100644 index 000000000..9d7cdbcce --- /dev/null +++ b/packages/db-ivm/src/iteration-tracker.ts @@ -0,0 +1,95 @@ +/** + * Creates an iteration counter with limit checks based on state changes. + * + * Tracks both total iterations AND iterations without state change. This catches: + * - True infinite loops (same state repeating) + * - Slow progress that exceeds total limit + * + * @example + * ```ts + * const checkLimit = createIterationLimitChecker({ + * maxSameState: 10000, // Max iterations without state change + * maxTotal: 100000, // Hard cap regardless of state changes + * }) + * + * while (pendingWork()) { + * const stateKey = operators.filter(op => op.hasPendingWork()).length + * if (checkLimit(() => ({ + * context: 'D2 graph execution', + * diagnostics: { totalOperators: operators.length } + * }), stateKey)) { + * break + * } + * step() + * } + * ``` + */ + +export type LimitExceededInfo = { + context: string + diagnostics?: Record +} + +export type IterationLimitOptions = { + /** Max iterations without state change before triggering (default: 10000) */ + maxSameState?: number + /** Hard cap on total iterations regardless of state changes (default: 100000) */ + maxTotal?: number +} + +/** + * Creates an iteration limit checker that logs a warning when limits are exceeded. + * + * @param options - Configuration for iteration limits + * @returns A function that checks limits and returns true if exceeded + */ +export function createIterationLimitChecker( + options: IterationLimitOptions = {}, +): (getInfo: () => LimitExceededInfo, stateKey?: string | number) => boolean { + const maxSameState = options.maxSameState ?? 10000 + const maxTotal = options.maxTotal ?? 100000 + + let totalIterations = 0 + let sameStateIterations = 0 + let lastStateKey: string | number | undefined + + return function checkLimit( + getInfo: () => LimitExceededInfo, + stateKey?: string | number, + ): boolean { + totalIterations++ + + // Track same-state iterations + if (stateKey !== undefined && stateKey !== lastStateKey) { + // State changed - reset same-state counter + sameStateIterations = 0 + lastStateKey = stateKey + } + sameStateIterations++ + + const sameStateExceeded = sameStateIterations > maxSameState + const totalExceeded = totalIterations > maxTotal + + if (sameStateExceeded || totalExceeded) { + const { context, diagnostics } = getInfo() + + const reason = sameStateExceeded + ? `${sameStateIterations} iterations without state change (limit: ${maxSameState})` + : `${totalIterations} total iterations (limit: ${maxTotal})` + + const diagnosticSection = diagnostics + ? `\nDiagnostic info: ${JSON.stringify(diagnostics, null, 2)}\n` + : `\n` + + console.warn( + `[TanStack DB] ${context} exceeded iteration limit: ${reason}. ` + + `Continuing with available data.` + + diagnosticSection + + `Please report this issue at https://github.com/TanStack/db/issues`, + ) + return true + } + + return false + } +} diff --git a/packages/db-ivm/tests/iteration-tracker.test.ts b/packages/db-ivm/tests/iteration-tracker.test.ts new file mode 100644 index 000000000..aac852836 --- /dev/null +++ b/packages/db-ivm/tests/iteration-tracker.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it, vi } from 'vitest' +import { createIterationLimitChecker } from '../src/iteration-tracker.js' + +describe(`createIterationLimitChecker`, () => { + it(`should not exceed limit on normal iteration counts`, () => { + const checkLimit = createIterationLimitChecker({ maxSameState: 100 }) + + for (let i = 0; i < 50; i++) { + expect(checkLimit(() => ({ context: `test` }))).toBe(false) + } + }) + + it(`should return true when same-state limit is exceeded`, () => { + const checkLimit = createIterationLimitChecker({ maxSameState: 10 }) + + for (let i = 0; i < 10; i++) { + expect(checkLimit(() => ({ context: `test` }))).toBe(false) + } + + // 11th iteration exceeds the limit + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + expect(checkLimit(() => ({ context: `test` }))).toBe(true) + consoleSpy.mockRestore() + }) + + it(`should reset same-state counter when state key changes`, () => { + const checkLimit = createIterationLimitChecker({ + maxSameState: 5, + maxTotal: 100, + }) + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + // 5 iterations with state key 1 - should not exceed + for (let i = 0; i < 5; i++) { + expect(checkLimit(() => ({ context: `test` }), 1)).toBe(false) + } + + // Change state key to 2 - counter resets + // 5 more iterations should not exceed + for (let i = 0; i < 5; i++) { + expect(checkLimit(() => ({ context: `test` }), 2)).toBe(false) + } + + // Change state key to 3 - counter resets again + // 5 more iterations should not exceed + for (let i = 0; i < 5; i++) { + expect(checkLimit(() => ({ context: `test` }), 3)).toBe(false) + } + + // Total is 15, but no same-state limit exceeded + expect(consoleSpy).not.toHaveBeenCalled() + consoleSpy.mockRestore() + }) + + it(`should trigger on total limit even with state changes`, () => { + const checkLimit = createIterationLimitChecker({ + maxSameState: 10, + maxTotal: 20, + }) + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + // Alternate state keys to avoid same-state limit + for (let i = 0; i < 20; i++) { + expect(checkLimit(() => ({ context: `test` }), i % 2)).toBe(false) + } + + // 21st iteration exceeds total limit + expect(checkLimit(() => ({ context: `test` }), 0)).toBe(true) + expect(consoleSpy).toHaveBeenCalledTimes(1) + expect(consoleSpy.mock.calls[0]![0]).toContain(`total iterations`) + consoleSpy.mockRestore() + }) + + it(`should only call getInfo when limit is exceeded (lazy evaluation)`, () => { + const checkLimit = createIterationLimitChecker({ maxSameState: 5 }) + const getInfo = vi.fn(() => ({ context: `test` })) + + // First 5 iterations should not call getInfo + for (let i = 0; i < 5; i++) { + checkLimit(getInfo) + } + expect(getInfo).not.toHaveBeenCalled() + + // 6th iteration exceeds limit and should call getInfo + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + checkLimit(getInfo) + expect(getInfo).toHaveBeenCalledTimes(1) + consoleSpy.mockRestore() + }) + + it(`should log warning with context and diagnostics`, () => { + const checkLimit = createIterationLimitChecker({ maxSameState: 2 }) + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + checkLimit(() => ({ context: `test` })) + checkLimit(() => ({ context: `test` })) + checkLimit(() => ({ + context: `D2 graph execution`, + diagnostics: { + totalOperators: 8, + operatorsWithWork: [`TopK`, `Filter`], + }, + })) + + expect(consoleSpy).toHaveBeenCalledTimes(1) + const warning = consoleSpy.mock.calls[0]![0] + expect(warning).toContain(`[TanStack DB] D2 graph execution`) + expect(warning).toContain(`iterations without state change`) + expect(warning).toContain(`Continuing with available data`) + expect(warning).toContain(`"totalOperators": 8`) + expect(warning).toContain(`TopK`) + expect(warning).toContain(`https://github.com/TanStack/db/issues`) + + consoleSpy.mockRestore() + }) + + it(`should log warning without diagnostics when not provided`, () => { + const checkLimit = createIterationLimitChecker({ maxSameState: 1 }) + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + checkLimit(() => ({ context: `test` })) + checkLimit(() => ({ context: `Graph execution` })) + + expect(consoleSpy).toHaveBeenCalledTimes(1) + const warning = consoleSpy.mock.calls[0]![0] + expect(warning).toContain(`[TanStack DB] Graph execution`) + expect(warning).not.toContain(`Diagnostic info:`) + + consoleSpy.mockRestore() + }) + + it(`should use default limits when not specified`, () => { + const checkLimit = createIterationLimitChecker({}) + const consoleSpy = vi.spyOn(console, `warn`).mockImplementation(() => {}) + + // Default maxSameState is 10000 - should not trigger + for (let i = 0; i < 1000; i++) { + expect(checkLimit(() => ({ context: `test` }))).toBe(false) + } + + expect(consoleSpy).not.toHaveBeenCalled() + consoleSpy.mockRestore() + }) +}) diff --git a/packages/db/src/collection/subscription.ts b/packages/db/src/collection/subscription.ts index 44c9af49f..35322c971 100644 --- a/packages/db/src/collection/subscription.ts +++ b/packages/db/src/collection/subscription.ts @@ -1,3 +1,4 @@ +import { createIterationLimitChecker } from '@tanstack/db-ivm' import { ensureIndexForExpression } from '../indexes/auto-index.js' import { and, eq, gte, lt } from '../query/builder/functions.js' import { PropRef, Value } from '../query/ir.js' @@ -399,24 +400,29 @@ export class CollectionSubscription } /** - * Sends a snapshot that fulfills the `where` clause and all rows are bigger or equal to the cursor. - * Requires a range index to be set with `setOrderByIndex` prior to calling this method. - * It uses that range index to load the items in the order of the index. + * Loads rows from the local collection in sorted order using the BTree index. + * + * Uses the BTree index (set via `setOrderByIndex`) to iterate through the local + * collection's data in sorted order, starting from the cursor position (`minValues`). + * Also triggers an async `loadSubset` call to fetch more data from the sync layer + * (e.g., Electric backend) if needed. * * For multi-column orderBy: - * - Uses first value from `minValues` for LOCAL index operations (wide bounds, ensures no missed rows) - * - Uses all `minValues` to build a precise composite cursor for SYNC layer loadSubset + * - Uses first value from `minValues` for BTree index operations (wide bounds, ensures no missed rows) + * - Uses all `minValues` to build a precise composite cursor for sync layer loadSubset + * + * Note 1: May load more rows than `limit` because it includes all rows equal to the + * cursor value. This prevents skipping duplicates when limit falls mid-duplicates. + * Note 2: Skips keys that have already been sent to prevent duplicates. * - * Note 1: it may load more rows than the provided LIMIT because it loads all values equal to the first cursor value + limit values greater. - * This is needed to ensure that it does not accidentally skip duplicate values when the limit falls in the middle of some duplicated values. - * Note 2: it does not send keys that have already been sent before. + * @returns true if local data was found, false if no more matching rows exist locally */ requestLimitedSnapshot({ orderBy, limit, minValues, offset, - }: RequestLimitedSnapshotOptions) { + }: RequestLimitedSnapshotOptions): boolean { if (!limit) throw new Error(`limit is required`) if (!this.orderByIndex) { @@ -502,8 +508,43 @@ export class CollectionSubscription ? compileExpression(new PropRef(orderByExpression.path), true) : null + // Safety limit to prevent infinite loops if the index iteration or filtering + // logic has issues. The loop should naturally terminate when the index is + // exhausted, but this provides a backstop. + const checkLimit = createIterationLimitChecker({ + maxSameState: 1000, + maxTotal: 10000, + }) + let hitIterationLimit = false + while (valuesNeeded() > 0 && !collectionExhausted()) { - const insertedKeys = new Set() // Track keys we add to `changes` in this iteration + // Use changes.length as state key - if we're making progress, this should increase + const stateKey = changes.length + + if ( + checkLimit( + () => ({ + context: `requestLimitedSnapshot`, + diagnostics: { + collectionId: this.collection.id, + collectionSize: this.collection.size, + limit, + offset, + valuesNeeded: valuesNeeded(), + keysInBatch: keys.length, + changesCollected: changes.length, + sentKeysCount: this.sentKeys.size, + cursorValue: biggestObservedValue, + minValueForIndex, + orderByDirection: orderBy[0]!.compareOptions.direction, + }, + }), + stateKey, + ) + ) { + hitIterationLimit = true + break + } for (const key of keys) { const value = this.collection.get(key)! @@ -515,7 +556,6 @@ export class CollectionSubscription // Extract the indexed value (e.g., salary) from the row, not the full row // This is needed for index.take() to work correctly with the BTree comparator biggestObservedValue = valueExtractor ? valueExtractor(value) : value - insertedKeys.add(key) // Track this key } keys = index.take(valuesNeeded(), biggestObservedValue, filterFn) @@ -556,20 +596,20 @@ export class CollectionSubscription if (whereFromCursor) { const { expression } = orderBy[0]! - const minValue = minValues[0] + const cursorMinValue = minValues[0] // Build the whereCurrent expression for the first orderBy column // For Date values, we need to handle precision differences between JS (ms) and backends (μs) // A JS Date represents a 1ms range, so we query for all values within that range let whereCurrentCursor: BasicExpression - if (minValue instanceof Date) { - const minValuePlus1ms = new Date(minValue.getTime() + 1) + if (cursorMinValue instanceof Date) { + const minValuePlus1ms = new Date(cursorMinValue.getTime() + 1) whereCurrentCursor = and( - gte(expression, new Value(minValue)), + gte(expression, new Value(cursorMinValue)), lt(expression, new Value(minValuePlus1ms)), ) } else { - whereCurrentCursor = eq(expression, new Value(minValue)) + whereCurrentCursor = eq(expression, new Value(cursorMinValue)) } cursorExpressions = { @@ -597,6 +637,10 @@ export class CollectionSubscription // Track this loadSubset call this.loadedSubsets.push(loadOptions) this.trackLoadSubsetPromise(syncResult) + + // Return true if we found and sent local data, false otherwise. + // The async loadSubset call may still fetch data from the backend. + return changes.length > 0 && !hitIterationLimit } // TODO: also add similar test but that checks that it can also load it from the collection's loadSubset function diff --git a/packages/db/src/query/live/collection-config-builder.ts b/packages/db/src/query/live/collection-config-builder.ts index 21cd04d1d..840e962e2 100644 --- a/packages/db/src/query/live/collection-config-builder.ts +++ b/packages/db/src/query/live/collection-config-builder.ts @@ -1,4 +1,4 @@ -import { D2, output } from '@tanstack/db-ivm' +import { D2, createIterationLimitChecker, output } from '@tanstack/db-ivm' import { compileQuery } from '../compiler/index.js' import { buildQuery, getQueryIR } from '../builder/index.js' import { @@ -336,7 +336,43 @@ export class CollectionConfigBuilder< // Always run the graph if subscribed (eager execution) if (syncState.subscribedToAllCollections) { + // Safety limit to prevent infinite loops when data loading and graph processing + // create a feedback cycle. + const checkLimit = createIterationLimitChecker({ + maxSameState: 1000, + maxTotal: 10000, + }) + while (syncState.graph.pendingWork()) { + // Use messagesCount as state key - if we're processing messages, we're making progress + const stateKey = syncState.messagesCount + + if ( + checkLimit(() => { + // Only compute diagnostics when limit is exceeded (lazy) + const collectionIds = Object.keys(this.collections) + const orderByInfo = Object.entries( + this.optimizableOrderByCollections, + ).map(([id, info]) => ({ + collectionId: id, + limit: info.limit, + offset: info.offset, + dataNeeded: info.dataNeeded?.() ?? `unknown`, + })) + return { + context: `Graph execution`, + diagnostics: { + liveQueryId: this.id, + sourceCollections: collectionIds, + runCount: this.runCount, + orderByConfig: orderByInfo, + }, + } + }, stateKey) + ) { + break + } + syncState.graph.run() // Flush accumulated changes after each graph step to commit them as one transaction. // This ensures intermediate join states (like null on one side) don't cause diff --git a/packages/db/src/query/live/collection-subscriber.ts b/packages/db/src/query/live/collection-subscriber.ts index ec4876b74..30a4f3efa 100644 --- a/packages/db/src/query/live/collection-subscriber.ts +++ b/packages/db/src/query/live/collection-subscriber.ts @@ -146,9 +146,8 @@ export class CollectionSubscriber< // Filter changes to prevent duplicate inserts to D2 pipeline. // This ensures D2 multiplicity stays at 1 for visible items, so deletes // properly reduce multiplicity to 0 (triggering DELETE output). - const changesArray = Array.isArray(changes) ? changes : [...changes] const filteredChanges: Array> = [] - for (const change of changesArray) { + for (const change of changes) { if (change.type === `insert`) { if (this.sentToD2Keys.has(change.key)) { // Skip duplicate insert - already sent to D2 diff --git a/packages/db/tests/infinite-loop-prevention.test.ts b/packages/db/tests/infinite-loop-prevention.test.ts new file mode 100644 index 000000000..f87da5c09 --- /dev/null +++ b/packages/db/tests/infinite-loop-prevention.test.ts @@ -0,0 +1,185 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' +import { createLiveQueryCollection, gt } from '../src/query/index.js' +import { mockSyncCollectionOptions } from './utils.js' + +/** + * Tests for infinite loop prevention in ORDER BY + LIMIT queries. + * + * The issue: When a live query has ORDER BY + LIMIT, the TopK operator + * requests data until it has `limit` items. If the WHERE clause filters + * out most data, the TopK may never be filled, causing excessive iterations. + * + * The band-aid fix: Safety limits that cap iterations and throw detailed + * error messages with diagnostic info when exceeded: + * - D2 graph: 100,000 iterations + * - maybeRunGraph: 10,000 iterations + * - requestLimitedSnapshot: 10,000 iterations + * + * These tests verify that queries complete without hitting safety limits. + */ + +type TestItem = { + id: number + value: number + category: string +} + +describe(`Infinite loop prevention`, () => { + it(`should complete ORDER BY + LIMIT query when WHERE filters out most data`, async () => { + // Scenario: Query wants 10 items with value > 90, but only 2 exist + // Should complete without hitting safety limits + const initialData: Array = [] + for (let i = 1; i <= 20; i++) { + initialData.push({ + id: i, + value: i * 5, // values: 5, 10, 15, ... 95, 100 + category: i <= 10 ? `A` : `B`, + }) + } + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `infinite-loop-test`, + getKey: (item: TestItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 90)) + .orderBy(({ items }) => items.value, `desc`) + .limit(10) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + // Should complete without hanging or hitting safety limits + await liveQueryCollection.preload() + + // Verify results + const results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(2) + expect(results.map((r) => r.value)).toEqual([100, 95]) + + // Verify not in error state (didn't hit safety limit) + expect( + liveQueryCollection.status, + `Query should not be in error state`, + ).not.toBe(`error`) + }) + + it(`should load new data when it arrives after initial load`, async () => { + // Start with data that doesn't match WHERE clause + const initialData: Array = [ + { id: 1, value: 10, category: `A` }, + { id: 2, value: 20, category: `A` }, + { id: 3, value: 30, category: `A` }, + ] + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `resume-loading-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + // Query wants items with value > 50, ordered by value, limit 5 + // Initially 0 items match + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 50)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Initially 0 results + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(0) + + // Add new data that matches WHERE clause + utils.begin() + utils.write({ type: `insert`, value: { id: 4, value: 60, category: `A` } }) + utils.write({ type: `insert`, value: { id: 5, value: 70, category: `A` } }) + utils.commit() + + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Should now have 2 items + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(2) + expect(results.map((r) => r.value)).toEqual([70, 60]) + + // Verify not in error state + expect(liveQueryCollection.status).not.toBe(`error`) + }) + + it(`should handle updates without entering error state`, async () => { + const initialData: Array = [ + { id: 1, value: 100, category: `A` }, // Only this matches WHERE > 95 + { id: 2, value: 50, category: `A` }, + { id: 3, value: 40, category: `A` }, + ] + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `updates-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .where(({ items }) => gt(items.value, 95)) + .orderBy(({ items }) => items.value, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + await liveQueryCollection.preload() + + // Should have 1 item initially + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + + // Send several updates + for (let i = 0; i < 10; i++) { + utils.begin() + utils.write({ + type: `update`, + value: { id: 1, value: 100 + i, category: `A` }, + }) + utils.commit() + await new Promise((resolve) => setTimeout(resolve, 10)) + } + + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Should still have results and not be in error state + results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(1) + expect(results[0]!.value).toBeGreaterThanOrEqual(100) + expect(liveQueryCollection.status).not.toBe(`error`) + }) +}) diff --git a/packages/db/tests/nan-comparator-infinite-loop.test.ts b/packages/db/tests/nan-comparator-infinite-loop.test.ts new file mode 100644 index 000000000..08da832f5 --- /dev/null +++ b/packages/db/tests/nan-comparator-infinite-loop.test.ts @@ -0,0 +1,384 @@ +import { describe, expect, it } from 'vitest' +import { createCollection } from '../src/collection/index.js' +import { createLiveQueryCollection } from '../src/query/index.js' +import { eq } from '../src/query/builder/functions.js' +import { mockSyncCollectionOptions } from './utils.js' + +/** + * Test that reproduces the infinite loop bug caused by NaN values in comparisons. + * + * The bug occurs when: + * 1. Data contains invalid Date objects (where getTime() returns NaN) + * 2. An ORDER BY + LIMIT query runs on the date field + * 3. The comparator returns NaN instead of -1, 0, or 1 + * 4. Binary search in TopK can't find a stable position + * 5. The comparison function is called infinitely + * + * This matches the production bug where: + * - Debugger paused in `gte` comparison code + * - App completely froze (not just slow) + * - Clearing Electric cache fixed it (fresh data had valid dates) + */ + +type TestItem = { + id: number + name: string + createdAt: Date +} + +describe(`NaN comparator infinite loop`, () => { + it(`should handle invalid dates in ORDER BY without infinite loop`, async () => { + // Create data with a mix of valid and INVALID dates + // Invalid dates have getTime() = NaN, which breaks comparisons + const initialData: Array = [ + { id: 1, name: `Valid 1`, createdAt: new Date(`2024-01-01`) }, + { id: 2, name: `Valid 2`, createdAt: new Date(`2024-01-02`) }, + { id: 3, name: `Invalid`, createdAt: new Date(`not a valid date`) }, // NaN! + { id: 4, name: `Valid 3`, createdAt: new Date(`2024-01-03`) }, + { id: 5, name: `Valid 4`, createdAt: new Date(`2024-01-04`) }, + ] + + // Verify we actually have an invalid date + expect(Number.isNaN(initialData[2]!.createdAt.getTime())).toBe(true) + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `nan-date-test`, + getKey: (item: TestItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + // This query should NOT hang - it should either: + // 1. Handle the NaN gracefully (ideal) + // 2. Throw an error (acceptable) + // 3. NOT infinite loop (critical) + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .orderBy(({ items }) => items.createdAt, `desc`) + .limit(3) + .select(({ items }) => ({ + id: items.id, + name: items.name, + createdAt: items.createdAt, + })), + ) + + // Set up a timeout to detect infinite loop + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject( + new Error( + `Test timed out - infinite loop detected in ORDER BY with NaN date`, + ), + ) + }, 5000) + }) + + // Race the preload against the timeout + const result = await Promise.race([ + liveQueryCollection.preload().then(() => `completed`), + timeoutPromise, + ]) + + expect(result).toBe(`completed`) + + // If we get here, the query completed without hanging + const results = Array.from(liveQueryCollection.values()) + // We should have some results (exact behavior depends on how NaN is handled) + expect(results.length).toBeGreaterThanOrEqual(0) + }) + + it(`should handle NaN in numeric ORDER BY without infinite loop`, async () => { + // Test with explicit NaN numeric values (not just invalid dates) + type NumericItem = { + id: number + value: number + } + + const initialData: Array = [ + { id: 1, value: 10 }, + { id: 2, value: 20 }, + { id: 3, value: NaN }, // Explicit NaN + { id: 4, value: 30 }, + { id: 5, value: 40 }, + ] + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `nan-numeric-test`, + getKey: (item: NumericItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .orderBy(({ items }) => items.value, `desc`) + .limit(3) + .select(({ items }) => ({ + id: items.id, + value: items.value, + })), + ) + + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject( + new Error( + `Test timed out - infinite loop detected in ORDER BY with NaN value`, + ), + ) + }, 5000) + }) + + const result = await Promise.race([ + liveQueryCollection.preload().then(() => `completed`), + timeoutPromise, + ]) + + expect(result).toBe(`completed`) + }) + + it(`should handle mixed valid/invalid dates during updates without infinite loop`, async () => { + // This simulates the Electric scenario where updates arrive with potentially invalid data + const initialData: Array = [ + { id: 1, name: `Item 1`, createdAt: new Date(`2024-01-01`) }, + { id: 2, name: `Item 2`, createdAt: new Date(`2024-01-02`) }, + { id: 3, name: `Item 3`, createdAt: new Date(`2024-01-03`) }, + ] + + const { utils, ...options } = mockSyncCollectionOptions({ + id: `nan-update-test`, + getKey: (item: TestItem) => item.id, + initialData, + }) + + const sourceCollection = createCollection(options) + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .orderBy(({ items }) => items.createdAt, `desc`) + .limit(5) + .select(({ items }) => ({ + id: items.id, + name: items.name, + })), + ) + + await liveQueryCollection.preload() + + // Initial results should work + let results = Array.from(liveQueryCollection.values()) + expect(results).toHaveLength(3) + + // Now send an UPDATE that introduces an invalid date + // This simulates Electric sending corrupted data + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject( + new Error( + `Test timed out - infinite loop detected after update with NaN date`, + ), + ) + }, 5000) + }) + + const updatePromise = (async () => { + utils.begin() + utils.write({ + type: `update`, + value: { + id: 2, + name: `Updated Item 2`, + createdAt: new Date(`invalid date string`), + }, + }) + utils.commit() + + // Wait for processing + await new Promise((resolve) => setTimeout(resolve, 100)) + + return `completed` + })() + + const result = await Promise.race([updatePromise, timeoutPromise]) + expect(result).toBe(`completed`) + + // Query should still return results (behavior with NaN may vary) + results = Array.from(liveQueryCollection.values()) + expect(results.length).toBeGreaterThanOrEqual(0) + }) + + it(`should handle JOIN + ORDER BY + LIMIT with rapid updates (Electric scenario)`, async () => { + // This simulates the production scenario: + // - Two collections JOINed together + // - ORDER BY + LIMIT on a date field + // - Rapid updates arriving during processing (like Electric initial sync) + + type User = { id: number; name: string } + type Post = { + id: number + userId: number + title: string + createdAt: Date + } + + const usersData: Array = [ + { id: 1, name: `Alice` }, + { id: 2, name: `Bob` }, + { id: 3, name: `Charlie` }, + ] + + const postsData: Array = [] + for (let i = 1; i <= 20; i++) { + postsData.push({ + id: i, + userId: (i % 3) + 1, + title: `Post ${i}`, + createdAt: new Date(`2024-01-${String(i).padStart(2, `0`)}`), + }) + } + + const usersCollection = createCollection( + mockSyncCollectionOptions({ + id: `join-users`, + getKey: (item: User) => item.id, + initialData: usersData, + }), + ) + + const { utils: postsUtils, ...postsOptions } = mockSyncCollectionOptions({ + id: `join-posts`, + getKey: (item: Post) => item.id, + initialData: postsData, + }) + + const postsCollection = createCollection(postsOptions) + + await usersCollection.preload() + await postsCollection.preload() + + // JOIN + ORDER BY createdAt DESC + LIMIT 5 + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ posts: postsCollection }) + .join({ users: usersCollection }, ({ posts, users }) => + eq(posts.userId, users.id), + ) + .orderBy(({ posts }) => posts.createdAt, `desc`) + .limit(5) + .select(({ posts, users }) => ({ + postId: posts.id, + title: posts.title, + author: users!.name, + createdAt: posts.createdAt, + })), + ) + + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject( + new Error( + `Test timed out - infinite loop in JOIN + ORDER BY + LIMIT`, + ), + ) + }, 5000) + }) + + // Start preload and send rapid updates simultaneously (like Electric sync) + const testPromise = (async () => { + const preloadPromise = liveQueryCollection.preload() + + // Simulate rapid Electric updates during initial sync + for (let i = 21; i <= 40; i++) { + postsUtils.begin() + postsUtils.write({ + type: `insert`, + value: { + id: i, + userId: (i % 3) + 1, + title: `Post ${i}`, + createdAt: new Date(`2024-02-${String(i - 20).padStart(2, `0`)}`), + }, + }) + postsUtils.commit() + // Small delay between updates + await new Promise((resolve) => setTimeout(resolve, 1)) + } + + await preloadPromise + return `completed` + })() + + const result = await Promise.race([testPromise, timeoutPromise]) + expect(result).toBe(`completed`) + + const results = Array.from(liveQueryCollection.values()) + expect(results.length).toBeLessThanOrEqual(5) + }) + + it(`should handle empty string dates (common data issue) without infinite loop`, async () => { + // Empty strings are a common data issue - they create invalid dates + // Simulate data that might come from a database with empty date fields + const rawData = [ + { id: 1, name: `Has date`, createdAt: `2024-01-01` }, + { id: 2, name: `Empty string`, createdAt: `` }, // Common issue! + { id: 3, name: `Has date`, createdAt: `2024-01-03` }, + ] + + // Convert to Date objects (as user code might do) + const initialData: Array = rawData.map((item) => ({ + id: item.id, + name: item.name, + createdAt: new Date(item.createdAt), // Empty string -> Invalid Date + })) + + // Verify empty string creates invalid date + expect(Number.isNaN(initialData[1]!.createdAt.getTime())).toBe(true) + + const sourceCollection = createCollection( + mockSyncCollectionOptions({ + id: `empty-string-date-test`, + getKey: (item: TestItem) => item.id, + initialData, + }), + ) + + await sourceCollection.preload() + + const liveQueryCollection = createLiveQueryCollection((q) => + q + .from({ items: sourceCollection }) + .orderBy(({ items }) => items.createdAt, `desc`) + .limit(3) + .select(({ items }) => ({ + id: items.id, + name: items.name, + })), + ) + + const timeoutPromise = new Promise((_, reject) => { + setTimeout(() => { + reject( + new Error(`Test timed out - infinite loop with empty string date`), + ) + }, 5000) + }) + + const result = await Promise.race([ + liveQueryCollection.preload().then(() => `completed`), + timeoutPromise, + ]) + + expect(result).toBe(`completed`) + }) +})