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

TypeError: Cannot read property 'eachAttribute' of null (store.unloadAll bug?) #4963

Closed
shamcode opened this issue May 2, 2017 · 45 comments
Closed

Comments

@shamcode
Copy link

shamcode commented May 2, 2017

Hi! I had error for this code:

this.store.findRecord( 'animal', 1 )
  .then( () => this.store.unloadAll( 'animal' ) )
  .then( () => this.store.findRecord( 'animal', 1 ) ) ;
}

Error:

vendor.js:30401 TypeError: Cannot read property 'eachAttribute' of null
    at new Snapshot (vendor.js:73205)
    at InternalModel.createSnapshot (vendor.js:67158)
    at _find (vendor.js:76062)
    at Class._fetchRecord (vendor.js:74068)
    at _fetchRecord (vendor.js:74129)
    at Class._flushPendingFetchForType (vendor.js:74230)
    at cb (vendor.js:31972)
    at OrderedSet.forEach (vendor.js:31777)
    at MapWithDefault.forEach (vendor.js:31980)
    at Class.flushAllPendingFetches (vendor.js:74110)

ember-data LOC

Repo for reproduce (ember-twiddle down?)

I can be wrong, but it seems that this._isDematerializing in InternalModel should be equal to false after call store.unloadAll method.

@nag5000
Copy link

nag5000 commented May 2, 2017

I'm having the same problem.
Workaround: unloadRecord -> schedule('destroy') => _isDematerializing: false -> findRecord

@shamcode
Copy link
Author

shamcode commented May 2, 2017

@NagiTs thank you, but is workaround :)

@bichotll
Copy link

bichotll commented May 2, 2017

Out of curiosity. What version are you using? Is it the newest 2.13?

@nag5000
Copy link

nag5000 commented May 2, 2017

Yes, ember-cli/ember/ember-data 2.13.
2.12 hasn't got this bug.

unloadRecord was reworked in 2.13 (commit: 2f18405, location:

).

A similar bug was fixed in #2870 (#3296 (comment)) 2 years ago.

@omh
Copy link

omh commented May 2, 2017

Same here, upgraded to 2.13 from 2.10 and encountered this bug :/

@omh
Copy link

omh commented May 2, 2017

@NagiTs: not sure I quite follow your steps for the workaround. Would you be able to elaborate a bit more please?

@nag5000
Copy link

nag5000 commented May 2, 2017

@omh

this.store.unloadRecord(rec);
Ember.run.schedule('destroy', this, function() {
  this.store.findRecord('my-model', 1);
});

@jherdman
Copy link

jherdman commented May 2, 2017

My application was suffering from this too. I was manually unloading some records that appear to have been inflight to work around #3084. Once I removed the manual unloading, everything went back to normal.

@simonihmig
Copy link
Contributor

Confirm. This has popped up since upgrading from 2.12 to 2.13. The workaround #4963 (comment) did work for me.

Wonder if unloadRecord/unloadAll should return a Promise if it is apparently async. So you could write

this.store.unloadRecord(rec).then(() => {
  return this.store.findRecord('my-model', 1);
});

@crhayes
Copy link

crhayes commented May 4, 2017

I'm having this same issue, although I am not explicitly attempting to fetch a new record afterwards.

@david-duncan
Copy link

We had same issue. Downgrading to 2.12.0 is an alternative solution until this is fixed

@raido
Copy link

raido commented May 10, 2017

One more use case, failing test - raido@b1f026e

@raido
Copy link

raido commented May 10, 2017

https://github.com/raido/ember-data-unloadAll-regression - this is basically the same as the unit test case I posted before, except one can easily test with different ED versions against the test case.

@snovity
Copy link

snovity commented May 11, 2017

Got the same issue with 2.13.0, but it seems in my case a 404 response from server when loading a record was causing it, downgrading to 2.12.2 solved the issue.

@shamcode
Copy link
Author

shamcode commented May 11, 2017

We extend store service as workaround:
app/services/store.js:

import Ember from 'ember';
import StoreService from 'ember-data/store';

const {RSVP, run} = Ember;

export default StoreService.extend({
  unloadAll() {
    const args = arguments;
    return new RSVP.Promise(resolve => {
      this._super(...args);
      run.schedule('destroy', this, function() {
        resolve();
      })
    })
  }
});

Thx @NagiTs and @simonihmig for idea.

@Kilowhisky
Copy link

+1 having this same issue. Seems a recent unloading records change is causing numerous regressions.

@sly7-7
Copy link
Contributor

sly7-7 commented May 19, 2017

yeah, maybe #4986 has the same root cause

kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this issue May 23, 2017
closes TryGhost/Ghost#8307
- Ember 2.13 has a behaviour change to `store.unloadAll` where the records aren't removed until later in the runloop resulting in errors if the store is used to find records before it's completed. See emberjs/data#4963
kirrg001 pushed a commit to TryGhost/Admin that referenced this issue May 23, 2017
closes TryGhost/Ghost#8307

- Ember 2.13 has a behaviour change to `store.unloadAll` where the records aren't removed until later in the runloop resulting in errors if the store is used to find records before it's completed. See emberjs/data#4963
@workmanw
Copy link

workmanw commented Jun 1, 2017

We are experiencing both this issue and #4986. Based on my own digging, I believe to both be related

@raido
Copy link

raido commented Jun 1, 2017

How we could get more attention on this?

@fantazey
Copy link

fantazey commented Jun 2, 2017

I have same issue.
@raido, we should left here MOAR comments

@stevesims
Copy link

We are experiencing this too, plus I believe also #4986, and have seen another exception that would appear to be closely related, but doesn't match exactly.

The further example is visiting route A which loads up a lot of records using a complex query, leaving it for route B, whereby the willTransition for the route A calls unloadRecord for all the data the complex query had loaded, and then re-visiting the route A. This causes the same query to be performed, but I then see an exception like Assertion Failed: calling set on destroyed object: <DS.PromiseManyArray:ember2176>.content = <DS.ManyArray:ember3569> from deep within ember-data ManyRelationship._updateLoadingPromise is in the stack. Obviously in this example the destroy loop cycle should have run several times.

@sly7-7
Copy link
Contributor

sly7-7 commented Jun 19, 2017

@shamcode I think #5011 fixes this, at least I just tried with your repo, and and the error is gone.
I'm closing

@sly7-7 sly7-7 closed this as completed Jun 19, 2017
@raido
Copy link

raido commented Jun 19, 2017

https://github.com/raido/ember-data-unloadAll-regression - this is basically the same as the unit test case I posted before, except one can easily test with different ED versions against the test case.

This is still failing.

@raido
Copy link

raido commented Jun 19, 2017

Also this: raido@12d6984 basically it the same as previous comment just in unit test form.

@workmanw
Copy link

workmanw commented Jun 20, 2017

So I can confirm that #5011 does fix this with regards to the eachAttribute issue. However I'm left with a new issue that immediately after unloading, I get a bunch of:

Assertion Failed: You looked up the 'evalWallItems' relationship on a 'wallitem' with id MTpXYWxsSXRlbSwxMTg5MDAwMQ but some of the associated records were not loaded. Either make sure they are all loaded together with the parent record, or specify that the relationship is async ('DS.hasMany({ async: true })')

In my app, I'm trying to unload a bunch of records that have bidirectional relationships. It seems regardless of which order I unload the records, I get this issue. This was not the case with 1.12. The same set of code works fine.


Edit: After much more research and digging, I can confirm that this issue is fixed. The other issue I mentioned in this comment was ultimately caused by the issue described in #4986. The error was being caught and it caused this other issue to surface. So nothing to see here, once #4986 is fixed I should be 👍.

@rinoldsimon
Copy link

Facing similar problem. Is the bug fixed in ember version 2.14.0 ?

@jrjohnson
Copy link
Contributor

Just a note in case it helps anyone. I was having this issue in some of my acceptance tests, turned out to be I hadn't built all of the relationships correctly in mirage. It was thrown when a record referenced in one model was not returned by the (fake) API.

@adamreisnz
Copy link

adamreisnz commented Aug 1, 2017

We are still having this issue in 2.16.0-canary (master), so it doesn't seem like #5011 fixed this for all cases.

TypeError: Cannot read property 'eachAttribute' of null
    at new Snapshot (-private.js:5111)

It works fine with Ember data 2.12.

Offending code:

items.forEach((item) => this.store.unloadRecord(item));

Where items are the child in a hasMany relationship. Basically as per #4986

Thoughts?
cc @runspired @stefanpenner

@Kilowhisky
Copy link

Kilowhisky commented Aug 1, 2017 via email

@adamreisnz
Copy link

@Kilowhisky thanks, but that was not the issue here.

@tylerturdenpants
Copy link
Member

@Kilowhisky now that you have brought that up I'm going to investigate it a little further. Never considered the possible mutation.

@rinoldsimon
Copy link

Does this bug has been fixed in Ember v2.15.1 ?

@BryanCrotaz
Copy link
Contributor

still seeing this in 2.14.10

@lolmaus
Copy link
Contributor

lolmaus commented Oct 3, 2017

My workaround is to destroy the record before unloading it:

// fails
record.unloadRecord()

// works
record
  .destroyRecord()
  .then(() => {
    record.unloadRecord()
  })

Also, we're applying this fix after unloading: #5041 (comment)

@Devoter
Copy link

Devoter commented Feb 2, 2018

Ember 2.13.2
I'm trying to unload already destroyed (from another user) record when the current user is trying to remove it again:

record.destroyRecord()
    .then(() => doSomething())
    .catch(reason => {
        if (reason && reason.errors && reason.errors[0] && reason.errors[0].status === '404') {
            record.unloadRecord();
        }
    });

But this code throws an error only if I have some reverse relations in my Ember models.
For example:

let ModelOne = DS.Model.extend({
    models: DS.hasMany('model-two')
});
let ModelTwo = DS.Model.extend({
    owner: DS.belongsTo('model-one')
});

I cannot update the project right now due to some dependencies.

@tylerturdenpants
Copy link
Member

@Devoter I don’t think this going to be addressed. There have been numerous unload issues on version of ember data past 2.12. Some small issues were addressed but not much. This is what I suggest (since I have too) to pin your project at 2.12.

@armiiller
Copy link

Similar issue here :/ pinning to 2.12 for now

@sly7-7
Copy link
Contributor

sly7-7 commented Feb 14, 2018

@armiiller @tylerturdenpants @Devoter did you try the current release, ie 3.0.1 ?
I was pinnin to 2.12 until this release, and it seems to work well on my side.

@armiiller
Copy link

@sly7-7 - I'll try it out later this afternoon. Have you noticed any other side affects from jumping a major release?

@sly7-7
Copy link
Contributor

sly7-7 commented Feb 15, 2018

@armiiller For the moment, not at all. All 💚

@Devoter
Copy link

Devoter commented Feb 16, 2018

@sly7-7 I cannot try the current release due to third party dependencies of my project.

@sly7-7
Copy link
Contributor

sly7-7 commented Feb 16, 2018

@Devoter Not even ember-data 2.18.1 ? If yes, it also has the fix, otherwise I'm just sad for you, I don't have any idea how to workaround this bug.

@amk221
Copy link
Contributor

amk221 commented Mar 8, 2018

I'm getting this in ED 3.0.2

@sly7-7
Copy link
Contributor

sly7-7 commented Mar 8, 2018

@amk221 Yeah, in some cases this bug is still present. I had to downgrade to 2.12.2. Issues are tracked in #5353 #5354 and #5343

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

No branches or pull requests