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

[BUGFIX relationships] fix infinite retry bug for failed relationship fetches #6112

Merged
merged 4 commits into from
Jun 13, 2019

Conversation

runspired
Copy link
Contributor

@runspired runspired commented May 15, 2019

This PR unskips tests skipped for landing RecordData and fixes regressions introduced around relationship fetching in 3.5

cc @code0100fun please confirm this resolves #5814 et al.

@runspired runspired force-pushed the fix-reload-on-error branch 3 times, most recently from 0dd8ddb to 2f778c2 Compare May 15, 2019 02:59
@runspired runspired force-pushed the fix-reload-on-error branch from 2f778c2 to 737cca3 Compare May 15, 2019 16:11
code0100fun pushed a commit to heroku/ember-data that referenced this pull request May 20, 2019
@runspired runspired force-pushed the fix-reload-on-error branch from 737cca3 to 5bfe4b4 Compare May 22, 2019 00:24
@code0100fun
Copy link
Contributor

🎉 Thanks for fixing this! It will be awesome to get back on the latest release 😄

This did fix my errors with infinite 404's. However, I noticed a few more errors in my test suite when running with this branch (they do not happen on current master branch). They all center around ember-data relationships and mostly when unloading a new record.

One bug seems to happen when pushing directly into a relationship (ex: aModel.hasMany('child-model').push(data) ) and immediately after calling newChildModel.unloadRecord(). This causes computeds with cache keys like 'aModel.childModels.[]' to re-evaluate but there is a null item in the relationship collection. This one is tricky and I'm not sure how to reproduce in a test.

Another bug I am still working through is that calling newChildModel.unloadRecord() causes the internal model to be in an isEmpty state. Afterwards, calling aModel.childModels calls InternalModel#getHasMany which eventually gets down to PrivateStore#_findEmptyInternalModel and calls _scheduleFetch for a model with a null id. This one is relativly straight forward so I think I can put together a failing test for it.

I still have one other test error to investigate but I'm hoping it is just another manifestation of one of the above issues.

@runspired runspired force-pushed the fix-reload-on-error branch 2 times, most recently from aa7579b to c78080d Compare June 13, 2019 22:08
@runspired
Copy link
Contributor Author

Test failures are the usual suspects (flakey Travis test, odd linting issue when running M3 with newer Data)

@runspired runspired merged commit 377b88d into master Jun 13, 2019
@runspired runspired deleted the fix-reload-on-error branch June 13, 2019 23:21
pete-the-pete pushed a commit to pete-the-pete/data that referenced this pull request Jul 23, 2019
… fetches (emberjs#6112)

* [BUGFIX relationships] fix infinite retry bug for failed relationship fetches
* Typescript cleanup
* adds test coverage for unloading newly created records from relationships
* [BUGFIX unloadRecord] unloading newly created records should not trigger fetch
MichalBryxi pushed a commit to MichalBryxi/empress-blog that referenced this pull request Jul 30, 2019
 - There is a problem with ember-data >= 3.5.0 that invalid relationships gets refetched over and over again
 - 3.11.x series of ember-data does not have the needed fix yet emberjs/data#6112 and never (beta) versions complain about `ember-fetch` that complains about other missing dependency and yady yady yada
 - Pinning ember-data to LTS version does not seem to hurt anyone and avoids this problem
@mansona
Copy link
Member

mansona commented Jul 30, 2019

Is this going to be backported? I'm just wondering because of the large impact of the bug?

@runspired
Copy link
Contributor Author

@mansona not likely given it wasn’t reported much despite being an issue for 6 releases and its a harder fix to port. It’ll be in 3.12.

@jakebixbyavalara
Copy link

@runspired we're actually experiencing this issue with v3.1.2 (which we upgraded to for the fix for #4963). We're aiming to upgrade rapidly soon, but have a few blocking factors that we have to tackle first.
Do you have any suggestions on patching to at least short circuit the retry loop?

@Leooo
Copy link

Leooo commented Feb 18, 2020

@runspired any Idea if we can consume Ember data 3.12 with Ember 3.4 ? Trying ED 3.5 upgrade atm and the bug above would be a blocker for us (upgrading to higher Ember versions across our main app + its engines would take us months). There was another blocker bug in ED 3.4

@mansona @jakebixbyavalara interested if you have any ways around this thanks
Thanks

@Mifrill
Copy link

Mifrill commented Aug 24, 2021

@Leooo as for "have any ways around", you can try this way: https://github.com/Mifrill/ember-data-stop-infinite-retry-for-failed-relationship-fetches

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 this pull request may close these issues.

Infinite belongsTo fetch after 404
6 participants