fix(plugin): decouple state update from the LS

dankeboy36 · dankeboy36 · commit bd623dbdef7b · 2025-02-25T15:26:04.000+01:00
To enhance the reliability of Arduino IDE extensions, the update process for `ArduinoState` has been modified to ensure independence from the language server's availability. This change addresses issues caused by `compileSummary` being `undefined` due to potential startup failures of the Arduino Language Server, as noted in dankeboy36/esp-exception-decoder#28 (comment). The `compile` command now resolves with a `CompileSummary` rather than `void`, facilitating a more reliable way for extensions to access necessary data. Furthermore, the command has been adjusted to allow resolution with `undefined` when the compiled data is partial. By transitioning to direct usage of the resolved compile value for state updates, the reliance on executed commands for extensions is eliminated. This update also moves the VSIX command execution to the frontend without altering existing IDE behavior. Closes #2642 Signed-off-by: dankeboy36 <dankeboy36@gmail.com>
diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts
@@ -131,7 +131,10 @@ import { OpenSketch } from './contributions/open-sketch';
 import { Close } from './contributions/close';
 import { SaveAsSketch } from './contributions/save-as-sketch';
 import { SaveSketch } from './contributions/save-sketch';
-import { VerifySketch } from './contributions/verify-sketch';
+import {
+  CompileSummaryProvider,
+  VerifySketch,
+} from './contributions/verify-sketch';
 import { UploadSketch } from './contributions/upload-sketch';
 import { CommonFrontendContribution } from './theia/core/common-frontend-contribution';
 import { EditContributions } from './contributions/edit-contributions';
@@ -788,6 +791,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   Contribution.configure(bind, BoardsDataMenuUpdater);
   Contribution.configure(bind, AutoSelectProgrammer);
 
+  bind(CompileSummaryProvider).toService(VerifySketch);
+
   bindContributionProvider(bind, StartupTaskProvider);
   bind(StartupTaskProvider).toService(BoardsServiceProvider); // to reuse the boards config in another window
 
diff --git a/arduino-ide-extension/src/browser/contributions/update-arduino-state.ts b/arduino-ide-extension/src/browser/contributions/update-arduino-state.ts
@@ -1,13 +1,11 @@
 import { DisposableCollection } from '@theia/core/lib/common/disposable';
 import URI from '@theia/core/lib/common/uri';
 import { inject, injectable } from '@theia/core/shared/inversify';
-import { HostedPluginSupport } from '../hosted/hosted-plugin-support';
 import type { ArduinoState } from 'vscode-arduino-api';
 import {
+  BoardsConfig,
   BoardsService,
   CompileSummary,
-  isCompileSummary,
-  BoardsConfig,
   PortIdentifier,
   resolveDetectedPort,
 } from '../../common/protocol';
@@ -18,8 +16,10 @@ import {
 } from '../../common/protocol/arduino-context-mapper';
 import { BoardsDataStore } from '../boards/boards-data-store';
 import { BoardsServiceProvider } from '../boards/boards-service-provider';
+import { HostedPluginSupport } from '../hosted/hosted-plugin-support';
 import { CurrentSketch } from '../sketches-service-client-impl';
 import { SketchContribution } from './contribution';
+import { CompileSummaryProvider } from './verify-sketch';
 
 /**
  * (non-API) exported for tests
@@ -43,6 +43,8 @@ export class UpdateArduinoState extends SketchContribution {
   private readonly boardsDataStore: BoardsDataStore;
   @inject(HostedPluginSupport)
   private readonly hostedPluginSupport: HostedPluginSupport;
+  @inject(CompileSummaryProvider)
+  private readonly compileSummaryProvider: CompileSummaryProvider;
 
   private readonly toDispose = new DisposableCollection();
 
@@ -60,14 +62,9 @@ export class UpdateArduinoState extends SketchContribution {
       this.configService.onDidChangeSketchDirUri((userDirUri) =>
         this.updateUserDirPath(userDirUri)
       ),
-      this.commandService.onDidExecuteCommand(({ commandId, args }) => {
-        if (
-          commandId === 'arduino.languageserver.notifyBuildDidComplete' &&
-          isCompileSummary(args[0])
-        ) {
-          this.updateCompileSummary(args[0]);
-        }
-      }),
+      this.compileSummaryProvider.onDidChangeCompileSummary(() =>
+        this.maybeUpdateCompileSummary()
+      ),
       this.boardsDataStore.onDidChange((event) => {
         const selectedFqbn =
           this.boardsServiceProvider.boardsConfig.selectedBoard?.fqbn;
@@ -88,12 +85,20 @@ export class UpdateArduinoState extends SketchContribution {
     this.updateSketchPath(this.sketchServiceClient.tryGetCurrentSketch());
     this.updateUserDirPath(this.configService.tryGetSketchDirUri());
     this.updateDataDirPath(this.configService.tryGetDataDirUri());
+    this.maybeUpdateCompileSummary();
   }
 
   onStop(): void {
     this.toDispose.dispose();
   }
 
+  private maybeUpdateCompileSummary() {
+    const { compileSummary } = this.compileSummaryProvider;
+    if (compileSummary) {
+      this.updateCompileSummary(compileSummary);
+    }
+  }
+
   private async updateSketchPath(
     sketch: CurrentSketch | undefined
   ): Promise<void> {
diff --git a/arduino-ide-extension/src/browser/contributions/verify-sketch.ts b/arduino-ide-extension/src/browser/contributions/verify-sketch.ts
@@ -1,7 +1,7 @@
-import { Emitter } from '@theia/core/lib/common/event';
+import { Emitter, Event } from '@theia/core/lib/common/event';
 import { nls } from '@theia/core/lib/common/nls';
 import { inject, injectable } from '@theia/core/shared/inversify';
-import type { CoreService } from '../../common/protocol';
+import type { CompileSummary, CoreService } from '../../common/protocol';
 import { ArduinoMenus } from '../menu/arduino-menus';
 import { CurrentSketch } from '../sketches-service-client-impl';
 import { ArduinoToolbar } from '../toolbar/arduino-toolbar';
@@ -15,6 +15,12 @@ import {
 } from './contribution';
 import { CoreErrorHandler } from './core-error-handler';
 
+export const CompileSummaryProvider = Symbol('CompileSummaryProvider');
+export interface CompileSummaryProvider {
+  readonly compileSummary: CompileSummary | undefined;
+  readonly onDidChangeCompileSummary: Event<void>;
+}
+
 export type VerifySketchMode =
   /**
    * When the user explicitly triggers the verify command from the primary UI: menu, toolbar, or keybinding. The UI shows the output, updates the toolbar items state, etc.
@@ -46,13 +52,18 @@ export interface VerifySketchParams {
 type VerifyProgress = 'idle' | VerifySketchMode;
 
 @injectable()
-export class VerifySketch extends CoreServiceContribution {
+export class VerifySketch
+  extends CoreServiceContribution
+  implements CompileSummaryProvider
+{
   @inject(CoreErrorHandler)
   private readonly coreErrorHandler: CoreErrorHandler;
 
   private readonly onDidChangeEmitter = new Emitter<void>();
   private readonly onDidChange = this.onDidChangeEmitter.event;
+  private readonly onDidChangeCompileSummaryEmitter = new Emitter<void>();
   private verifyProgress: VerifyProgress = 'idle';
+  private _compileSummary: CompileSummary | undefined;
 
   override registerCommands(registry: CommandRegistry): void {
     registry.registerCommand(VerifySketch.Commands.VERIFY_SKETCH, {
@@ -117,6 +128,14 @@ export class VerifySketch extends CoreServiceContribution {
     super.handleError(error);
   }
 
+  get compileSummary(): CompileSummary | undefined {
+    return this._compileSummary;
+  }
+
+  get onDidChangeCompileSummary(): Event<void> {
+    return this.onDidChangeCompileSummaryEmitter.event;
+  }
+
   private async verifySketch(
     params?: VerifySketchParams
   ): Promise<CoreService.Options.Compile | undefined> {
@@ -141,7 +160,7 @@ export class VerifySketch extends CoreServiceContribution {
         return options;
       }
 
-      await this.doWithProgress({
+      const compileSummary = await this.doWithProgress({
         progressText: nls.localize(
           'arduino/sketch/compile',
           'Compiling sketch...'
@@ -160,6 +179,13 @@ export class VerifySketch extends CoreServiceContribution {
         nls.localize('arduino/sketch/doneCompiling', 'Done compiling.'),
         { timeout: 3000 }
       );
+
+      this._compileSummary = compileSummary;
+      this.onDidChangeCompileSummaryEmitter.fire();
+      if (this._compileSummary) {
+        this.fireBuildDidComplete(this._compileSummary);
+      }
+
       // Returns with the used options for the compilation
       // so that follow-up tasks (such as upload) can reuse the compiled code.
       // Note that the `fqbn` is already decorated with the board settings, if any.
@@ -201,6 +227,28 @@ export class VerifySketch extends CoreServiceContribution {
       compilerWarnings,
     };
   }
+
+  // Execute the a command contributed by the Arduino Tools VSIX to send the `ino/buildDidComplete` notification to the language server
+  private fireBuildDidComplete(compileSummary: CompileSummary): void {
+    const params = {
+      ...compileSummary,
+    };
+    console.info(
+      `Executing 'arduino.languageserver.notifyBuildDidComplete' with ${JSON.stringify(
+        params.buildOutputUri
+      )}`
+    );
+    this.commandService
+      .executeCommand('arduino.languageserver.notifyBuildDidComplete', params)
+      .catch((err) =>
+        console.error(
+          `Unexpected error when firing event on build did complete. ${JSON.stringify(
+            params.buildOutputUri
+          )}`,
+          err
+        )
+      );
+  }
 }
 
 export namespace VerifySketch {
diff --git a/arduino-ide-extension/src/common/protocol/core-service.ts b/arduino-ide-extension/src/common/protocol/core-service.ts
@@ -171,7 +171,7 @@ export interface CoreService {
   compile(
     options: CoreService.Options.Compile,
     cancellationToken?: CancellationToken
-  ): Promise<void>;
+  ): Promise<CompileSummary | undefined>;
   upload(
     options: CoreService.Options.Upload,
     cancellationToken?: CancellationToken
diff --git a/arduino-ide-extension/src/node/core-service-impl.ts b/arduino-ide-extension/src/node/core-service-impl.ts
@@ -1,7 +1,6 @@
 import { type ClientReadableStream } from '@grpc/grpc-js';
 import { ApplicationError } from '@theia/core/lib/common/application-error';
 import type { CancellationToken } from '@theia/core/lib/common/cancellation';
-import { CommandService } from '@theia/core/lib/common/command';
 import {
   Disposable,
   DisposableCollection,
@@ -69,15 +68,13 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
   private readonly responseService: ResponseService;
   @inject(MonitorManager)
   private readonly monitorManager: MonitorManager;
-  @inject(CommandService)
-  private readonly commandService: CommandService;
   @inject(BoardDiscovery)
   private readonly boardDiscovery: BoardDiscovery;
 
   async compile(
     options: CoreService.Options.Compile,
     cancellationToken?: CancellationToken
-  ): Promise<void> {
+  ): Promise<CompileSummary | undefined> {
     const coreClient = await this.coreClient;
     const { client, instance } = coreClient;
     const request = this.compileRequest(options, instance);
@@ -91,7 +88,7 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
     );
     const toDisposeOnFinally = new DisposableCollection(handler);
 
-    return new Promise<void>((resolve, reject) => {
+    return new Promise<CompileSummary | undefined>((resolve, reject) => {
       let hasRetried = false;
 
       const handleUnexpectedError = (error: Error) => {
@@ -164,50 +161,24 @@ export class CoreServiceImpl extends CoreClientAware implements CoreService {
         call
           .on('data', handler.onData)
           .on('error', handleError)
-          .on('end', resolve);
+          .on('end', () => {
+            if (isCompileSummary(compileSummary)) {
+              resolve(compileSummary);
+            } else {
+              console.error(
+                `Have not received the full compile summary from the CLI while running the compilation. ${JSON.stringify(
+                  compileSummary
+                )}`
+              );
+              resolve(undefined);
+            }
+          });
       };
 
       startCompileStream();
-    }).finally(() => {
-      toDisposeOnFinally.dispose();
-      if (!isCompileSummary(compileSummary)) {
-        if (cancellationToken && cancellationToken.isCancellationRequested) {
-          // NOOP
-          return;
-        }
-        console.error(
-          `Have not received the full compile summary from the CLI while running the compilation. ${JSON.stringify(
-            compileSummary
-          )}`
-        );
-      } else {
-        this.fireBuildDidComplete(compileSummary);
-      }
     });
   }
 
-  // This executes on the frontend, the VS Code extension receives it, and sends an `ino/buildDidComplete` notification to the language server.
-  private fireBuildDidComplete(compileSummary: CompileSummary): void {
-    const params = {
-      ...compileSummary,
-    };
-    console.info(
-      `Executing 'arduino.languageserver.notifyBuildDidComplete' with ${JSON.stringify(
-        params.buildOutputUri
-      )}`
-    );
-    this.commandService
-      .executeCommand('arduino.languageserver.notifyBuildDidComplete', params)
-      .catch((err) =>
-        console.error(
-          `Unexpected error when firing event on build did complete. ${JSON.stringify(
-            params.buildOutputUri
-          )}`,
-          err
-        )
-      );
-  }
-
   private compileRequest(
     options: CoreService.Options.Compile & {
       exportBinaries?: boolean;
diff --git a/arduino-ide-extension/src/test/browser/update-arduino-state.test.ts b/arduino-ide-extension/src/test/browser/update-arduino-state.test.ts
@@ -31,6 +31,7 @@ import {
   UpdateArduinoState,
   UpdateStateParams,
 } from '../../browser/contributions/update-arduino-state';
+import { CompileSummaryProvider } from '../../browser/contributions/verify-sketch';
 import { NotificationCenter } from '../../browser/notification-center';
 import {
   CurrentSketch,
@@ -61,10 +62,12 @@ describe('update-arduino-state', function () {
   let currentSketchMock: CurrentSketch | undefined;
   let sketchDirUriMock: URI | undefined;
   let dataDirUriMock: URI | undefined;
+  let compileSummary: CompileSummary | undefined;
   let onCurrentSketchDidChangeEmitter: Emitter<CurrentSketch>;
   let onDataDirDidChangeEmitter: Emitter<URI | undefined>;
   let onSketchDirDidChangeEmitter: Emitter<URI | undefined>;
   let onDataStoreDidChangeEmitter: Emitter<BoardsDataStoreChangeEvent>;
+  let compileSummaryDidChangeEmitter: Emitter<void>;
 
   beforeEach(async () => {
     toDisposeAfterEach = new DisposableCollection();
@@ -76,15 +79,18 @@ describe('update-arduino-state', function () {
     currentSketchMock = undefined;
     sketchDirUriMock = undefined;
     dataDirUriMock = undefined;
+    compileSummary = undefined;
     onCurrentSketchDidChangeEmitter = new Emitter();
     onDataDirDidChangeEmitter = new Emitter();
     onSketchDirDidChangeEmitter = new Emitter();
     onDataStoreDidChangeEmitter = new Emitter();
+    compileSummaryDidChangeEmitter = new Emitter();
     toDisposeAfterEach.pushAll([
       onCurrentSketchDidChangeEmitter,
       onDataDirDidChangeEmitter,
       onSketchDirDidChangeEmitter,
       onDataStoreDidChangeEmitter,
+      compileSummaryDidChangeEmitter,
     ]);
 
     const container = createContainer();
@@ -418,10 +424,8 @@ describe('update-arduino-state', function () {
       buildPlatform: undefined,
       buildOutputUri: 'file:///path/to/build',
     };
-    await commandRegistry.executeCommand(
-      'arduino.languageserver.notifyBuildDidComplete',
-      summary
-    );
+    compileSummary = summary;
+    compileSummaryDidChangeEmitter.fire();
     await wait(50);
 
     const params = stateUpdateParams.filter(
@@ -585,6 +589,12 @@ describe('update-arduino-state', function () {
       new ContainerModule((bind, unbind, isBound, rebind) => {
         bindSketchesContribution(bind, unbind, isBound, rebind);
         bind(UpdateArduinoState).toSelf().inSingletonScope();
+        bind(CompileSummaryProvider).toConstantValue(<CompileSummaryProvider>{
+          get compileSummary(): CompileSummary | undefined {
+            return compileSummary;
+          },
+          onDidChangeCompileSummary: compileSummaryDidChangeEmitter.event,
+        });
         rebind(BoardsService).toConstantValue(<BoardsService>{
           getDetectedPorts() {
             return {};
diff --git a/arduino-ide-extension/src/test/node/core-service-impl.slow-test.ts b/arduino-ide-extension/src/test/node/core-service-impl.slow-test.ts
