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

Grounddb throws error, when being used on Meteor.users and with appcache. #22

Open
akralj opened this issue Dec 18, 2013 · 20 comments
Open

Comments

@akralj
Copy link

akralj commented Dec 18, 2013

Thank you for this great package.
I tested it with a normal collection and it works like a charm. Only Meteor.users seems to be different when having custom fields in it.

I deployed it to: http://grounddbusertest.meteor.com/ + repo at: https://github.com/akralj/grounddbUserTest

Steps to reproduce (with chrome or safari):

  1. http://grounddbusertest.meteor.com/
  2. login as : 111@111 with password: 111111
  3. cut off your network
  4. press "reload app" button which changes the collection & refreshes browser
  5. reconnect network
  6. do Meteor.reconnect() or wait up to 10s
    Uncaught Error: It doesn't make sense to be adding something we know exists: 3cKZpRMZmKpRXatwa
    livedata.js?a2e72054ba38809f8cac1e9197d52610e3efef8e:4258

groundDB.db.users is being nuked and meteor thinks one is logged out even if Meteor.userId() still exists in localstorage

It seems like Groundb & the normal Meteor sync overlap. I tested it with a a few versions of meteor and grounddb.

Here the main code.

 if (Meteor.isClient) {
    GroundDB(Meteor.users);
    Deps.autorun(function() {
      return Meteor.subscribe("userData");
    });
    Template.hello.helpers({
      date: function() {
        return Meteor.user().date;
      },
      userId: function() {
        return Meteor.userId();
      }
    });
    Template.hello.events({
      "click button": function() {
        Meteor.users.update({
          _id: Meteor.userId()
        }, {
          $set: {
            date: new Date()
          }
        });
        return Meteor.setTimeout((function() {
          return window.location.reload();
        }), 333);
      }
    });
  }

  if (Meteor.isServer) {
    Meteor.publish("userData", function() {
      return Meteor.users.find({
        _id: this.userId
      }, {
        fields: {
          date: 1
        }
      });
    });
    Meteor.users.allow({
      update: function() {
        return true;
      }
    });
  }
@raix
Copy link

raix commented Dec 18, 2013

I have to check up on this, Meteor.users is a bit special since it resumes methods by itself - the error from live data is Meteor throwing errors for data already found - its an odd thing on the client grounddb overwrites the original meteor code to make offline work. Is the error thrown on client or server?

@akralj
Copy link
Author

akralj commented Dec 18, 2013

The error is thrown on the client. On further investigation it looks like it has nothing to do with Methods being resumed. I added
GroundDB.skipMethods { "/users/update": true}
to my code so that the update method is not being cached, but still the same error is being thrown. So it must come from the Meteor.users Collection being automatically synced back to the server once meteor is online again.

@akralj
Copy link
Author

akralj commented Jan 17, 2014

I came back to the issue and found that any update to any collection (Meteor.users or a normal one) made offline throws the above error:
Uncaught Error: It doesn't make sense to be adding something we know exists.
I could reproduce it in your playground app (see screenshot):
grounddb error

@raix
Copy link

raix commented Jan 17, 2014

Nice catch, I guess it's time for adding client-side conflict resolution, I'll have a look at it next week or so. I'll refactor groundDB at the same time.

@akralj
Copy link
Author

akralj commented Jan 18, 2014

Hi Morten,
great to hear that you are on it. I was thinking a lot how to implement it myself, but syncing of databases seems is to big of a task for me at the moment.
I have a few questions regarding how grounddb works internally.

  1. Is it correct, that the whole collection is written into localstorage every time an item of the collection changes?
  2. It seems like the current implementation of grounddb can't handle offline changes at all (meteor, does the changes by itself, but this does not work when page was reloaded). Is this true?
  3. Is it correct, that when a user comes from offline (either page refresh or coming from an backgrounded ios page) that the whole collection is transferred to the client again. Or do you do some diffing between the data which was loaded from localstorage and the data which comes from the wire after a reconnect?

I don't really understand how a sync between offline (e.g. localstorage) <-> minimongo <-> mongo should be working, without having some sort of special collection, which logs each change with a version number + timestamp and sends it to the relevant data stores. What are your plans on this one?
Please tell me if my questions are not clear enough.
Thanks andrej

@raix
Copy link

raix commented Jan 18, 2014

  1. Nope not every change, if you do multiple changes it will only save once. But the current version does save the whole collection into one localstorage item. It uses minimax to compress, but ideally it should one doc pr. Item. And work async.
  2. It's a bug, discussed in Grounddb throws error, when being used on Meteor.users and with appcache.  #22 it will be fixed as part of the refactoring
  3. Meteor does not support session resume, i think they will at some point - but i have been thinking about this as part of a custom groundDB publish. But again it's also kind of Nice to get all the data making sure that the local db is not corrupted - ideally this should not be nessesary and we could save bandwidth.

So groundDB is just caching the db and method calls allowing it to resume. The current solution dont have any interface for conflict resolution. It's the tricky part and vary between apps.

@akralj
Copy link
Author

akralj commented Apr 25, 2014

Hi Morten,
do you have any plans to tackle this?
I had another look today an saw, that grounddb works perfectly when no user is logged in. the moment one logs in everything works fine(adding, removing records), apart from updates with throw a similar error like before:
Uncaught Error: Server sent add for existing id: 7qsQfsz2boCQakN7q livedata.js
the strange thing is that the change gets applied on the server, but the record which changed in the local collection is wipped out temporarily and only shows up on refreshing the browser (resubscribing!).
any idea what this could be?
cheers andrej

@raix
Copy link

raix commented Apr 25, 2014

I'm not sure - but I'm going to dig into it soon - Got some deadlines at the moment, but have to use groundDB for a project with deadline within 30 days - so something is bound to happen...

@timbrandin
Copy link

I have the same issue on another collection, so it's not just related to the users collection.

@raix
Copy link

raix commented Sep 19, 2014

I've just pushed the new ground:db - I havent had time yet to test this issue further - It'll prop. be a couple of days before I get to it - so help is much welcome

@tagrudev
Copy link
Contributor

tagrudev commented Nov 5, 2014

Hello @raix , did you manage to dig into this ?

I have a project with GroundDB and appcache - Once I log in I do GroundDB(Meteor.users) (on the client side) which successfully inserts the information in the localStorage. But once I go offline and refresh the page -

Meteor.user() - returns undefined ( although the information stays in the storage )

@raix
Copy link

raix commented Nov 8, 2014

Its on the todo list - I have to add this to the QA test and figure out whats causing the issue :)

@tagrudev
Copy link
Contributor

Would very much appreciate that :) 👍

@tagrudev
Copy link
Contributor

Hmm I've managed to make it work nothing is different though - I just put the GroundDB('users') -> inside lib/collections/users.coffee and I publish the user collection. Now I can't reproduce the failing 🔢

@raix
Copy link

raix commented Nov 10, 2014

So at the moment grounddb is pretty dumb in relations to the user collection - it should wait committing changes when the user that committed those changes logs in... for now it waits 500ms https://github.com/GroundMeteor/db/blob/Meteor-0-9-1/groundDB.client.js#L733-L740

Not sure if this could be the issue - ground:db actually could intercept the meteor.apply 'login' and wait for it to return (if called on connection) - a better solution than a simple wait...

// I still havent added a qa test for grounding the users collection - its on the todo :)

@isAlmogK
Copy link

I'm using meteor users and not getting any errors well test some more.

@markshust
Copy link

appears to be an issue with fast-render and grounddb. related thread kadirahq/fast-render#80

EDIT: never mind my stupidity, this was due to a circular ref

@sasikanth513
Copy link

Same issue on different collections too any temporary fix for this?

@sasikanth513
Copy link

@raix any update on this?

we're frequently getting Server sent add for existing id: LC6G5kzN44STKeLgz:4745 warning and our app crashes

@mitiaptest
Copy link

+1

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

No branches or pull requests

8 participants