Skip to content

Commit 031e12a

Browse files
sergeibbbsergeibbb
committed
WIP: control cache invlidation or persistance on errors
1 parent 95423e1 commit 031e12a

File tree

2 files changed

+47
-25
lines changed

2 files changed

+47
-25
lines changed

‎src/cache.ts‎

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type { ResourceDescriptor } from './git/models/resourceDescriptor';
1111
import type { GitHostIntegration } from './plus/integrations/models/gitHostIntegration';
1212
import type { IntegrationBase } from './plus/integrations/models/integration';
1313
import { isPromise } from './system/promise';
14+
import { CacheController } from './system/promiseCache';
1415

1516
type Caches = {
1617
defaultBranch: { key: `repo:${string}`; value: DefaultBranch };
@@ -31,7 +32,7 @@ type CacheKey<T extends Cache> = Caches[T]['key'];
3132
type CacheValue<T extends Cache> = Caches[T]['value'];
3233
type CacheResult<T> = Promise<T | undefined> | T | undefined;
3334

34-
type Cacheable<T> = () => { value: CacheResult<T>; expiresAt?: number };
35+
type Cacheable<T> = (cacheable: CacheController) => { value: CacheResult<T>; expiresAt?: number };
3536
type Cached<T> =
3637
| {
3738
value: T | undefined;
@@ -46,6 +47,8 @@ type Cached<T> =
4647
etag?: string;
4748
};
4849

50+
type ExpiryOptions = { expiryOverride?: boolean | number; expireOnError?: boolean };
51+
4952
export class CacheProvider implements Disposable {
5053
private readonly _cache = new Map<`${Cache}:${CacheKey<Cache>}`, Cached<CacheResult<CacheValue<Cache>>>>();
5154

@@ -65,7 +68,7 @@ export class CacheProvider implements Disposable {
6568
key: CacheKey<T>,
6669
etag: string | undefined,
6770
cacheable: Cacheable<CacheValue<T>>,
68-
options?: { expiryOverride?: boolean | number },
71+
options?: ExpiryOptions,
6972
): CacheResult<CacheValue<T>> {
7073
const item = this._cache.get(`${cache}:${key}`);
7174

@@ -85,8 +88,18 @@ export class CacheProvider implements Disposable {
8588
(expiry != null && expiry > 0 && expiry < Date.now()) ||
8689
(item.etag != null && item.etag !== etag)
8790
) {
88-
const { value, expiresAt } = cacheable();
89-
return this.set<T>(cache, key, value, etag, expiresAt)?.value as CacheResult<CacheValue<T>>;
91+
const cacheController = new CacheController();
92+
const { value, expiresAt } = cacheable(cacheController);
93+
if (isPromise(value)) {
94+
void value.finally(() => {
95+
if (cacheController.invalidated) {
96+
this.delete(cache, key);
97+
}
98+
});
99+
}
100+
return this.set<T>(cache, key, value, etag, expiresAt, options?.expireOnError)?.value as CacheResult<
101+
CacheValue<T>
102+
>;
90103
}
91104

92105
return item.value as CacheResult<CacheValue<T>>;
@@ -95,7 +108,7 @@ export class CacheProvider implements Disposable {
95108
getCurrentAccount(
96109
integration: IntegrationBase,
97110
cacheable: Cacheable<Account>,
98-
options?: { expiryOverride?: boolean | number },
111+
options?: ExpiryOptions,
99112
): CacheResult<Account> {
100113
const { key, etag } = getIntegrationKeyAndEtag(integration);
101114
return this.get('currentAccount', `id:${key}`, etag, cacheable, options);
@@ -117,7 +130,7 @@ export class CacheProvider implements Disposable {
117130
resource: ResourceDescriptor,
118131
integration: IntegrationBase | undefined,
119132
cacheable: Cacheable<IssueOrPullRequest>,
120-
options?: { expiryOverride?: boolean | number },
133+
options?: ExpiryOptions,
121134
): CacheResult<IssueOrPullRequest> {
122135
const { key, etag } = getResourceKeyAndEtag(resource, integration);
123136

@@ -138,7 +151,7 @@ export class CacheProvider implements Disposable {
138151
resource: ResourceDescriptor,
139152
integration: IntegrationBase | undefined,
140153
cacheable: Cacheable<Issue>,
141-
options?: { expiryOverride?: boolean | number },
154+
options?: ExpiryOptions,
142155
): CacheResult<Issue> {
143156
const { key, etag } = getResourceKeyAndEtag(resource, integration);
144157

@@ -165,7 +178,7 @@ export class CacheProvider implements Disposable {
165178
resource: ResourceDescriptor,
166179
integration: IntegrationBase | undefined,
167180
cacheable: Cacheable<PullRequest>,
168-
options?: { expiryOverride?: boolean | number },
181+
options?: ExpiryOptions,
169182
): CacheResult<PullRequest> {
170183
const { key, etag } = getResourceKeyAndEtag(resource, integration);
171184

@@ -192,7 +205,7 @@ export class CacheProvider implements Disposable {
192205
repo: ResourceDescriptor,
193206
integration: GitHostIntegration | undefined,
194207
cacheable: Cacheable<PullRequest>,
195-
options?: { expiryOverride?: boolean | number },
208+
options?: ExpiryOptions,
196209
): CacheResult<PullRequest> {
197210
const { key, etag } = getResourceKeyAndEtag(repo, integration);
198211
// Wrap the cacheable so we can also add the result to the issuesOrPrsById cache
@@ -210,7 +223,7 @@ export class CacheProvider implements Disposable {
210223
repo: ResourceDescriptor,
211224
integration: GitHostIntegration | undefined,
212225
cacheable: Cacheable<PullRequest>,
213-
options?: { expiryOverride?: boolean | number },
226+
options?: ExpiryOptions,
214227
): CacheResult<PullRequest> {
215228
const { key, etag } = getResourceKeyAndEtag(repo, integration);
216229
// Wrap the cacheable so we can also add the result to the issuesOrPrsById cache
@@ -227,7 +240,7 @@ export class CacheProvider implements Disposable {
227240
repo: ResourceDescriptor,
228241
integration: GitHostIntegration | undefined,
229242
cacheable: Cacheable<DefaultBranch>,
230-
options?: { expiryOverride?: boolean | number },
243+
options?: ExpiryOptions,
231244
): CacheResult<DefaultBranch> {
232245
const { key, etag } = getResourceKeyAndEtag(repo, integration);
233246
return this.get('defaultBranch', `repo:${key}`, etag, cacheable, options);
@@ -237,7 +250,7 @@ export class CacheProvider implements Disposable {
237250
repo: ResourceDescriptor,
238251
integration: GitHostIntegration | undefined,
239252
cacheable: Cacheable<RepositoryMetadata>,
240-
options?: { expiryOverride?: boolean | number },
253+
options?: ExpiryOptions,
241254
): CacheResult<RepositoryMetadata> {
242255
const { key, etag } = getResourceKeyAndEtag(repo, integration);
243256
return this.get('repoMetadata', `repo:${key}`, etag, cacheable, options);
@@ -249,17 +262,14 @@ export class CacheProvider implements Disposable {
249262
value: CacheResult<CacheValue<T>>,
250263
etag: string | undefined,
251264
expiresAt?: number,
265+
expireOnError?: boolean,
252266
): Cached<CacheResult<CacheValue<T>>> {
253267
let item: Cached<CacheResult<CacheValue<T>>>;
254268
if (isPromise(value)) {
255-
void value.then(
256-
v => {
257-
this.set(cache, key, v, etag, expiresAt);
258-
},
259-
() => {
260-
this.delete(cache, key);
261-
},
262-
);
269+
void value.then(v => this.set(cache, key, v, etag, expiresAt, expireOnError));
270+
if (expireOnError !== false) {
271+
void value.catch(() => this.delete(cache, key));
272+
}
263273

264274
item = { value: value, etag: etag, cachedAt: Date.now() };
265275
} else {
@@ -280,8 +290,8 @@ export class CacheProvider implements Disposable {
280290
key: string,
281291
etag: string | undefined,
282292
): Cacheable<PullRequest> {
283-
return () => {
284-
const item = cacheable();
293+
return cacheController => {
294+
const item = cacheable(cacheController);
285295
if (isPromise(item.value)) {
286296
void item.value.then(v => {
287297
if (v != null) {

‎src/plus/integrations/models/integration.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -600,19 +600,31 @@ export abstract class IntegrationBase<
600600

601601
const currentAccount = await this.container.cache.getCurrentAccount(
602602
this,
603-
() => ({
603+
cacheable => ({
604604
value: (async () => {
605605
try {
606606
const account = await this.getProviderCurrentAccount?.(this._session!, opts);
607607
this.resetRequestExceptionCount('getCurrentAccount');
608608
return account;
609609
} catch (ex) {
610+
if (ex instanceof CancellationError) {
611+
cacheable.invalidate();
612+
return undefined;
613+
}
614+
610615
this.handleProviderException('getCurrentAccount', ex, { scope: scope });
611-
return undefined;
616+
617+
// Invalidate the cache on error, except for auth errors
618+
if (!(ex instanceof AuthenticationError)) {
619+
cacheable.invalidate();
620+
}
621+
622+
// Re-throw to the caller
623+
throw ex;
612624
}
613625
})(),
614626
}),
615-
{ expiryOverride: expiryOverride },
627+
{ expiryOverride: expiryOverride, expireOnError: false },
616628
);
617629
return currentAccount;
618630
}

0 commit comments

Comments
 (0)