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

destroyRecord regression in 2.13.x #4995

Closed
yads opened this issue May 24, 2017 · 10 comments · Fixed by #5408
Closed

destroyRecord regression in 2.13.x #4995

yads opened this issue May 24, 2017 · 10 comments · Fixed by #5408

Comments

@yads
Copy link

yads commented May 24, 2017

I started experiencing a regression after upgrading to 2.13.x from 2.12.2. The error happens when calling destroyRecord, then later querying records. Here is a test case route that reproduces the issue for me:

export default Ember.Route.extend({
  model() {
    this.store.findRecord('my-model', '123')
      .then(m => m.destroyRecord())
      .then(() => this.store.query('my-model', {}));
  }
});

The gotcha in our system is that the DELETE handler does not immediately delete the record, but marks the record for deletion. So it will still be returned in query calls until it's finished being deleted.

The error that I am getting in 2.13.1 is

Attempted to handle event `pushedData` on <my-model:123> while in state root.deleted.saved. 

Error
    at InternalModel._unhandledEvent (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:140691:13)
    at InternalModel.send (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:140560:14)
    at InternalModel.pushedData (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:140502:12)
    at InternalModel.setupData (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:140465:12)
    at Class._load (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:148474:21)
    at Class._pushInternalModel (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:148836:32)
    at http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:148779:39
    at Backburner.run (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:17614:23)
    at Backburner.join (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:17640:23)
    at Class._push (http://localhost:4200/ui/assets/vendor-541f5b0fb0f07999ce5d23075731eb0c.js:148763:52)
@yads yads changed the title destroyRecord regression in 0.13.x destroyRecord regression in 2.13.x Jun 14, 2017
@pangratz
Copy link
Member

pangratz commented Oct 7, 2017

This is still happening on latest master: https://ember-twiddle.com/ee240d40b116a8e7ab0e4120c24486a0?fileTreeShown=false&openFiles=routes.application.js%2C

@yads
Copy link
Author

yads commented Oct 9, 2017

I wound up having to change our UI to send a manual DELETE request, rather than use the destroyRecord api.

@thedig
Copy link

thedig commented Oct 23, 2017

Any progress here on a fix? This issue has gotten to be a big problem while trying to upgrade from 2.13.x to 2.15.x (currently we're locked on ED 2.12.x while on Ember 2.13.x, though having such a large disparity onward - 2.12 ED and 2.15 Ember - feels misguided, so this is a blocker for us on upgrading Ember).

@runspired
Copy link
Contributor

@yads your solution is weird, destroyRecord sends a delete request, and then on success it unloads the record. I'm unsure how that differs from what you've done, but maybe that also points to where we need to look to fix.

@yads
Copy link
Author

yads commented Oct 23, 2017

As far as I can tell, committing a record as being deleted also marks it in the store as having state 'root.deleted.saved'. When the store sees it again via a reload query, it seems to then have an issue with this. Manually sending a DELETE request, does not mark it in the store as having been deleted.

@runspired
Copy link
Contributor

Oh, so you mean not via deleteRecord. Question, why are you reloading deleted records, is this the same as #5237 maybe?

@workmanw
Copy link

workmanw commented Oct 23, 2017

I'm following this because we also ran into a similar issue. I think is because when you unload a record it no longer ends up in root.deleted.saved, it's just root.loaded.saved now. See: #4918

Edit: Actually @yads never said if he unloads the record or just leaves it alone, so maybe I preemptively assumed that.

@yads
Copy link
Author

yads commented Oct 23, 2017

@runspired it looks like that issue you referenced is essentially the same. The reason we are reloading the record is because sending the DELETE request merely starts the deletion process, which can take some time depending on the entity being deleted. So we want to reload the data to make sure the user is seeing the new status of the entity.

@thec0keman
Copy link

I believe we are experiencing similar behavior:

  • destroyRecord is called, succeeds
  • the query then refreshes, which returns an instance of the same record
  • Attempted to handle event pushedData on <user:280883> while in state root.deleted.saved exception is thrown

In our case, adding an model.unloadRecord() before the query refreshes solves the problem.

@MatthewSBarnes
Copy link

MatthewSBarnes commented Jul 10, 2018

If @thec0keman's suggestion doesn't work, I did something along the lines of what is discussed in #4972.

this.store._removeFromIdMap(your-problem-model._internalModel);

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

Successfully merging a pull request may close this issue.

7 participants