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

unloadingRecord w/ relationships fail in flushCanonical #3084

Closed
sdahlbac opened this issue May 20, 2015 · 17 comments
Closed

unloadingRecord w/ relationships fail in flushCanonical #3084

sdahlbac opened this issue May 20, 2015 · 17 comments

Comments

@sdahlbac
Copy link

The following models:

service-report.coffee
serviceReportLines: DS.hasMany('serviceReportLine', async: true)

service-report-line.coffee
task: DS.belongsTo('task', async: true)
serviceReport: DS.belongsTo('serviceReport', async: true)
usedSpareParts: DS.hasMany('usedSparePart', async: true)
timeEntries: DS.hasMany('timeEntry', async: true)

I have the following serializers:

app.ServiceReportSerializer = DS.RESTSerializer.extend(
  DS.EmbeddedRecordsMixin,
  attrs:
    serviceReportLines: embedded: "always"
)

app.ServiceReportLineSerializer = DS.RESTSerializer.extend(
  DS.EmbeddedRecordsMixin,
  attrs:
    usedSpareParts: embedded: "always"
    timeEntries: embedded: "always"
)

calling store.unloadRecord with a service-record gives the following error

Error: Assertion Failed: calling set on destroyed object
at new Error (native)
at Error.EmberError (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:12153:21)
at Object.Ember.default.assert (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:4854:13)
at Object.set (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:15722:22)
at exports.default.mixin.Mixin.create.set (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:29322:20)
at Ember.Object.extend.flushCanonical (http://localhost:42000/fieldops/ui/vendor/ember-data-1.0.0-beta.17.js:6882:14)
at ember$data$lib$system$relationships$state$has$many$$ManyRelationship.flushCanonical (http://localhost:42000/fieldops/ui/vendor/ember-data-1.0.0-beta.17.js:7125:22)
at Queue.invoke (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:878:18)
at Object.Queue.flush (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:943:13)
at Object.DeferredActionQueues.flush (http://localhost:42000/fieldops/ui/vendor/ember-1.12.0.debug.js:748:19)onerrorDefault @ ember-1.12.0.debug.js:26510__exports__.default.trigger @ ember-1.12.0.debug.js:46118Promise._onerror @ ember-1.12.0.debug.js:47130publishRejection @ ember-1.12.0.debug.js:45361(anonymous function) @ ember-1.12.0.debug.js:26472Queue.invoke @ ember-1.12.0.debug.js:878Queue.flush @ ember-1.12.0.debug.js:943DeferredActionQueues.flush @ ember-1.12.0.debug.js:748Backburner.end @ ember-1.12.0.debug.js:173Backburner.run @ ember-1.12.0.debug.js:228run @ ember-1.12.0.debug.js:15863ember$data$lib$system$adapter$$Adapter.extend.ajax.Ember.RSVP.Promise.hash.success @ ember-data-1.0.0-beta.17.js:1670m.Callbacks.j @ jquery-1.11.2.min.js:2m.Callbacks.k.fireWith @ jquery-1.11.2.min.js:2x @ jquery-1.11.2.min.js:4m.ajaxTransport.send.b @ jquery-1.11.2.min.js:4

@pangratz
Copy link
Member

Can you reproduce this in this JSBin? This would help further track down the issue ...

@pangratz
Copy link
Member

Also, this looks like a duplicate of #2982, as the JSBin posted by @piotrpalek #2982 (comment) shows the same error stack trace. @sdahlbac can you confirm that the situation causing the error in the JSBin is similar to yours?

@sdahlbac
Copy link
Author

it looks very similar, but I think that in our case the failing line was this.set('length', toSet.length) and not !record.get('isDeleted')

but I'll play around with the jsbin (thanks for that btw) and see if I manage to come up with a reproduction case.

@pangratz
Copy link
Member

I updated the used ember-data version in the JSBin to use the latest build from the references branch and the the bug is gone. Maybe you can try out this branch as well and see if this helps with your issue? The build is available at ember-data-references.js.

Actually the above statement is not true as the error is still present using the references branch. I will investigate further... Sorry for the false alarm ...

@sdahlbac
Copy link
Author

I played around with the JSBin and managed to get the crash there

http://emberjs.jsbin.com/nevemitaru/3/edit?js,console

basically I just added

// ... and find it again
this.store.find('serviceReport', 1);

after unloading it

@sly7-7
Copy link
Contributor

sly7-7 commented May 20, 2015

After some quick debugging it appears that the failing flushCanonical is beeing called on the unloaded record's manyArray. This happens because 'destroy' is called synchronously, and flushcanonical is scheduled later in an other queue. Now I don't know if we could simply guard against the destroyed many array during flushcanonical or if we also should shedule later the relationship's destroy call.

@sly7-7
Copy link
Contributor

sly7-7 commented May 28, 2015

Well, I was able to make a test to reproduce the issue:

test("WTF is going on", function() {
  Post.reopen({
    comments: DS.hasMany('comment')
  });

  Comment.reopen({
    message: DS.belongsTo('post')
  });

  env.adapter.find = function(store, type, id, snapshot) {
    equal(type, Post, "find type was Post");
    equal(id, "1", "find id was 1");
    return Ember.RSVP.resolve({ id: 1, comments: [1,2] });
  };

  run(function() {
    env.store.pushMany('comment', [
      { id: 1, body: "First" },
      { id: 2, body: "Second" },
      { id: 3, body: "Third" }
    ]);
    //var myPost = env.store.push('post', { id: 1, comments: [3] }); // works
    var myPost = env.store.push('post', { id: 1, comments: [1] }); // crashes
    //var myPost = env.store.push('post', { id: 2, comments: [1] }); // crashes
    myPost.unloadRecord();
    //env.store.find('post', 1); // works
  });
  run(function(){
    env.store.find('post', 1); // crashes
  });
});

@igorT I definitely need you to go further, there seems to be some kind of race condition cause by the runloop, also, when there is no shared comments between the unloaded post and the later found post, it does not crash.

@igorT
Copy link
Member

igorT commented Jun 3, 2015

Taking a look

@igorT
Copy link
Member

igorT commented Jun 3, 2015

I think this should be fixed by moving to getters for relationships, and being smarter about unloading

@jdhines
Copy link

jdhines commented Oct 30, 2015

I have been trying to solve this issue for weeks now, unsuccessfully, when trying to do unloadAll() before doing a findAll() to make sure I get stuff fresh, and I found that I only have the problem when running something other than production. In other words, for my app, everything works fine with ember serve --environment=production but not with ember serve --environment=development. Why the difference, I don't know.

@Kilowhisky
Copy link

+1. I'm having the issue as well in my websocket based model update service. The only thing i've been able to do is reload the app so that it gets the correct data. Anything we can do to help move along this issue?

@jherdman
Copy link

I ran into this recently. The work around was to unload the has-many associations first.

@Kilowhisky
Copy link

I tried that but then encountered a problem where if the unloaded record was restored the relationship wasn't restored. Probably an unrelated bug.

@jherdman
Copy link

I'll keep my eye out for that, it's definitely a scenario I'll run into with my workaround.

@oswaldoacauan
Copy link

Any workarround for this?

@AC-TimRourke
Copy link

+1. This is also an issue for me. I've tried @jherdman's proposed solution, which works until I need to restore the records that were unloaded, as @Kilowhisky mentioned. In addition, the unload operation seems to lock up the browser while processing, though I can't say anything definitive as to why that is yet.

jgwhite added a commit to jgwhite/data that referenced this issue Mar 4, 2016
jgwhite added a commit to jgwhite/data that referenced this issue Mar 4, 2016
bmac pushed a commit to bmac/data that referenced this issue Mar 18, 2016
bmac pushed a commit that referenced this issue Mar 18, 2016
bmac pushed a commit that referenced this issue Mar 18, 2016
bmac pushed a commit that referenced this issue Mar 18, 2016
@wecc
Copy link
Contributor

wecc commented Oct 22, 2016

I'm closing this as #3559 has been merged. Feel free to re-open if that's not the case.

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

10 participants