error handling, messaging and refactoring. (#1333)

wangmingliang-ms · web-flow · commit 1790bd00784e · 2024-05-21T18:27:40.000+08:00
* error handling, messaging and refactoring.

* resolve comments.
diff --git a/src/copilot/Copilot.ts b/src/copilot/Copilot.ts
@@ -28,7 +28,8 @@ export default class Copilot {
         const messages = [...systemMessagesOrSamples];
         const _send = async (message: string): Promise<boolean> => {
             rounds++;
-            logger.info(`User: \n`, message);
+            logger.debug(`User: \n`, message);
+            logger.info(`User: ${message.split('\n')[0]}...`);
             messages.push(new LanguageModelChatMessage(LanguageModelChatMessageRole.User, message));
             logger.info('Copilot: thinking...');
 
@@ -47,7 +48,8 @@ export default class Copilot {
                 throw new Error(`Failed to send request to copilot: ${e}`);
             }
             messages.push(new LanguageModelChatMessage(LanguageModelChatMessageRole.Assistant, rawAnswer));
-            logger.info(`Copilot: \n`, rawAnswer);
+            logger.debug(`Copilot: \n`, rawAnswer);
+            logger.info(`Copilot: ${rawAnswer.split('\n')[0]}...`);
             answer += rawAnswer;
             return answer.trim().endsWith(this.endMark);
         };
diff --git a/src/copilot/inspect/DocumentRenderer.ts b/src/copilot/inspect/DocumentRenderer.ts
@@ -104,7 +104,11 @@ export class DocumentRenderer {
         if (settings.length === 0) {
             settings.push('diagnostics');
             settings.push('rulerhighlights');
-            settings.push(isCodeLensDisabled() ? 'guttericons' : 'codelenses');
+            const disabled = isCodeLensDisabled();
+            if (disabled) {
+                logger.warn('CodeLens is disabled, fallback to GutterIcons');
+            }
+            settings.push(disabled ? 'guttericons' : 'codelenses');
         }
         return settings;
     }
diff --git a/src/copilot/inspect/InspectionCopilot.ts b/src/copilot/inspect/InspectionCopilot.ts
@@ -3,7 +3,7 @@ import Copilot from "../Copilot";
 import { getClassesContainedInRange, getInnermostClassContainsRange, getIntersectionMethodsOfRange, getProjectJavaVersion, getUnionRange, logger } from "../utils";
 import { Inspection } from "./Inspection";
 import path from "path";
-import { TextDocument, SymbolKind, ProgressLocation, commands, Position, Range, Selection, window, LanguageModelChatMessage, LanguageModelChatMessageRole } from "vscode";
+import { TextDocument, SymbolKind, ProgressLocation, commands, Position, Range, Selection, window, LanguageModelChatMessage } from "vscode";
 import { COMMAND_FIX_INSPECTION } from "./commands";
 import InspectionCache from "./InspectionCache";
 import { SymbolNode } from "./SymbolNode";
@@ -162,21 +162,21 @@ export default class InspectionCopilot extends Copilot {
             const symbolKind = SymbolKind[symbols[0].kind].toLowerCase();
             const inspections = await window.withProgress({
                 location: ProgressLocation.Notification,
-                title: `Inspecting ${symbolKind} ${symbolName}... of \"${path.basename(document.fileName)}\"`,
+                title: `Inspecting ${symbolKind} ${symbolName}... of "${path.basename(document.fileName)}"`,
                 cancellable: false
             }, (_progress) => {
                 return this.doInspectRange(document, expandedRange);
             });
 
             // show message based on the number of inspections
             if (inspections.length < 1) {
-                void window.showInformationMessage(`Inspected ${symbolKind} ${symbolName}... of \"${path.basename(document.fileName)}\" and got 0 suggestions.`);
+                void window.showInformationMessage(`Inspected ${symbolKind} ${symbolName}... of "${path.basename(document.fileName)}" and got 0 suggestions.`);
             } else if (inspections.length == 1) {
                 // apply the only suggestion automatically
                 void commands.executeCommand(COMMAND_FIX_INSPECTION, inspections[0].problem, inspections[0].solution, 'auto');
             } else {
                 // show message to go to the first suggestion
-                void window.showInformationMessage(`Inspected ${symbolKind} ${symbolName}... of \"${path.basename(document.fileName)}\" and got ${inspections.length} suggestions.`, "Go to").then(selection => {
+                void window.showInformationMessage(`Inspected ${symbolKind} ${symbolName}... of "${path.basename(document.fileName)}" and got ${inspections.length} suggestions.`, "Go to").then(selection => {
                     selection === "Go to" && void Inspection.revealFirstLineOfInspection(inspections[0]);
                 });
             }
@@ -220,10 +220,8 @@ export default class InspectionCopilot extends Copilot {
         const adjustedRange = new Range(new Position(range.start.line, 0), new Position(range.end.line, document.lineAt(range.end.line).text.length));
         const content: string = document.getText(adjustedRange);
         const startLine = range.start.line;
-        const javaVersion = await getProjectJavaVersion(document);
-        const inspections = await this.inspectCode(content, {
-            javaVersion
-        });
+        const projectContext = await this.collectProjectContext(document);
+        const inspections = await this.inspectCode(content, projectContext);
         inspections.forEach(s => {
             s.document = document;
             // real line index to the start of the document
@@ -356,6 +354,11 @@ export default class InspectionCopilot extends Copilot {
         }
         return codeLines;
     }
+
+    async collectProjectContext(document: TextDocument): Promise<ProjectContext> {
+        const javaVersion = await getProjectJavaVersion(document);
+        return { javaVersion };
+    }
 }
 
 export interface ProjectContext {
diff --git a/src/copilot/inspect/commands.ts b/src/copilot/inspect/commands.ts
@@ -1,11 +1,12 @@
-import { TextDocument, Range, Selection, commands } from "vscode";
+import { TextDocument, Range, Selection, commands, window } from "vscode";
 import { instrumentOperationAsVsCodeCommand, sendInfo } from "vscode-extension-telemetry-wrapper";
 import InspectionCopilot from "./InspectionCopilot";
 import { Inspection, InspectionProblem } from "./Inspection";
-import { uncapitalize } from "../utils";
+import { logger, uncapitalize } from "../utils";
 import { SymbolNode } from "./SymbolNode";
 import { DocumentRenderer } from "./DocumentRenderer";
 import InspectionCache from "./InspectionCache";
+import path from "path";
 
 export const COMMAND_INSPECT_CLASS = 'java.copilot.inspect.class';
 export const COMMAND_INSPECT_RANGE = 'java.copilot.inspect.range';
@@ -14,12 +15,24 @@ export const COMMAND_IGNORE_INSPECTIONS = 'java.copilot.inspection.ignore';
 
 export function registerCommands(copilot: InspectionCopilot, renderer: DocumentRenderer) {
     instrumentOperationAsVsCodeCommand(COMMAND_INSPECT_CLASS, async (document: TextDocument, clazz: SymbolNode) => {
-        await copilot.inspectClass(document, clazz);
+        try {
+            await copilot.inspectClass(document, clazz);
+        } catch (e) {
+            window.showErrorMessage(`Failed to inspect class "${clazz.symbol.name}" with error ${e}.`);
+            logger.error(`Failed to inspect class "${clazz.symbol.name}".`, e);
+            throw e;
+        }
         renderer.rerender(document);
     });
 
     instrumentOperationAsVsCodeCommand(COMMAND_INSPECT_RANGE, async (document: TextDocument, range: Range | Selection) => {
-        await copilot.inspectRange(document, range);
+        try {
+            await copilot.inspectRange(document, range);
+        } catch (e) {
+            window.showErrorMessage(`Failed to inspect range of "${path.basename(document.fileName)}" with error ${e}.`);
+            logger.error(`Failed to inspect range of "${path.basename(document.fileName)}".`, e);
+            throw e;
+        }
         renderer.rerender(document);
     });
 
diff --git a/src/copilot/inspect/index.ts b/src/copilot/inspect/index.ts
@@ -11,6 +11,7 @@ export const DEPENDENT_EXTENSIONS = ['github.copilot-chat', 'redhat.java'];
 
 export async function activateCopilotInspection(context: ExtensionContext): Promise<void> {
     logger.info('Waiting for dependent extensions to be ready...');
+    await waitUntilExtensionsInstalled(DEPENDENT_EXTENSIONS);
     await waitUntilExtensionsActivated(DEPENDENT_EXTENSIONS);
     logger.info('Activating Java Copilot features...');
     doActivate(context);
@@ -49,25 +50,41 @@ async function rewrite(document: TextDocument, range: Range | Selection, _contex
 export async function waitUntilExtensionsActivated(extensionIds: string[], interval: number = 1500) {
     const start = Date.now();
     return new Promise<void>((resolve) => {
-        if (extensionIds.every(id => extensions.getExtension(id)?.isActive)) {
+        const notActivatedExtensionIds = extensionIds.filter(id => !extensions.getExtension(id)?.isActive);
+        if (notActivatedExtensionIds.length == 0) {
             logger.info(`All dependent extensions [${extensionIds.join(', ')}] are activated.`);
             return resolve();
         }
-        const notInstalledExtensionIds = extensionIds.filter(id => !extensions.getExtension(id));
-        if (notInstalledExtensionIds.length > 0) {
-            sendInfo('java.copilot.inspection.dependentExtensions.notInstalledExtensions', { extensionIds: `[${notInstalledExtensionIds.join(',')}]` });
-            logger.info(`Dependent extensions [${notInstalledExtensionIds.join(', ')}] are not installed, setting checking interval to 10s.`);
-        } else {
-            logger.info(`All dependent extensions are installed, but some are not activated, keep checking interval ${interval}ms.`);
-        }
-        interval = notInstalledExtensionIds.length > 0 ? 10000 : interval;
+        logger.info(`Dependent extensions [${notActivatedExtensionIds.join(', ')}] are not activated, waiting...`);
         const id = setInterval(() => {
             if (extensionIds.every(id => extensions.getExtension(id)?.isActive)) {
                 clearInterval(id);
-                sendInfo('java.copilot.inspection.dependentExtensions.waited', { time: Date.now() - start });
-                logger.info(`waited for ${Date.now() - start}ms for all dependent extensions [${extensionIds.join(', ')}] to be installed/activated.`);
+                sendInfo('java.copilot.inspection.dependentExtensions.waitActivated', { time: Date.now() - start });
+                logger.info(`waited for ${Date.now() - start}ms for all dependent extensions [${extensionIds.join(', ')}] to be activated.`);
                 resolve();
             }
         }, interval);
     });
+}
+
+export async function waitUntilExtensionsInstalled(extensionIds: string[]) {
+    const start = Date.now();
+    return new Promise<void>((resolve) => {
+        const notInstalledExtensionIds = extensionIds.filter(id => !extensions.getExtension(id));
+        if (notInstalledExtensionIds.length == 0) {
+            logger.info(`All dependent extensions [${extensionIds.join(', ')}] are installed.`);
+            return resolve();
+        }
+        sendInfo('java.copilot.inspection.dependentExtensions.notInstalledExtensions', { extensionIds: `[${notInstalledExtensionIds.join(',')}]` });
+        logger.info(`Dependent extensions [${notInstalledExtensionIds.join(', ')}] are not installed, waiting...`);
+
+        const disposable = extensions.onDidChange(() => {
+            if (extensionIds.every(id => extensions.getExtension(id))) {
+                disposable.dispose();
+                sendInfo('java.copilot.inspection.dependentExtensions.waitInstalled', { time: Date.now() - start });
+                logger.info(`waited for ${Date.now() - start}ms for all dependent extensions [${extensionIds.join(', ')}] to be installed.`);
+                resolve();
+            }
+        });
+    });
 }
diff --git a/src/copilot/utils.ts b/src/copilot/utils.ts
@@ -1,5 +1,6 @@
-import { LogOutputChannel, SymbolKind, TextDocument, commands, window, Range, Selection, workspace, DocumentSymbol } from "vscode";
+import { LogOutputChannel, SymbolKind, TextDocument, commands, window, Range, Selection, workspace, DocumentSymbol, ProgressLocation, version } from "vscode";
 import { SymbolNode } from "./inspect/SymbolNode";
+import { SemVer } from "semver";
 
 export const CLASS_KINDS: SymbolKind[] = [SymbolKind.Class, SymbolKind.Interface, SymbolKind.Enum];
 export const METHOD_KINDS: SymbolKind[] = [SymbolKind.Method, SymbolKind.Constructor];
@@ -85,6 +86,32 @@ export function isCodeLensDisabled(): boolean {
 export async function getProjectJavaVersion(document: TextDocument): Promise<number> {
     const uri = document.uri.toString();
     const key = "org.eclipse.jdt.core.compiler.source";
-    const settings: { [key]: string } = await commands.executeCommand("java.project.getSettings", uri, [key]);
-    return parseInt(settings[key]) || 17;
+    try {
+        const settings: { [key]: string } = await retryOnFailure(async () => {
+            return await commands.executeCommand("java.project.getSettings", uri, [key]);
+        });
+        return parseInt(settings[key]) || 17;
+    } catch (e) {
+        throw new Error(`Failed to get Java version, please check if the project is loaded normally: ${e}`);
+    }
 }
+
+export async function retryOnFailure<T>(task: () => Promise<T>, timeout: number = 15000, retryInterval: number = 3000): Promise<T> {
+    const startTime = Date.now();
+
+    while (true) {
+        try {
+            return await task();
+        } catch (error) {
+            if (Date.now() - startTime >= timeout) {
+                throw error;
+            } else {
+                await new Promise(resolve => setTimeout(resolve, retryInterval));
+            }
+        }
+    }
+}
+
+export function isLlmApiReady(): boolean {
+    return new SemVer(version).compare(new SemVer("1.90.0-insider")) >= 0;
+}
diff --git a/src/extension.ts b/src/extension.ts
@@ -24,6 +24,7 @@ import { scheduleAction } from "./utils/scheduler";
 import { showWelcomeWebview, WelcomeViewSerializer } from "./welcome";
 import { activateCopilotInspection } from "./copilot/inspect";
 import { ProjectSettingsViewSerializer } from "./project-settings/projectSettingsView";
+import { isLlmApiReady } from "./copilot/utils";
 
 let cleanJavaWorkspaceIndicator: string;
 let activatedTimestamp: number;
@@ -47,7 +48,7 @@ async function initializeExtension(_operationId: string, context: vscode.Extensi
   initDaemon(context);
 
   activatedTimestamp = performance.now();
-  if(context.storageUri) {
+  if (context.storageUri) {
     const javaWorkspaceStoragePath = path.join(context.storageUri.fsPath, "..", "redhat.java");
     cleanJavaWorkspaceIndicator = path.join(javaWorkspaceStoragePath, "jdt_ws", ".cleanWorkspace");
   }
@@ -83,7 +84,9 @@ async function initializeExtension(_operationId: string, context: vscode.Extensi
     });
   }
 
-  activateCopilotInspection(context);
+  if (isLlmApiReady()) {
+    activateCopilotInspection(context);
+  }
 }
 
 async function presentFirstView(context: vscode.ExtensionContext) {

Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,11 @@ export class DocumentRenderer {`
`104`	`104`	`if (settings.length === 0) {`
`105`	`105`	`settings.push('diagnostics');`
`106`	`106`	`settings.push('rulerhighlights');`
`107`		`- settings.push(isCodeLensDisabled() ? 'guttericons' : 'codelenses');`
	`107`	`+ const disabled = isCodeLensDisabled();`
	`108`	`+ if (disabled) {`
	`109`	`+ logger.warn('CodeLens is disabled, fallback to GutterIcons');`
	`110`	`+ }`
	`111`	`+ settings.push(disabled ? 'guttericons' : 'codelenses');`
`108`	`112`	`}`
`109`	`113`	`return settings;`
`110`	`114`	`}`