Skip to content

Conversation

@fcannizzaro
Copy link
Contributor

Hi! I debugged what was happening in #418 and finally I discovered the issue's cause.

The AbortController inside the sync config of electric-db-collection after being aborted (by the cleanup fn after the gcTime and 0 active subscribers) was never re-initialized again.

And cause the signal was already aborted, the electric client skips the sync request! (https://github.com/electric-sql/electric/blob/3baf67a11887fea0bd046b06e081a93f16c88173/packages/typescript-client/src/client.ts#L474)

I only moved the AbortController instantiation inside the sync() function instead of the parent scope.

@changeset-bot
Copy link

changeset-bot bot commented Sep 9, 2025

🦋 Changeset detected

Latest commit: 3c47b2a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tanstack/electric-db-collection Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 9, 2025

More templates

@tanstack/db

npm i https://pkg.pr.new/@tanstack/db@523

@tanstack/db-ivm

npm i https://pkg.pr.new/@tanstack/db-ivm@523

@tanstack/electric-db-collection

npm i https://pkg.pr.new/@tanstack/electric-db-collection@523

@tanstack/query-db-collection

npm i https://pkg.pr.new/@tanstack/query-db-collection@523

@tanstack/react-db

npm i https://pkg.pr.new/@tanstack/react-db@523

@tanstack/solid-db

npm i https://pkg.pr.new/@tanstack/solid-db@523

@tanstack/svelte-db

npm i https://pkg.pr.new/@tanstack/svelte-db@523

@tanstack/trailbase-db-collection

npm i https://pkg.pr.new/@tanstack/trailbase-db-collection@523

@tanstack/vue-db

npm i https://pkg.pr.new/@tanstack/vue-db@523

commit: 3c47b2a

Copy link
Collaborator

@samwillis samwillis left a comment

Choose a reason for hiding this comment

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

This is great, really well spotted.

I don't suppose you could try and create a test that reproduces the problem so we can validate this fix and prevent a regression in future?

Copy link
Collaborator

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

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

Looks great! Could you add a test which reproduces the bug? Call cleanup & then query again to make sure sync starts up again?

@fcannizzaro
Copy link
Contributor Author

fcannizzaro commented Sep 9, 2025

Tomorrow I'll make these changes and write a test for it

@KyleAMathews
Copy link
Collaborator

I'm pretty sure I've run into the same bug so thanks for figuring it out!

@KyleAMathews
Copy link
Collaborator

A customer is running into this in production so I'm going to add a test to get this out. Thanks again for debugging things and putting up the PR!!

@fcannizzaro
Copy link
Contributor Author

fcannizzaro commented Sep 10, 2025

Yeah if you can write the test since I tried multiple ways to write it but I don't know how to test this case.. cause the mock is done on a global AbortController and ShapeStream but now it changes for every sync() so how it should be done ?

@fcannizzaro fcannizzaro force-pushed the fix-electric-preload-issue-after-gc branch from b104472 to 94c3b77 Compare September 10, 2025 20:55
@KyleAMathews
Copy link
Collaborator

Yeah, I couldn't get it to work either after trying for a bit — our mocks need modified quite a bit to make a test work. Let's merge and release.

@KyleAMathews KyleAMathews merged commit baaed71 into TanStack:main Sep 10, 2025
6 checks passed
@fcannizzaro fcannizzaro deleted the fix-electric-preload-issue-after-gc branch September 10, 2025 21:39
@github-actions github-actions bot mentioned this pull request Sep 10, 2025
@KyleAMathews
Copy link
Collaborator

I deployed this to my app and confirmed it fixed the bug

Uziniii pushed a commit to Uziniii/db that referenced this pull request Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants