BaseEntity.reload returned promise doesn't wait for all queries

[davewasmer]

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[x] N/A
[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[ ] @next
[x] 0.2.12

Steps to reproduce or a small repository showing the problem:

The BaseEntity.reload() method fetches the entity from the database and does a mass assignment from the newly fetched entity onto the existing entity that is being reloaded:

    async reload(): Promise<void> {
        const base: any = this.constructor;
        const newestEntity: BaseEntity = await base.getRepository().findOneOrFail(base.getId(this));

        ObjectUtils.assign(this, newestEntity);  // <-- mass assignment
    }

This mass assignment triggers any lazy relationships for this entity to load since lazy relationships are represented as getters. That results in (potentially) a series of queries hitting the database.

However, the promise returned by reload() doesn't track any of these. The result is that queries hit the database after await entity.reload() finishes.

This is somewhat surprising, and also makes certain scenarios difficult. For example, tests that hit .reload() and also teardown the database connection on completion will start throwing Connection terminated errors, because the test can't wait for those queries that .reload() triggers to finish before disconnecting).

I'm not sure the best path forward to fix this. Whether or not those lazy relationships should be reloaded isn't obvious to me. If we are okay not reloading them, then we could filter the mass assignment down to exclude lazy-loaded relationships. Or, if we do want to reload those lazy relationships as well, then we could filter out those relationships so we can capture their promises.

[ghost]

Lazy-Loading with Promises was abandonded in the end of 2018. There sadly will be no further support and i also do have mny problems with Lazy-Loading, which seem to be never solved.
See this comment as ref:
#2902 (comment)

[davewasmer]

Hm, from my reading of that comment, @pleerock said he was considering dropping it. But the website still lists it as a feature, albeit an experimental.

@pleerock have you made a final choice on lazy relations yet? If so, and assuming the decision is to drop support, would you be willing to accept a PR that removes them from the website, or at least makes it clear that it's not supported (vs. experimental).

[ghost]

The thing is, that the PR from my link would fix some very important thing for lazy loading. But from the comment from las year i assume it never gets merged. If Promises would be supported, they should merge the PRs that are important for them to work and not putting them on hold.

[Kononnable]

Lazy loading will be reinvented in 0.4.0. Current behavior isn't working very well(e.g. when setting value you have to pack it in a promise) and it's error prone.

As for referenced PR - I'm not sure if it will be merged. I think it's more about priorities. Unfortunately we don't have as much time as we would like so we have to prioritize. Deciding either to merge or not this PR takes time and PRs 'with laziness' were troublesome in the past. It's easy to break other functionalities by them so it have to be revived carefully(which takes more time then reviving some other PRs).

[mvlabat]

I'm having the same issue with lazy relations and using reload. Is there any timeline when to expect the fix or 0.4.0?

[hivtushok]

+1 having this issue, it caused flakiness in our tests. Any chances to have a fix for it anytime soon?