-
Notifications
You must be signed in to change notification settings - Fork 79
fix handling of electric must-refetch #460
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.
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
Conversation
🦋 Changeset detectedLatest commit: cb8ca29 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
More templates
@tanstack/db
@tanstack/db-ivm
@tanstack/electric-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Size Change: 0 B Total Size: 64.4 kB ℹ️ View Unchanged
|
Size Change: 0 B Total Size: 1.16 kB ℹ️ View Unchanged
|
} else { | ||
console.error( | ||
`An error occurred while syncing collection: ${collection.id}, \n` + | ||
`it has been marked as ready to avoid blocking apps waiting for '.preload()' to finish. \n` + | ||
`You can provide an 'onError' handler on the shapeOptions to handle this error, and this message will not be logged.`, | ||
errorParams | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KyleAMathews I have added this logging here so that a user is alerted to the fact the collection has been marked ready on an error. You added the original code to do this and I wanted to check your thinking.
// Commit the truncate transaction immediately | ||
commit() | ||
transactionStarted = false | ||
// Reset hasUpToDate so we continue accumulating changes until next up-to-date |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are no longer committing it here, when will it be committed and where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the next up-to-date from electric, line 558 just below.
I tried to add a test to validate that a 409 didn't reach the onError handler, but it would require very complex mocking, and time is limited...