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

* 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
https://github.com/dankeboy36/esp-exception-decoder/issues/28#issuecomment-2681800772.

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 arduino/arduino-ide#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 b11bde1c47

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>

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
 

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
+      );
+    }
+  }
 }

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 {

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.

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

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;

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 compileSummaryMock: CompileSummary | undefined;
   let onCurrentSketchDidChangeEmitter: Emitter<CurrentSketch>;
   let onDataDirDidChangeEmitter: Emitter<URI | undefined>;
   let onSketchDirDidChangeEmitter: Emitter<URI | undefined>;
   let onDataStoreDidChangeEmitter: Emitter<BoardsDataStoreChangeEvent>;
+  let compileSummaryDidChangeEmitter: Emitter<CompileSummary | undefined>;
 
   beforeEach(async () => {
     toDisposeAfterEach = new DisposableCollection();
@@ -76,15 +79,18 @@ describe('update-arduino-state', function () {
     currentSketchMock = undefined;
     sketchDirUriMock = undefined;
     dataDirUriMock = undefined;
+    compileSummaryMock = 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
-    );
+    compileSummaryMock = summary;
+    compileSummaryDidChangeEmitter.fire(compileSummaryMock);
     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 compileSummaryMock;
+          },
+          onDidChangeCompileSummary: compileSummaryDidChangeEmitter.event,
+        });
         rebind(BoardsService).toConstantValue(<BoardsService>{
           getDetectedPorts() {
             return {};

arduino-ide-extension/src/test/node/core-service-impl.slow-test.ts

@@ -1,12 +1,11 @@
-import { CancellationTokenSource } from '@theia/core/lib/common/cancellation';
-import { CommandRegistry } from '@theia/core/lib/common/command';
 import { DisposableCollection } from '@theia/core/lib/common/disposable';
 import { isWindows } from '@theia/core/lib/common/os';
 import { FileUri } from '@theia/core/lib/node/file-uri';
-import { Container, injectable } from '@theia/core/shared/inversify';
+import { Container } from '@theia/core/shared/inversify';
 import { expect } from 'chai';
 import {
   BoardsService,
+  CompileSummary,
   CoreService,
   SketchesService,
   isCompileSummary,
@@ -36,11 +35,9 @@ describe('core-service-impl', () => {
       this.timeout(testTimeout);
       const coreService = container.get<CoreService>(CoreService);
       const sketchesService = container.get<SketchesService>(SketchesService);
-      const commandService =
-        container.get<TestCommandRegistry>(TestCommandRegistry);
       const sketch = await sketchesService.createNewSketch();
 
-      await coreService.compile({
+      const compileSummary = await coreService.compile({
         fqbn: uno,
         sketch,
         optimizeForDebug: false,
@@ -48,18 +45,9 @@ describe('core-service-impl', () => {
         verbose: true,
       });
 
-      const executedBuildDidCompleteCommands =
-        commandService.executedCommands.filter(
-          ([command]) =>
-            command === 'arduino.languageserver.notifyBuildDidComplete'
-        );
-      expect(executedBuildDidCompleteCommands.length).to.be.equal(1);
-      const [, args] = executedBuildDidCompleteCommands[0];
-      expect(args.length).to.be.equal(1);
-      const arg = args[0];
-      expect(isCompileSummary(arg)).to.be.true;
-      expect('buildOutputUri' in arg).to.be.true;
-      expect(arg.buildOutputUri).to.be.not.undefined;
+      expect(isCompileSummary(compileSummary)).to.be.true;
+      expect((<CompileSummary>compileSummary).buildOutputUri).to.be.not
+        .undefined;
 
       const tempBuildPaths = await sketchesService.getBuildPath(sketch);
       if (isWindows) {
@@ -68,7 +56,7 @@ describe('core-service-impl', () => {
         expect(tempBuildPaths.length).to.be.equal(1);
       }
 
-      const { buildOutputUri } = arg;
+      const { buildOutputUri } = <CompileSummary>compileSummary;
       const buildOutputPath = FileUri.fsPath(buildOutputUri).toString();
       expect(tempBuildPaths.includes(buildOutputPath)).to.be.true;
     });
@@ -91,35 +79,5 @@ async function start(
 }
 
 async function createContainer(): Promise<Container> {
-  return createBaseContainer({
-    additionalBindings: (bind, rebind) => {
-      bind(TestCommandRegistry).toSelf().inSingletonScope();
-      rebind(CommandRegistry).toService(TestCommandRegistry);
-    },
-  });
-}
-
-@injectable()
-class TestCommandRegistry extends CommandRegistry {
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  readonly executedCommands: [string, any[]][] = [];
-
-  override async executeCommand<T>(
-    commandId: string,
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    ...args: any[]
-  ): Promise<T | undefined> {
-    const { token } = new CancellationTokenSource();
-    this.onWillExecuteCommandEmitter.fire({
-      commandId,
-      args,
-      token,
-      waitUntil: () => {
-        // NOOP
-      },
-    });
-    this.executedCommands.push([commandId, args]);
-    this.onDidExecuteCommandEmitter.fire({ commandId, args });
-    return undefined;
-  }
+  return createBaseContainer();
 }