[FIX] web: cache rejected promises #237984
Conversation
|
|
C3POdoo
requested review from
a team,
BastienFafchamps and
kebeclibre
and removed request for
a team
LucasLefevre
force-pushed
the
saas-18.3-rejected-persistent-cache-lul
branch
3 times, most recently
from
ed9003e to
58fe3ad
Compare
| fallback() | ||
| .then((result) => { | ||
| this.indexedDB.write(table, key, result); | ||
| this.ramCache.write(table, key, result); |
There was a problem hiding this comment.
is this necessary as we already have def that resolves with result in the ram cache? If this is necessary somehow, I think you should write Promise.resolve(result) in the cache, s.t. further calls receive a promise as expected (line 44)
There was a problem hiding this comment.
Yes, it is necessary when the value is retrieved first from indexedDB (which resolves def), but then the fallback returns another value: ramCache needs to be updated with the newer value. (it is covered by a test).
I adapted the code with Promise.resolve(result)
As of 19 I think it's fine because we reworked the function (which is more complicated as of 18.4)
👍 I'll just leave the test in the fw-port then
There was a problem hiding this comment.
Yes, it is necessary when the value is retrieved first from indexedDB (which resolves
def), but then the fallback returns another value:ramCacheneeds to be updated with the newer value. (it is covered by a test).
Of course
Let's say we trigger two times a RPC that fails.
The first time, the cache is empty and we make the RPC. The ram cache is
filled with `prom`.
The second time, we get `prom` from the ram cache.
When the RPC actually finishes, we end up in
```
.catch((error) => {
this.ramCache.delete(table, key);
def.reject(error);
});
```
=> the RPC failure is catched, which means `prom` itself is not rejected.
Instead, it resolves with the result of `.catch(...)`, which is `undefined`
Here is a simplified toy example:
```
const prom = Promise.resolve(
Promise.reject(new Error("fetch failed")).catch(() => {})
);
```
In this example, `prom` is successful and the result is `undefined`
Co-authored-by: rrahir <rar@odoo.com>
LucasLefevre
force-pushed
the
saas-18.3-rejected-persistent-cache-lul
branch
from
58fe3ad to
05d2a5b
Compare
Let's say we trigger two times a RPC that fails.
The first time, the cache is empty and we make the RPC. The ram cache is
filled with `prom`.
The second time, we get `prom` from the ram cache.
When the RPC actually finishes, we end up in
```
.catch((error) => {
this.ramCache.delete(table, key);
def.reject(error);
});
```
=> the RPC failure is catched, which means `prom` itself is not rejected.
Instead, it resolves with the result of `.catch(...)`, which is `undefined`
Here is a simplified toy example:
```
const prom = Promise.resolve(
Promise.reject(new Error("fetch failed")).catch(() => {})
);
```
In this example, `prom` is successful and the result is `undefined`
closes #237984
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>
Co-authored-by: rrahir <rar@odoo.com>
3 participants
Let's say we trigger two times a RPC that fails.
The first time, the cache is empty and we make the RPC. The ram cache is filled with
prom.The second time, we get
promfrom the ram cache.When the RPC actually finishes, we end up in
=> the RPC failure is catched, which means
promitself is not rejected. Instead, it resolves with the result of.catch(...), which isundefinedHere is a simplified toy example:
In this example,
promis successful and the result isundefined"
Description of the issue/feature this PR addresses:
Current behavior before PR:
Desired behavior after PR is merged:
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr