Skip to content

test: failing repro for preload hanging after cleanup (#1576)#1605

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

Closed
kevin-dp wants to merge 1 commit into from
Closed

test: failing repro for preload hanging after cleanup (#1576) #1605

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 27 additions & 0 deletions packages/db/tests/query/live-query-collection.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -594,6 +594,33 @@ describe(`createLiveQueryCollection`, () => {
finalSubscription.unsubscribe()
})

it(`loads its data again when preloaded after the live query and its source collection were cleaned up`, async () => {
const activeUsers = createLiveQueryCollection({
query: (q) =>
q
.from({ user: usersCollection })
.where(({ user }) => eq(user.active, true)),
})

await activeUsers.preload()
expect(activeUsers.status).toBe(`ready`)
expect(activeUsers.size).toBe(2)

// Tear down the source collection and the live query, e.g. when switching
// to a different data set at runtime. Cleaning up a source collection puts
// the dependent live query into an error state.
await usersCollection.cleanup()
expect(activeUsers.status).toBe(`error`)

await activeUsers.cleanup()
expect(activeUsers.status).toBe(`cleaned-up`)

// Preloading again restarts sync and resolves once the data is loaded.
await activeUsers.preload()
expect(activeUsers.status).toBe(`ready`)
expect(activeUsers.size).toBe(2)
Comment on lines +619 to +621

@coderabbitaicoderabbitaiBotJun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add timeout protection to prevent indefinite test hang.

According to the PR objectives, this regression test captures a bug where preload() hangs indefinitely after cleanup due to the isInErrorState flag never being reset. Without timeout protection, if the bug exists, this await activeUsers.preload() call will block CI indefinitely.

The PR objectives mention including a 2-second timeout guard to fail fast. Consider adding a timeout mechanism here.

⏱️ Proposed timeout protection
 // Preloading again restarts sync and resolves once the data is loaded.
- await activeUsers.preload()+ await Promise.race([+ activeUsers.preload(),+ new Promise((_, reject) =>+ setTimeout(() => reject(new Error('preload() timed out after cleanup')), 2000)+ ),+ ])
expect(activeUsers.status).toBe(`ready`)
expect(activeUsers.size).toBe(2)
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
awaitactiveUsers.preload()
expect(activeUsers.status).toBe(`ready`)
expect(activeUsers.size).toBe(2)
awaitPromise.race([
activeUsers.preload(),
newPromise((_,reject)=>
setTimeout(()=>reject(newError('preload() timed out after cleanup')),2000)
),
])
expect(activeUsers.status).toBe(`ready`)
expect(activeUsers.size).toBe(2)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/tests/query/live-query-collection.test.ts` around lines 616 -
618, The await activeUsers.preload() call lacks timeout protection and could
hang indefinitely if the isInErrorState flag bug exists. Wrap the preload() call
with a 2-second timeout mechanism (such as using Promise.race with a timeout
promise or Jest's timeout utilities) to ensure the test fails fast rather than
blocking CI indefinitely if the preload operation does not complete within the
specified time frame.

})

it(`should handle temporal values correctly in live queries`, async () => {
// Define a type with temporal values
type Task = {
Expand Down
Loading