Controls cache invalidation or persistance on errors#4945
Merged
Conversation
a635ec7 to
811c35b
Compare
sergeibbb
added a commit
that referenced
this pull request
Jan 30, 2026
811c35b to
81c5010
Compare
🤖 Augment PR SummarySummary: This PR refines integration caching behavior to better control whether cached entries are invalidated or retained when requests fail. Changes:
Technical Notes: Failed-promise caching behavior is now configurable via 🤖 Was this summary useful? React with 👍 or 👎 |
27a493c to
0cf4471
Compare
ramin-t
reviewed
Feb 12, 2026
Contributor
ramin-t
left a comment
There was a problem hiding this comment.
Just wanted to double-check with you on something I noticed
Comment on lines
+625
to
+626
| // Re-throw to the caller | ||
| throw ex; |
Contributor
There was a problem hiding this comment.
We are throwing the error up the chain now on every error except for cancellations, where before these changes we would swallow the error and return undefined.
Just want to make sure this is intended and we have considered side effects of this logic change.
Member
Author
There was a problem hiding this comment.
@axosoft-ramint
Yes — it's intended. Actually the main goal of the current PR is being able to throw while keep caching the result (even an error result).
The outcome is the following:
- Once the
undefinedwas cached, the Launchpad stopped indicating that there is a problem with GitLab authentication token: it just showed an empty list. We wanted to throw, but before these changes the error result was not cached so we would hit the same endpoint again and again, receiving the same problematic result. - Now, when it throws, it will display the note that an error has happened. Even if it's some other (non-auth) error, it seems useful to keep the clarity for user and indicate that part of PRs is not shown due to an error.
49b4888 to
68ec239
Compare
68ec239 to
c6ccc34
Compare
Also add an additional check for setting resolved values in possible race condition. The `set` method now returns the promise associated with an asynchronous cache value. This enables the `get` method to reliably attach a `finally` handler for cache invalidation directly to the promise managing the cache item's eventual resolution. This change also introduces a check within the `set` method's promise resolution callback to prevent stale updates. If a cache entry has been modified or removed while an asynchronous value was pending, the original promise's resolution will no longer overwrite the current cache state. This enhances cache consistency and robustness against race conditions. Error handling for `expireOnError` is also properly integrated into the returned promise chain. (#4942, #4944)
c6ccc34 to
aa01d59
Compare
ianhattendorf
approved these changes
Feb 26, 2026
ramin-t
approved these changes
Feb 26, 2026
Contributor
ramin-t
left a comment
There was a problem hiding this comment.
Thanks for the explanation!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Solves #4942
Controls cache invalidation or persistence on errors.
The goal is to be able to preserve some types of errors in cache, to avoid retrieving the same failures repeatedly.
Be careful, because it makes #4943 more noticeable by preserving the cached Auth Error even if user re-connects. Workaround for user: reload the window. Probably we do not want to merge this PR until #4943 is done.
Background
Initially it's been done as part of #4731 but while revealing more and more nuances I decided to ship in smaller changes to make things reviewed and merged faster.
Checklist
Fixes $XXX -orCloses #XXX -prefix to auto-close the issue that your PR addresses