Skip to content
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

Better debugging information for Diagnostic Errors #612

Closed
esbenp opened this issue Jan 23, 2020 · 10 comments
Closed

Better debugging information for Diagnostic Errors #612

esbenp opened this issue Jan 23, 2020 · 10 comments
Labels
wontfix This will not be worked on

Comments

@esbenp
Copy link
Contributor

esbenp commented Jan 23, 2020

We are dealing with a lot of Diagnostic error: Cannot update a record with pending updates errors these days. It's extremely hard to debug because it happens of course on somebody's client somewhere and is entirely dependant on their local state.

Would probably be easier to debug if table + ID was part of this error - maybe even properties. Something like Diagnostic error: Cannot update a record with pending updates (table: documents, ID: 1234, pending_updates: title, body). Otherwise it's practically impossible to figure out what caused the error

What do you think? I could whip out a quick PR

@radex
Copy link
Collaborator

radex commented Jan 23, 2020

@esbenp Go for it! I'm a fan of helpful diagnostics :)

@esbenp
Copy link
Contributor Author

esbenp commented Jan 23, 2020

Alright I will. I am not super familiar with everything but could you maybe elaborate on what is a common cause for this? Is it preparedUpdates that are never flushed, or is it flushes that fail? Understanding what it means will help with making a better log statement

@radex
Copy link
Collaborator

radex commented Jan 23, 2020

this sets the pending update flag:

this._hasPendingUpdate = true

and this resets it:

record._hasPendingUpdate = false // TODO: What if this fails?

both happen synchronously when you correctly use this:

const record = record.prepareUpdate(x)
await database.batch(record)

so to hit this bug, you either have to prepare and update and then NOT pass it to batch synchronously (that's a bug, and watermelon in dev mode sends warnings about this), or something crashed on this path. example:

await database.batch(
  record1.prepareUpdate(),
  record2.prepareUpdate(x => { throw new Error() } )
)

record1 will be stuck with a pending update, because something else crashed before it got to the batch call

@esbenp
Copy link
Contributor Author

esbenp commented Jan 23, 2020

This is super helpful information, thank you! I actually thought this was related to multiple clients editing/deleting the same thing and then crashing when syncing (this error appears when we sync in our app).

Searching for this error gave me nothing, but I am thinking this would be extremely helpful as a "Common errors" in docs - what do you say?

@radex
Copy link
Collaborator

radex commented Jan 23, 2020

@esbenp yeah! that would be very helpful! docs-master is where you can put in .md format documentation

@stale
Copy link

stale bot commented Jul 25, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Jul 25, 2020
@stale stale bot closed this as completed Aug 15, 2020
@bureyburey
Copy link

joining in on the issue.

using these versions:
@nozbe/watermelondb: 0.21.0
@nozbe/with-observables: 1.2.0

we've also been encountering this same problem with several of our users.
we do not use prepareUpdate or database.batch() in our app, so I assume it comes from within 🍉 itself.

has there been any changes in the library that may cause this since this ticket was closed?

what's even more troubling is that we were unable to reproduce it by ourselves...
would love to hear your input on this :)

@davevilela
Copy link

@bureyburey I'm also having this problem, and it doesn't seem to have anything related to using prepareUpdate, as I haven't found a single usage of this method in my entire codebase.

@rohankm
Copy link

rohankm commented Jan 1, 2023

im getting this error on delte
Diagnostic error: Cannot mark a record with pending changes as deleted

@sarthakpranesh
Copy link

I have a common database wrapper were I have applied a really simple fix for this. All I did is wrap my r.prepareUpdate in a try-catch block, if it throws an error I log the error as warning and return the same record back (without calling prepareUpdate) so the developers consuming the DB wrapper know that they had possibly supplied two records that are same for batch update or their was a already existing prepared update for the record that did not get batched.

This does the trick for me for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants