diff --git a/src/cache.ts b/src/cache.ts index 8fc7cc0ab4752..a59da6394e576 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -11,6 +11,7 @@ import type { ResourceDescriptor } from './git/models/resourceDescriptor'; import type { GitHostIntegration } from './plus/integrations/models/gitHostIntegration'; import type { IntegrationBase } from './plus/integrations/models/integration'; import { isPromise } from './system/promise'; +import { CacheController } from './system/promiseCache'; 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/integrationService.ts b/src/plus/integrations/integrationService.ts index 1497b530478eb..2e6497b92db8b 100644 --- a/src/plus/integrations/integrationService.ts +++ b/src/plus/integrations/integrationService.ts @@ -792,29 +792,29 @@ export class IntegrationService implements Disposable { } const results = await Promise.allSettled(promises); - + const successfulResults = [ + ...flatten( + filterMap(results, r => + r.status === 'fulfilled' && r.value?.value != null ? r.value.value : undefined, + ), + ), + ]; const errors = [ ...filterMap(results, r => r.status === 'fulfilled' && r.value?.error != null ? r.value.error : undefined, ), ]; - if (errors.length) { - return { - error: errors.length === 1 ? errors[0] : new AggregateError(errors), - duration: Date.now() - start, - }; - } + + const error = + errors.length === 0 + ? undefined + : errors.length === 1 + ? errors[0] + : new AggregateError(errors, 'Failed to get some pull requests'); return { - value: [ - ...flatten( - filterMap(results, r => - r.status === 'fulfilled' && r.value != null && r.value?.error == null - ? r.value.value - : undefined, - ), - ), - ], + value: successfulResults, + error: error, duration: Date.now() - start, }; } diff --git a/src/plus/integrations/models/integration.ts b/src/plus/integrations/models/integration.ts index fc7294407158b..9ece13f344fe5 100644 --- a/src/plus/integrations/models/integration.ts +++ b/src/plus/integrations/models/integration.ts @@ -46,7 +46,7 @@ export type IntegrationKey = T extend export type IntegrationConnectedKey = `connected:${IntegrationKey}`; export type IntegrationResult = - | { value: T; duration?: number; error?: never } + | { value: T; duration?: number; error?: Error } | { error: Error; duration?: number; value?: never } | undefined; @@ -600,19 +600,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; } diff --git a/src/plus/launchpad/launchpad.ts b/src/plus/launchpad/launchpad.ts index e623efa428164..1fe78b34bbc59 100644 --- a/src/plus/launchpad/launchpad.ts +++ b/src/plus/launchpad/launchpad.ts @@ -41,6 +41,7 @@ import type { IntegrationIds } from '../../constants.integrations'; import { GitCloudHostIntegrationId, GitSelfManagedHostIntegrationId } from '../../constants.integrations'; import type { LaunchpadTelemetryContext, Source, Sources, TelemetryEvents } from '../../constants.telemetry'; import type { Container } from '../../container'; +import { AuthenticationError } from '../../errors'; import type { QuickPickItemOfT } from '../../quickpicks/items/common'; import { createQuickPickItemOfT, createQuickPickSeparator } from '../../quickpicks/items/common'; import type { DirectiveQuickPickItem } from '../../quickpicks/items/directive'; @@ -52,6 +53,8 @@ import { openUrl } from '../../system/-webview/vscode/uris'; import { getScopedCounter } from '../../system/counter'; import { fromNow } from '../../system/date'; import { some } from '../../system/iterable'; +import { Logger } from '../../system/logger'; +import { AggregateError } from '../../system/promise'; import { interpolate, pluralize } from '../../system/string'; import { ProviderBuildStatusState, ProviderPullRequestReviewState } from '../integrations/providers/models'; import type { LaunchpadCategorizedResult, LaunchpadItem } from './launchpadProvider'; @@ -157,6 +160,11 @@ const instanceCounter = getScopedCounter(); const defaultCollapsedGroups: LaunchpadGroup[] = ['draft', 'other', 'snoozed']; +const OpenLogsQuickInputButton: QuickInputButton = { + iconPath: new ThemeIcon('output'), + tooltip: 'Open Logs', +}; + export class LaunchpadCommand extends QuickCommand { private readonly source: Source; private readonly telemetryContext: LaunchpadTelemetryContext | undefined; @@ -565,10 +573,10 @@ export class LaunchpadCommand extends QuickCommand { return groupedAndSorted; }; - function getItemsAndQuickpickProps(isFiltering?: boolean) { + const getItemsAndQuickpickProps = (isFiltering?: boolean) => { const result = context.inSearch ? context.searchResult : context.result; - if (result?.error != null) { + if (result?.error != null && !result?.items?.length) { return { title: `${context.title} \u00a0\u2022\u00a0 Unable to Load Items`, placeholder: `Unable to load items (${ @@ -582,7 +590,7 @@ export class LaunchpadCommand extends QuickCommand { }; } - if (!result?.items.length) { + if (!result?.items?.length) { if (context.inSearch === 'mode') { return { title: `Search For Pull Request \u00a0\u2022\u00a0 ${context.title}`, @@ -616,6 +624,11 @@ export class LaunchpadCommand extends QuickCommand { } const items = getLaunchpadQuickPickItems(result.items, isFiltering); + + // Add error information item if there's an error but items were still loaded + const errorItem: DirectiveQuickPickItem | undefined = + result?.error != null ? this.createErrorQuickPickItem(result.error) : undefined; + const hasPicked = items.some(i => i.picked); if (context.inSearch === 'mode') { const offItem: ToggleSearchModeQuickPickItem = { @@ -630,7 +643,9 @@ export class LaunchpadCommand extends QuickCommand { return { title: `Search For Pull Request \u00a0\u2022\u00a0 ${context.title}`, placeholder: 'Enter a term to search for a pull request to act on', - items: isFiltering ? [...items, offItem] : [offItem, ...items], + items: isFiltering + ? [...(errorItem != null ? [errorItem] : []), ...items, offItem] + : [offItem, ...(errorItem != null ? [errorItem] : []), ...items], }; } @@ -646,10 +661,14 @@ export class LaunchpadCommand extends QuickCommand { title: context.title, placeholder: 'Choose a pull request or paste a pull request URL to act on', items: isFiltering - ? [...items, onItem] - : [onItem, ...getLaunchpadQuickPickItems(result.items, isFiltering)], + ? [...(errorItem != null ? [errorItem] : []), ...items, onItem] + : [ + onItem, + ...(errorItem != null ? [errorItem] : []), + ...getLaunchpadQuickPickItems(result.items, isFiltering), + ], }; - } + }; const updateItems = async ( quickpick: QuickPick< @@ -830,6 +849,16 @@ export class LaunchpadCommand extends QuickCommand { return; } + if (button === OpenLogsQuickInputButton) { + Logger.showOutputChannel(); + return; + } + + if (button === ConnectIntegrationButton) { + await this.container.integrations.manageCloudIntegrations({ source: 'launchpad' }); + return; + } + if (!item) return; switch (button) { @@ -1403,6 +1432,25 @@ export class LaunchpadCommand extends QuickCommand { this.source, ); } + + private createErrorQuickPickItem(error: Error): DirectiveQuickPickItem { + if (error instanceof AggregateError) { + const firstAuthError = error.errors.find(e => e instanceof AuthenticationError); + error = firstAuthError ?? error.errors[0] ?? error; + } + + const isAuthError = error instanceof AuthenticationError; + + return createDirectiveQuickPickItem(Directive.Noop, false, { + label: isAuthError ? '$(warning) Authentication Required' : '$(warning) Unable to fully load items', + detail: isAuthError + ? `${String(error)} — Click to reconnect your integration` + : error.name === 'HttpError' && 'status' in error && typeof error.status === 'number' + ? `${error.status}: ${String(error)}` + : String(error), + buttons: isAuthError ? [ConnectIntegrationButton, OpenLogsQuickInputButton] : [OpenLogsQuickInputButton], + }); + } } function getLaunchpadItemInformationRows( @@ -1657,10 +1705,10 @@ function updateTelemetryContext(context: Context) { if (context.telemetryContext == null) return; let updatedContext: NonNullable<(typeof context)['telemetryContext']>; - if (context.result.error != null) { + if (context.result.error != null || !context.result.items) { updatedContext = { ...context.telemetryContext, - 'items.error': String(context.result.error), + 'items.error': String(context.result.error ?? 'items not loaded'), }; } else { const grouped = countLaunchpadItemGroups(context.result.items); diff --git a/src/plus/launchpad/launchpadProvider.ts b/src/plus/launchpad/launchpadProvider.ts index 11cf6af7f2e4e..c3d29e204c53f 100644 --- a/src/plus/launchpad/launchpadProvider.ts +++ b/src/plus/launchpad/launchpadProvider.ts @@ -149,7 +149,7 @@ export type LaunchpadCategorizedResult = | { items: LaunchpadItem[]; timings?: LaunchpadCategorizedTimings; - error?: never; + error?: Error; } | { error: Error; @@ -221,11 +221,6 @@ export class LaunchpadProvider implements Disposable { } const prs = getSettledValue(prsResult)?.value; - if (prs?.error != null) { - Logger.error(prs.error, scope, 'Failed to get pull requests'); - throw prs.error; - } - const subscription = getSettledValue(subscriptionResult); let suggestionCounts; @@ -252,7 +247,7 @@ export class LaunchpadProvider implements Disposable { search, connectedIntegrations, ); - const result: { readonly value: PullRequest[]; duration: number } = { + const result: { readonly value: PullRequest[]; duration: number; error?: Error } = { value: [], duration: 0, }; @@ -690,7 +685,7 @@ export class LaunchpadProvider implements Disposable { isSearching ? typeof options.search === 'string' ? this.getSearchedPullRequests(options.search, cancellation) - : { prs: { value: options.search, duration: 0 }, suggestionCounts: undefined } + : { prs: { value: options.search, duration: 0, error: undefined }, suggestionCounts: undefined } : this.getPullRequestsWithSuggestionCounts({ force: options?.force, cancellation: cancellation }), ]); @@ -719,6 +714,7 @@ export class LaunchpadProvider implements Disposable { codeSuggestionCounts: prsWithSuggestionCounts?.suggestionCounts?.duration, enrichedItems: enrichedItems?.duration, }, + error: prsWithSuggestionCounts?.prs?.error, }; return result; } @@ -848,6 +844,7 @@ export class LaunchpadProvider implements Disposable { codeSuggestionCounts: prsWithSuggestionCounts?.suggestionCounts?.duration, enrichedItems: enrichedItems?.duration, }, + error: prsWithSuggestionCounts?.prs?.error, }; return result; } finally { diff --git a/src/webviews/home/homeWebview.ts b/src/webviews/home/homeWebview.ts index 25601205d14e4..ab726ce03125a 100644 --- a/src/webviews/home/homeWebview.ts +++ b/src/webviews/home/homeWebview.ts @@ -1942,13 +1942,13 @@ async function getLaunchpadItemInfo( ): Promise { launchpadPromise ??= container.launchpad.getCategorizedItems(); let result = await launchpadPromise; - if (result.error != null) return undefined; + if (result.error != null || !result.items) return undefined; let lpi = result.items.find(i => i.url === pr.url); if (lpi == null) { // result = await container.launchpad.getCategorizedItems({ search: pr.url }); result = await container.launchpad.getCategorizedItems({ search: [pr] }); - if (result.error != null) return undefined; + if (result.error != null || !result.items) return undefined; lpi = result.items.find(i => i.url === pr.url); }