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

Records created with a previously deleted ID generates an uncaught error #145

Closed
billdami opened this issue Oct 13, 2014 · 11 comments
Closed

Comments

@billdami
Copy link

In my application, I have what I think is a fairly standard User model, whose IDs are based on the authenticated users' fbRef.getAuth().uid value (which in my case is assigned by the Anonymous authentication provider). I have a global users which dynamically updates as users authenticate/unauthenticate, that is populated using a basic this.store.find('user') call.

The problem arises when a user signs out, and then re-authenticates/signs in with the same auth uid. I recieve the following error in the consoles of the other currently connected browsers:

Uncaught Error: Attempted to handle event pushedData on <myapp@model:user::ember938:anonymous:-JZ51lM7UkJUuD6pp9Yw> while in state root.deleted.uncommitted.

(note however, that the user/browser that did the signing out/re-logging in receives no error)

Is there anything else I need to do in this scenario to make sure that records automatically deleted by emberfire are fully committed in each connected client's cached store, or is this a limitation of some sort?

For reference, here is how I am creating the records when a user signs in:

var user = this.store.createRecord('user', {
    id: fbRef.getAuth().uid,
    nickname: nickname,
    joinedAt: new Date()
});

user.save();

And when they sign out:

this.store.find('user', fbRef.getAuth().uid).then(function(user) {
    user.destroyRecord();
});
@sararob
Copy link
Contributor

sararob commented Dec 15, 2014

Could you share the code you're using to authenticate users with Anonymous Auth?

@tstirrat
Copy link
Contributor

tstirrat commented Apr 3, 2015

I can confirm this, a record removed on the server will also remove locally, but the deleted record will still be cached locally in the store. This is because we use model.deleteRecord instead of model.destroyRecord here.

On the client that deletes the record, as long as you use destroyRecord it is usually ok:

post.get('id'); // -Jm0LzWtETCIw-LT9ZUP
post.destroyRecord().then(function () {
  store.recordForId('user', '-Jm0LzWtETCIw-LT9ZUP'); // undefined
});

But on a different client that observed the delete from the server, the deleted record is still there:

// -Jm0LzWtETCIw-LT9ZUP was deleted on server
store.recordForId('user', '-Jm0LzWtETCIw-LT9ZUP'); // returns the deleted (hidden) record

store.createRecord('user', { id: '-Jm0LzWtETCIw-LT9ZUP', ... });
// Assertion Failed: The id -Jm0LzWtETCIw-LT9ZUP has already been used with another record of type dummy@model:user:.

The workaround is to find the record and unload it manually before creating:

store.recordForId('user', '-Jm0LzWtETCIw-LT9ZUP').unloadRecord();
store.createRecord('user', { id: '-Jm0LzWtETCIw-LT9ZUP', ... }); // works

@jamiechong
Copy link

@tstirrat For the case where there is a different client observing only, how can I manually unload the record?

On some other client, a record with a previously used ID is pushed, the observing client receives the Firebase event and attempts to push the new record (with a re-used ID) into the store, but this is all happening within the emberfire codebase - how can I apply this workaround in my app?

And why not use destroyRecord? Is there any reason to use deleteRecord which allows for rollbacks? If we're trying to keep the store in sync with Firebase, committing the change right away with destroyRecord seems to make sense to me.

@tstirrat
Copy link
Contributor

@jamiechong Not sure I understand, why does your client have a record with duplicate id in its store if things happened via another client? can you elaborate?

@tstirrat
Copy link
Contributor

And yes, we should really be doing destroyRecord or unloadRecord

@jamiechong
Copy link

@tstirrat Sure. Say I'm saving unique records by explicitly setting the id. In a real-time situation I have one interface (UI-A) that can CRUD these records and another interface (UI-B) that merely displays the data.

  1. UI-A creates a record with id="my-unique-id", which adds it to its store.
  2. UI-B via emberfire gets notified by Firebase's on child_added event. This adds the my-unique-id record to the store of UI-B.
  3. UI-A deletes the record and can apply the workaround
  4. UI-B removes the record from its store via emberfire and on child_removed
  5. UI-A repeats step 1 using the same id="my-unique_id".
  6. UI-B tries to repeat step 2, but gets an error because the store of UI-B believes my-unique-id is still loaded.

@jamiechong
Copy link

Also note that I did try replacing deleteRecord with destroyRecord on Line 295 just to see if it would work, but it caused all sorts of other issues that I didn't investigate further.

@tstirrat
Copy link
Contributor

cool, thanks for clarifying that, yes that is a problem

@tstirrat
Copy link
Contributor

I wonder if unloadRecord is the most appropriate thing to use instead of destroyRecord

@leifcr
Copy link
Contributor

leifcr commented Sep 1, 2015

@tstirrat I think unloadRecord might be used, since destroyRecord will trigger another server request, which triggers an error response, as stated by @jamiechong.

destroyRecord is the same as calling deleteRecord, then save. More information here: http://guides.emberjs.com/v2.0.0/models/creating-and-deleting-records/#toc_deleting-records

However, unloadRecord only works for non-dirty records so if UI-B is able to change the record before it's informed about the destroy event, it will fail.

Perhaps delete, then unload or unload if delete fails?

@gabrielgrant
Copy link
Contributor

I'm inclined to say that using only unloadRecord is best: if a record has been edited locally before being deleted by another client, that is a legitimate data conflict that should raise an error to allow the client-side code to handle it gracefully (eg. alerting the user that the record has been deleted on the server, and asking if they want to re-create it using their local data, or just lose their changes)

The one potential issue, as raised in #322, is that afaik just calling unloadRecord will not fire the record's didDelete event. That being said, I'm not convinced the didDelete event should fire -- there's a clear difference between a record being deleted locally vs receiving an update from the server. But if the event is desired, then it could just be triggered manually.

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

7 participants