diff --git a/CHANGELOG.md b/CHANGELOG.md index ef9c399f7432d..02cfe987c6456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Fixes an issue where the GitKraken MCP installation could fail or conflict across remote environments like WSL, SSH, or containers ([#4918](https://github.com/gitkraken/vscode-gitlens/issues/4918)) - Fixes an issue where diffing untracked files in the _Commit Composer_ could trigger unwanted file system events ([#4917](https://github.com/gitkraken/vscode-gitlens/issues/4917)) - Fixes an issue where commit messages become invisible during interactive rebase in some themes ([#4886](https://github.com/gitkraken/vscode-gitlens/issues/4886)) +- Fixes an issue where authentication errors were not properly displayed in _Launchpad_ when GitLab integration credentials were revoked ([#4944](https://github.com/gitkraken/vscode-gitlens/issues/4944)) - Fixes issue in the _Commit Graph_ minimap where it only shows a spinner when the repo has no commits ([#4741](https://github.com/gitkraken/vscode-gitlens/issues/4741)) - Fixes an inline markdown rendering issue in the _Interactive Rebase Editor_ ([#4914](https://github.com/gitkraken/vscode-gitlens/issues/4914)) - Fixes an issue where opening a deep link to create a PR worktree would incorrectly prompt to add a remote that already exists ([#4926](https://github.com/gitkraken/vscode-gitlens/issues/4926)) diff --git a/src/cache.ts b/src/cache.ts index cc667dec0a6ee..aa272ec2ec7a3 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -11,6 +11,7 @@ import type { ResourceDescriptor } from './git/models/resourceDescriptor.js'; import type { GitHostIntegration } from './plus/integrations/models/gitHostIntegration.js'; import type { IntegrationBase } from './plus/integrations/models/integration.js'; import { isPromise } from './system/promise.js'; +import { CacheController } from './system/promiseCache.js'; type Caches = { defaultBranch: { key: `repo:${string}`; value: DefaultBranch }; @@ -31,7 +32,7 @@ type CacheKey = Caches[T]['key']; type CacheValue = Caches[T]['value']; type CacheResult = Promise | T | undefined; -type Cacheable = () => { value: CacheResult; expiresAt?: number }; +type Cacheable = (cacheable: CacheController) => { value: CacheResult; expiresAt?: number }; type Cached = | { value: T | undefined; @@ -46,6 +47,8 @@ type Cached = etag?: string; }; +type ExpiryOptions = { expiryOverride?: boolean | number; expireOnError?: boolean }; + export class CacheProvider implements Disposable { private readonly _cache = new Map<`${Cache}:${CacheKey}`, Cached>>>(); @@ -65,7 +68,7 @@ export class CacheProvider implements Disposable { key: CacheKey, etag: string | undefined, cacheable: Cacheable>, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult> { const item = this._cache.get(`${cache}:${key}`); @@ -85,8 +88,18 @@ export class CacheProvider implements Disposable { (expiry != null && expiry > 0 && expiry < Date.now()) || (item.etag != null && item.etag !== etag) ) { - const { value, expiresAt } = cacheable(); - return this.set(cache, key, value, etag, expiresAt)?.value as CacheResult>; + const cacheController = new CacheController(); + const { value, expiresAt } = cacheable(cacheController); + if (isPromise(value)) { + void value.finally(() => { + if (cacheController.invalidated) { + this.delete(cache, key); + } + }); + } + return this.set(cache, key, value, etag, expiresAt, options?.expireOnError)?.value as CacheResult< + CacheValue + >; } return item.value as CacheResult>; @@ -95,7 +108,7 @@ export class CacheProvider implements Disposable { getCurrentAccount( integration: IntegrationBase, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getIntegrationKeyAndEtag(integration); return this.get('currentAccount', `id:${key}`, etag, cacheable, options); @@ -117,7 +130,7 @@ export class CacheProvider implements Disposable { resource: ResourceDescriptor, integration: IntegrationBase | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(resource, integration); @@ -138,7 +151,7 @@ export class CacheProvider implements Disposable { resource: ResourceDescriptor, integration: IntegrationBase | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(resource, integration); @@ -165,7 +178,7 @@ export class CacheProvider implements Disposable { resource: ResourceDescriptor, integration: IntegrationBase | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(resource, integration); @@ -192,7 +205,7 @@ export class CacheProvider implements Disposable { repo: ResourceDescriptor, integration: GitHostIntegration | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(repo, integration); // Wrap the cacheable so we can also add the result to the issuesOrPrsById cache @@ -210,7 +223,7 @@ export class CacheProvider implements Disposable { repo: ResourceDescriptor, integration: GitHostIntegration | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(repo, integration); // Wrap the cacheable so we can also add the result to the issuesOrPrsById cache @@ -227,7 +240,7 @@ export class CacheProvider implements Disposable { repo: ResourceDescriptor, integration: GitHostIntegration | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(repo, integration); return this.get('defaultBranch', `repo:${key}`, etag, cacheable, options); @@ -237,7 +250,7 @@ export class CacheProvider implements Disposable { repo: ResourceDescriptor, integration: GitHostIntegration | undefined, cacheable: Cacheable, - options?: { expiryOverride?: boolean | number }, + options?: ExpiryOptions, ): CacheResult { const { key, etag } = getResourceKeyAndEtag(repo, integration); return this.get('repoMetadata', `repo:${key}`, etag, cacheable, options); @@ -249,17 +262,14 @@ export class CacheProvider implements Disposable { value: CacheResult>, etag: string | undefined, expiresAt?: number, + expireOnError?: boolean, ): Cached>> { let item: Cached>>; if (isPromise(value)) { - void value.then( - v => { - this.set(cache, key, v, etag, expiresAt); - }, - () => { - this.delete(cache, key); - }, - ); + void value.then(v => this.set(cache, key, v, etag, expiresAt, expireOnError)); + if (expireOnError !== false) { + void value.catch(() => this.delete(cache, key)); + } item = { value: value, etag: etag, cachedAt: Date.now() }; } else { @@ -280,8 +290,8 @@ export class CacheProvider implements Disposable { key: string, etag: string | undefined, ): Cacheable { - return () => { - const item = cacheable(); + return cacheController => { + const item = cacheable(cacheController); if (isPromise(item.value)) { void item.value.then(v => { if (v != null) { diff --git a/src/plus/integrations/models/integration.ts b/src/plus/integrations/models/integration.ts index 16325aa506ee7..ff1295509e41f 100644 --- a/src/plus/integrations/models/integration.ts +++ b/src/plus/integrations/models/integration.ts @@ -603,19 +603,31 @@ export abstract class IntegrationBase< const currentAccount = await this.container.cache.getCurrentAccount( this, - () => ({ + cacheable => ({ value: (async () => { try { const account = await this.getProviderCurrentAccount?.(this._session!, opts); this.resetRequestExceptionCount('getCurrentAccount'); return account; } catch (ex) { + if (ex instanceof CancellationError) { + cacheable.invalidate(); + return undefined; + } + this.handleProviderException('getCurrentAccount', ex, { scope: scope }); - return undefined; + + // Invalidate the cache on error, except for auth errors + if (!(ex instanceof AuthenticationError)) { + cacheable.invalidate(); + } + + // Re-throw to the caller + throw ex; } })(), }), - { expiryOverride: expiryOverride }, + { expiryOverride: expiryOverride, expireOnError: false }, ); return currentAccount; }