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

ItemsMap doesn't match real object #1187

Closed
androidfans opened this issue Jun 22, 2020 · 29 comments · Fixed by #1279
Closed

ItemsMap doesn't match real object #1187

androidfans opened this issue Jun 22, 2020 · 29 comments · Fixed by #1279

Comments

@androidfans
Copy link

2020-06-22 12-55-12屏幕截图

board's items map doesn't match my real object in DB. In this case, when I call boards.list(). It will return a single true that caused some bug in my app.

I don't have a certain repro step. but it happens when sync be triggered.

Can remotestoragejs just add a single cleanup when this kind of case detected

@raucao
Copy link
Member

raucao commented Jun 22, 2020

There's no list() function on BaseClient, so it would be useful if you could link or show the actual code there. Also if you can, add a little more context about the situation in which this happens. Otherwise it's pretty difficult to understand what's going on. Thanks.

By the way, there's an open issue for adding metadata to the local listings! Otherwise you can only get metadata when caching is turned off. See the orange warning box in the function documentation of getListing().

@raucao
Copy link
Member

raucao commented Jun 22, 2020

Oh, I think I understand the confusion. A listing is never supposed to give you the actual objects. That's what you would do with getAll() for example. Was that the question perhaps?

@androidfans
Copy link
Author

sorry, i didn't remember the correct api name. when i say list(). I actually mean
privateClient.getAll('', false);

@androidfans
Copy link
Author

androidfans commented Jun 22, 2020

as the pic shows above. boad's itemsmap has 3 key. but db only has 1 object

@raucao
Copy link
Member

raucao commented Jun 22, 2020

I'm sorry, but the screenshot doesn't show the output of getAll. It is not possible to infer what your code is doing, and what the situation is. You will need to provide some code and context, if you want someone to understand your problem.

Also, if it's not possible to reproduce for yourself, then it's going to be even more difficult for other people to reproduce. Did this happen more than once at least?

@androidfans
Copy link
Author

androidfans commented Jun 22, 2020

I will save a snapshot when it happens next time.

@HerrZatacke
Copy link

HerrZatacke commented Jul 7, 2020

Hey guys,

I guess i have kind of the same problem:
As shown in my screenshot, I have 5 items (out of 25) that are marked false.

As I just started using rs.js, I don't exactly know what I can do to investigate, but I'd be happy to help in any way possible.

calling this results in 20 items being returned

// rs is the instance of remotestorage attached to the window
rs.scope('/gb-printer-web/')
  .getAll('images/')
  .then((images) => {
    console.log(Object.values(images).length)
  });

I do not know what causes the "false" elements. But when using the "inspektor" app, I can see all 25 items, as well as when I check the 'common' prop besides the 'local' one.

A few observations that might help:

  • All these items were created in quick succession, together with some additional Objects with each 12.3KB (so alltogether ~50 requests to the remote server with a combined size of ~300kb)
  • Of the 25 larger Items, the exact same number (=5) were not stored remoteley, despite the request having been sent.
  • A page reload does not solve the problem
  • running rs.local.flush() solved it last time (I wont do it now, in case there is a way providing my current data could help you)
  • Logging out (and back in) solved it last time as well (I wont do that either now...)
  • Logging in in a different browser works (as it pulls the correct data from the remote server)

2020-07-07_17h23_04

@HerrZatacke
Copy link

HerrZatacke commented Jul 7, 2020

So... while fiddlling a little more something strage happened... 🤔

First I tried adding another image (item), which worked (so from now on I have 21 working and 26 total items)

Then I edited one of the working items in a different Browser, and after the "faulty" browser synced, all items were there and a call to .getAll('images/') resulted in 26 Items (as I would have expected in the beginning).

After just waiting a few seconds (probably until the next sync) on the "faulty" browser, the items disappeared again and a call to .getAll('images/') resulted in 21 Items once again.

Then I edited one of the faulty items in a different Browser, and after the "faulty" browser synced, all items were back again and a call to .getAll('images/') resulted in 26 Items

After just waiting another few seconds on the "faulty" browser, the items except for the previously faulty one disappeared again and a call to .getAll('images/') resulted in 22 Items now.

My installed versions btw are (from package.json):

    "remotestorage-widget": "^1.5.2",
    "remotestoragejs": "^1.2.3",

@raucao
Copy link
Member

raucao commented Jul 8, 2020

Thanks! This detailed description should be very helpful for debugging the issue. Looks to me like there's some kind of race condition bug in the outgoing sync.

I'm currently hoping to get reviews on the TypeScript port of the library, so we can finally start properly refactoring the Sync class (and the changes in that branch may already fix some subtle bugs like this): #1175

@HerrZatacke
Copy link

HerrZatacke commented Jul 8, 2020

I built a small app to reproduce the bug:
https://github.com/HerrZatacke/rs-test
Also directly published to github pages:
https://herrzatacke.github.io/rs-test/

It seems the bug happens when creating "images" (that's what my app is handling) and it's syncing data to remote.
Maybe it happens while the automatic sync-interval is running it's own update

@raucao
Copy link
Member

raucao commented Jul 8, 2020

That would explain why it's been undetected so far. Not many apps both cache and sync binary files. Another good hint for debugging!

@androidfans
Copy link
Author

I repro this problem again.
My operations before repro this bug is :

  1. clear all cache and localstorage
  2. create an object
  3. delete object
  4. login an account and start syncing

@raucao
Copy link
Member

raucao commented Jul 9, 2020

This is becoming weirdly common (see latest linked issue, too). I was not aware of this bug happening on any meaningful scale until this issue was opened. Interestingly, all of you also took their screenshots in Chrome. So I'm starting to wonder if something broke with IndexedDB (or how we use it) in Chrome, or if this is reproducible in other browsers and with other caching back-ends.

Just from all of what I've read so far, my wild guess would be something in the direction of a function not waiting for a fulfilled promise somewhere. This could also be related to the fairly recent addition of using fetch() instead of oldskool AJAX where it's available.

@HerrZatacke
Copy link

Just a sidenote, as in #1188 actual binary files were mentioned: My App does not store actual binary data (even if it's called rawImages). My Raw-Data is just an Array of 360 Strings. (don't ask why 😅)

@rosano
Copy link
Contributor

rosano commented Jul 9, 2020

Interestingly, all of you also took their screenshots in Chrome.

Mine was in Safari.

@rosano
Copy link
Contributor

rosano commented Aug 26, 2020

I seem to have reproduced this three times today with my flashcards app like this:

  • open app in private window in Chrome
  • import 1500 objects (~2000 IndexedDB entries) in anonymous mode
  • connect storage and sync

After the sync I was able to access the new objects in Safari but not my iPhone standalone PWA. In that private window, pre-existing data was synchronized but failed to appear because the highest-level folders were 'false':

Screen Shot 2020-08-26 at 10 33 58

@raucao
Copy link
Member

raucao commented Sep 2, 2020

Does the same happen in a non-private window? That would be immensely helpful to know. Private modes are notorious for breaking all kinds of local storage in surprising new ways.

@rosano
Copy link
Contributor

rosano commented Sep 2, 2020

Just tried it in non-private Chrome window and the same thing happens: there were two parent objects in anonymous mode, and after connecting there are four (according to local.itemsMap but the two that came downstream are false. Not sure if this is relevant but according to the console there are 2008 entries in the non-private window and 2007 in the other [private] window I used to check that what I imported was able to be downloaded.

@rosano
Copy link
Contributor

rosano commented Sep 2, 2020

I don't know how the syncing works but I imagined a hypothesis that the more objects there are to sync, the more likely it is that some child will arrive before the parent thus making the parent false and then for some reason it fails to check the status after all children have been downloaded.

@rosano
Copy link
Contributor

rosano commented Oct 27, 2020

Just wanted to add this info in case it helps anyone with debugging.

I am sometimes able to re-fetch all data by calling the undocumented client.flush, but currently it's failing because of an error "undefined is not an object (evaluating 'a.itemsMap')", which I think is due to accessing these 'false' items in this line. Seems like the only way out is to sign out and in again. I have also tried recursively getting all objects with maxAge set to zero, but it doesn't bring down the latest data.

@rosano
Copy link
Contributor

rosano commented Apr 1, 2021

I seem to have reached a state where a client receives sync events and upon reloading the app, the new documents are not accessible. Perhaps it's happening more often now because there are many more documents. Are there are things that I can test now that this is less random?

@raucao
Copy link
Member

raucao commented Apr 1, 2021

What does "not accessible" mean? Do you see the respective IndexedDB nodes before reloading?

@rosano
Copy link
Contributor

rosano commented Apr 8, 2021

I managed to capture it on video: https://www.dropbox.com/s/3rj4z2vgdq5t29b/2021.04.08%201%20disappearing.mp4?dl=0

Items created on one client sync to another but disappear on reload. I have observed this in at least three of my apps, and I suspect it might occur when there is a large quantity of documents—if you start fresh with a low document count it might not occur as much. It might also occur because I use Safari on macOS and iOS, and as we know Safari has problems.

It seems like the IndexedDB entries are the same on both clients:

Chrome

Screen Shot 2021-04-08 at 18 13 36

Safari

Screen Shot 2021-04-08 at 18 15 31

@rosano
Copy link
Contributor

rosano commented Apr 8, 2021

Okay so another observation is that if I create a new document on the second client and reload, it begins to display the documents that were previously not accessible. This might have to do with my folder structure: each day has a folder, notes created on a given day that sync to another client will disappear if that client has no other notes on that day, but if you create a new note on the receiving client they will appear again. https://www.dropbox.com/s/phojiag6w3ax020/2021.04.08%201%20reappearing.mp4?dl=0

@rosano
Copy link
Contributor

rosano commented Oct 17, 2021

Noel De Martin encountered this recently with Chrome on Android rosano/hyperdraft#4

@raucao I'm wondering how you haven't seen this in the wild yet. Are you using 5apps or a different provider? or using mostly anonymous mode?

@raucao
Copy link
Member

raucao commented Oct 18, 2021

@raucao I'm wondering how you haven't seen this in the wild yet. Are you using 5apps or a different provider? or using mostly anonymous mode?

I guess the same way that nobody opened an issue before June 2020? It's difficult to explain why when there's no way for me to reproduce it. I'm using 5apps, usually connected to one of my various accounts, both personal ones and organizational.

@raucao
Copy link
Member

raucao commented Oct 18, 2021

I have created a new tracking issue for the sync refactoring, as well as a project to collect sync-related issues and missing features:

darkdread pushed a commit to darkdread/remotestorage.js that referenced this issue Dec 5, 2022
Multiple reads after tasks have been fetched causes folder index to be
misaligned. This fix forces fetched tasks to be queued, ensuring that
task handlers finish committing their changes before running the next
task handler.
darkdread pushed a commit to darkdread/remotestorage.js that referenced this issue Dec 12, 2022
Updated inmemorystorage.ts getNodes to follow
behavior of localStorage and indexeddb.
@raucao
Copy link
Member

raucao commented Jan 5, 2023

The fix for this has just been released with version 2.0.0-beta.6.

@rosano
Copy link
Contributor

rosano commented Apr 22, 2023

My impression from upgrading all my apps in the last months is that:

  • clients with incomplete data due to this issue pre-upgrade may not 'fix' themselves; might be helpful if there's a way to detect this inconsistent state and help notify the end-user / app, or manually trigger a 'refresh' of the data.
  • it definitely seems to me like the issue happens far less often, but it might still not be zero; I keep losing track of which apps I logged out and connected storage to 'start on the latest version of this library' but I think even doing this might not 100% prevent the issue from happening… Something that might trigger this could be reloading the page while syncing, especially if it's a larger sync.

Not sure if there's any value to sharing this considering this is very tricky to reproduce, but just thought to share my observations. I would be curious to know if there was a way to somehow guarantee that this could not happen, like what the library test would be. Will try to experiment with syncing larger amounts of data (like with my flashcards app) and report back if I can replicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants