diff --git a/client/vscode-lib/src/controller.ts b/client/vscode-lib/src/controller.ts index e9d63622..7fb28427 100644 --- a/client/vscode-lib/src/controller.ts +++ b/client/vscode-lib/src/controller.ts @@ -23,7 +23,6 @@ import { createCodeLensProvider } from './ui/editor/codeLens.js' import { createHoverProvider } from './ui/editor/hover.js' import { createShowFileItemsList } from './ui/fileItemsList.js' import { createStatusBarItem } from './ui/statusBarItem.js' -import { Cache, bestEffort } from './util/cache.js' import { ErrorReporterController, UserAction } from './util/errorReporter.js' import { importProvider } from './util/importHelpers.js' import { observeWorkspaceConfigurationChanges, toEventEmitter } from './util/observable.js' @@ -122,18 +121,15 @@ export function createController({ }) disposables.push(client) - const errorLog = (error: any) => { - console.error(error) - outputChannel.appendLine(error) - } - // errorReporter contains a lot of logic and state on how we notify and log // errors, as well as state around if we should turn off a feature (see // skipIfImplicitAction) - const errorReporter = new ErrorReporterController( - createErrorNotifier(outputChannel, extensionId, client), - errorLog, - ) + const errorReporter = new ErrorReporterController((error: any, providerUri: string | undefined) => { + const prefix = `OpenCtx error (from provider ${providerUri ?? 'unknown'}): ` + console.error(prefix, error) + const errorDetail = error.stack ? `${error} (stack trace follows)\n${error.stack}` : error + outputChannel.appendLine(`${prefix}${errorDetail}`) + }) disposables.push(errorReporter) // Note: We distingiush between an explicit user action and an implicit @@ -211,54 +207,3 @@ function ignoreDoc(params: AnnotationsParams): boolean { function makeRange(range: Range): vscode.Range { return new vscode.Range(range.start.line, range.start.character, range.end.line, range.end.character) } - -function createErrorNotifier( - outputChannel: vscode.OutputChannel, - extensionId: string, - client: Pick, -) { - // Fetching the name can be slow or fail. So we use a cache + timeout when - // getting the name of a provider. - const nameCache = new Cache({ ttlMs: 10 * 1000 }) - const getName = async (providerUri: string | undefined) => { - if (providerUri === undefined) { - return undefined - } - const fill = async () => { - const meta = await bestEffort(client.meta({}, { providerUri: providerUri }), { - defaultValue: [], - delay: 250, - }) - return meta.pop()?.name ?? providerUri - } - return await nameCache.getOrFill(providerUri, fill) - } - - const actionItems = [ - { - title: 'Show Logs', - do: () => { - outputChannel.show() - }, - }, - { - title: 'Open Settings', - do: () => { - vscode.commands.executeCommand('workbench.action.openSettings', { - query: `@ext:${extensionId} openctx.providers`, - }) - }, - }, - ] satisfies (vscode.MessageItem & { do: () => void })[] - - return async (providerUri: string | undefined, error: any) => { - const name = await getName(providerUri) - const message = name - ? `Error from OpenCtx provider "${name}": ${error}` - : `Error from OpenCtx: ${error}` - const action = await vscode.window.showErrorMessage(message, ...actionItems) - if (action) { - action.do() - } - } -} diff --git a/client/vscode-lib/src/util/cache.ts b/client/vscode-lib/src/util/cache.ts index 8ed41c11..592baedd 100644 --- a/client/vscode-lib/src/util/cache.ts +++ b/client/vscode-lib/src/util/cache.ts @@ -22,7 +22,10 @@ export class Cache { } } -/** resolves promise, but will return defaultValue if promise throws or takes longer than delay ms */ +/** + * Try to resolve {@link promise}, but return {@link opts.defaultValue} if it throws or takes longer than + * {@link opts.delay} ms. + */ export async function bestEffort( promise: Promise, opts: { diff --git a/client/vscode-lib/src/util/errorReporter.ts b/client/vscode-lib/src/util/errorReporter.ts index fbeb4f2a..5c94fbdd 100644 --- a/client/vscode-lib/src/util/errorReporter.ts +++ b/client/vscode-lib/src/util/errorReporter.ts @@ -4,8 +4,6 @@ import { Observable } from 'observable-fns' import type * as vscode from 'vscode' import { type ErrorWaiter, createErrorWaiter } from './errorWaiter.js' -const MIN_TIME_SINCE_LAST_ERROR = 1000 * 60 * 15 /* 15 min */ - /** * The users action affects our behaviour around reporting, so we explicitly * tell ErrorReporter the intent. @@ -29,12 +27,8 @@ export enum UserAction { */ export class ErrorReporterController implements vscode.Disposable { private errorWaiters = new Map() - private errorNotificationVisible = new Set() - constructor( - private showErrorNotification: (providerUri: string | undefined, error: any) => Thenable, - private errorLog: (error: any) => void, - ) {} + constructor(private errorLog: (error: any, providerUri: string | undefined) => void) {} /** * wraps providerMethod to ensure it reports errors to the user. @@ -120,7 +114,7 @@ export class ErrorReporterController implements vscode.Disposable { errorByUri.set(providerUri, errors) // Always log the error. - this.errorLog(error) + this.errorLog(error, providerUri) } // report takes the seen errors so far and decides if they should be @@ -137,21 +131,6 @@ export class ErrorReporterController implements vscode.Disposable { const errorWaiter = this.getErrorWaiter(providerUri) errorWaiter.gotError(hasError) - - // From here on out it is about notifying for an error - if (!hasError) { - continue - } - - // Show an error notification unless we've recently shown one - // (to avoid annoying the user). - const shouldNotify = - userAction === UserAction.Explicit || - errorWaiter.timeSinceLastError() > MIN_TIME_SINCE_LAST_ERROR - if (shouldNotify) { - const error = errors.length === 1 ? errors[0] : new AggregateError(errors) - this.maybeShowErrorNotification(providerUri, error) - } } // Clear out seen errors so that report may be called again. @@ -181,22 +160,6 @@ export class ErrorReporterController implements vscode.Disposable { return errorWaiter } - private maybeShowErrorNotification(providerUri: string, error: any) { - // We store the notification thenable so we can tell if the last - // notification for providerUri is still showing. - if (this.errorNotificationVisible.has(providerUri)) { - return - } - this.errorNotificationVisible.add(providerUri) - const onfinally = () => this.errorNotificationVisible.delete(providerUri) - - // If providerUri is the empty string communicate that via undefined - this.showErrorNotification(providerUri === '' ? undefined : providerUri, error).then( - onfinally, - onfinally, - ) - } - public dispose() { for (const errorWaiter of this.errorWaiters.values()) { errorWaiter.dispose()