fix(plugin): decouple state update from the LS (#2643)

dankeboy36 · giacomocusinato · web-flow · commit 5ec191500028 · 2025-03-10T15:20:22.000+07:00
* fix(plugin): decouple state update from the LS 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> * fix: install missing libx11-dev and libxkbfile-dev Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix: pick better GH step name Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix: install the required dependencies on Linux Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix(revert): do not manually install deps on Linux Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * chore: pin `ubuntu-22.04` for linux actions * fix: restore accidentally removed dispose on finally Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix(test): align mock naming 💄 Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix: let the ino contribution notify the LS + event emitter dispatches the new state. Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * fix(test): emit the new compiler summary state Signed-off-by: dankeboy36 <dankeboy36@gmail.com> * chore(revert): unpin linux version, use latest revert of b11bde1 Signed-off-by: dankeboy36 <dankeboy36@gmail.com> --------- Signed-off-by: dankeboy36 <dankeboy36@gmail.com> Co-authored-by: Giacomo Cusinato <7659518+giacomocusinato@users.noreply.github.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/ino-language.ts b/arduino-ide-extension/src/browser/contributions/ino-language.ts
@@ -8,6 +8,7 @@ import {
   ArduinoDaemon,
   BoardIdentifier,
   BoardsService,
+  CompileSummary,
   ExecutableService,
   isBoardIdentifierChangeEvent,
   sanitizeFqbn,
@@ -23,6 +24,7 @@ import { HostedPluginEvents } from '../hosted/hosted-plugin-events';
 import { NotificationCenter } from '../notification-center';
 import { CurrentSketch } from '../sketches-service-client-impl';
 import { SketchContribution, URI } from './contribution';
+import { CompileSummaryProvider } from './verify-sketch';
 
 interface DaemonAddress {
   /**
@@ -107,6 +109,8 @@ export class InoLanguage extends SketchContribution {
   private readonly notificationCenter: NotificationCenter;
   @inject(BoardsDataStore)
   private readonly boardDataStore: BoardsDataStore;
+  @inject(CompileSummaryProvider)
+  private readonly compileSummaryProvider: CompileSummaryProvider;
 
   private readonly toDispose = new DisposableCollection();
   private readonly languageServerStartMutex = new Mutex();
@@ -173,6 +177,13 @@ export class InoLanguage extends SketchContribution {
           }
         }
       }),
+      this.compileSummaryProvider.onDidChangeCompileSummary(
+        (compileSummary) => {
+          if (compileSummary) {
+            this.fireBuildDidComplete(compileSummary);
+          }
+        }
+      ),
     ]);
     Promise.all([
       this.boardsServiceProvider.ready,
@@ -317,4 +328,32 @@ export class InoLanguage extends SketchContribution {
       params
     );
   }
+
+  // Execute the a command contributed by the Arduino Tools VSIX to send the `ino/buildDidComplete` notification to the language server
+  private async fireBuildDidComplete(
+    compileSummary: CompileSummary
+  ): Promise<void> {
+    const params = {
+      ...compileSummary,
+    };
+    console.info(
+      `Executing 'arduino.languageserver.notifyBuildDidComplete' with ${JSON.stringify(
+        params.buildOutputUri
+      )}`
+    );
+
+    try {
+      await 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
+      );
+    }
+  }
 }
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,13 @@ 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(
+        (compilerSummary) => {
+          if (compilerSummary) {
+            this.updateCompileSummary(compilerSummary);
+          }
         }
-      }),
+      ),
       this.boardsDataStore.onDidChange((event) => {
         const selectedFqbn =
           this.boardsServiceProvider.boardsConfig.selectedBoard?.fqbn;
@@ -88,6 +89,10 @@ export class UpdateArduinoState extends SketchContribution {
     this.updateSketchPath(this.sketchServiceClient.tryGetCurrentSketch());
     this.updateUserDirPath(this.configService.tryGetSketchDirUri());
     this.updateDataDirPath(this.configService.tryGetDataDirUri());
+    const { compileSummary } = this.compileSummaryProvider;
+    if (compileSummary) {
+      this.updateCompileSummary(compileSummary);
+    }
   }
 
   onStop(): 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<CompileSummary | undefined>;
+}
+
 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,20 @@ 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<
+    CompileSummary | undefined
+  >();
   private verifyProgress: VerifyProgress = 'idle';
+  private _compileSummary: CompileSummary | undefined;
 
   override registerCommands(registry: CommandRegistry): void {
     registry.registerCommand(VerifySketch.Commands.VERIFY_SKETCH, {
@@ -117,6 +130,21 @@ export class VerifySketch extends CoreServiceContribution {
     super.handleError(error);
   }
 
+  get compileSummary(): CompileSummary | undefined {
+    return this._compileSummary;
+  }
+
+  private updateCompileSummary(
+    compileSummary: CompileSummary | undefined
+  ): void {
+    this._compileSummary = compileSummary;
+    this.onDidChangeCompileSummaryEmitter.fire(this._compileSummary);
+  }
+
+  get onDidChangeCompileSummary(): Event<CompileSummary | undefined> {
+    return this.onDidChangeCompileSummaryEmitter.event;
+  }
+
   private async verifySketch(
     params?: VerifySketchParams
   ): Promise<CoreService.Options.Compile | undefined> {
@@ -141,7 +169,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 +188,9 @@ export class VerifySketch extends CoreServiceContribution {
         nls.localize('arduino/sketch/doneCompiling', 'Done compiling.'),
         { timeout: 3000 }
       );
+
+      this.updateCompileSummary(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.
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,26 @@ 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
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
