moved out progress reporting from indexes update

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

Diff for: arduino-ide-extension/src/node/core-client-provider.ts

@@ -19,7 +19,7 @@ import {
   UpdateLibrariesIndexResponse,
 } from './cli-protocol/cc/arduino/cli/commands/v1/commands_pb';
 import * as commandsGrpcPb from './cli-protocol/cc/arduino/cli/commands/v1/commands_grpc_pb';
-import { NotificationServiceServer, ProgressMessage } from '../common/protocol';
+import { NotificationServiceServer } from '../common/protocol';
 import { Deferred, retry } from '@theia/core/lib/common/promise-util';
 import {
   Status as RpcStatus,
@@ -29,8 +29,10 @@ import { ConfigServiceImpl } from './config-service-impl';
 import { ArduinoDaemonImpl } from './arduino-daemon-impl';
 import { DisposableCollection } from '@theia/core/lib/common/disposable';
 import { Disposable } from '@theia/core/shared/vscode-languageserver-protocol';
-import { v4 } from 'uuid';
-import { InstallWithProgress } from './grpc-installable';
+import {
+  IndexesUpdateProgressHandler,
+  InstallWithProgress,
+} from './grpc-installable';
 import type { DefaultCliConfig } from './cli-config';
 import { ServiceError } from './service-error';
 
@@ -242,91 +244,45 @@ export class CoreClientProvider {
   private async updateIndexes(
     client: CoreClientProvider.Client
   ): Promise<CoreClientProvider.Client> {
-    const progressId = v4();
-    // Note: at this point, the IDE2 backend might not have any connected clients, so this notification is not delivered to anywhere
-    // Hence, clients must handle gracefully when no `willUpdate` is received before any `didProgress`.
-    this.notificationService.notifyIndexWillUpdate(progressId);
-    // The index update progress responses are not much helpful from the CLI. They cannot provide a fine-grain progress.
-    // So IDE2 could report two total works only: index update and library index update.
-    // See here an example: https://github.com/arduino/arduino-ide/issues/906#issuecomment-1171145630
-    // Due to this, IDE2 fakes the progress to provide slightly better UX.
-    const totalPlatformIndexCount =
-      (this.configService.cliConfiguration?.board_manager?.additional_urls
-        ?.length ?? 0) + 1; // +1 for the `package_index.tar.bz2` when updating the platform index.
-    // The `library_index.json.gz` and `library_index.json.sig` when running the library index update.
-    const totalLibraryIndexCount = 2;
-    let work = {
-      done: 0,
-      total: totalPlatformIndexCount + totalLibraryIndexCount + 1, // +1 for better UX, leave the last step complete when all subtasks are done.
-    };
-    const reportProgress = ({
-      progressId,
-      message,
-    }: Omit<ProgressMessage, 'work'>) => {
-      work = {
-        ...work,
-        done: work.done + 1,
-      };
-      this.notificationService.notifyIndexUpdateDidProgress({
-        message,
-        progressId,
-        work,
-      });
-    };
-    const toDispose = new DisposableCollection();
+    const progressHandler = this.createProgressHandler();
     try {
-      const onDidProgressEmitter = new Emitter<ProgressMessage>();
-      const onDidProgress = onDidProgressEmitter.event;
-      toDispose.pushAll([
-        onDidProgressEmitter,
-        onDidProgress(({ message, progressId }) =>
-          reportProgress({ progressId, message })
-        ),
-      ]);
       await Promise.all([
-        this.updateIndex(client, progressId, onDidProgressEmitter),
-        this.updateLibraryIndex(client, progressId, onDidProgressEmitter),
-      ]);
-      this.notificationService.notifyIndexDidUpdate(progressId);
+        this.updateIndex(client, progressHandler),
+        this.updateLibraryIndex(client, progressHandler),
+      ]).then(() => progressHandler.reportEnd());
     } catch (err) {
-      this.notificationService.notifyIndexUpdateDidFail({
-        progressId,
-        message: ServiceError.is(err) ? err.details : String(err),
-      });
-    } finally {
-      toDispose.dispose();
+      console.error('Failed to update indexes', err);
+      progressHandler.reportError(
+        ServiceError.is(err) ? err.details : String(err)
+      );
     }
     return client;
   }
 
   private async updateIndex(
     client: CoreClientProvider.Client,
-    progressId: string,
-    onDidProgressEmitter: Emitter<ProgressMessage>
+    progressHandler: IndexesUpdateProgressHandler
   ): Promise<void> {
     return this.doUpdateIndex(
       () =>
         client.client.updateIndex(
           new UpdateIndexRequest().setInstance(client.instance)
         ),
-      progressId,
-      onDidProgressEmitter,
+      progressHandler,
       'platform-index'
     );
   }
 
   private async updateLibraryIndex(
     client: CoreClientProvider.Client,
-    progressId: string,
-    onDidProgressEmitter: Emitter<ProgressMessage>
+    progressHandler: IndexesUpdateProgressHandler
   ): Promise<void> {
     return this.doUpdateIndex(
       () =>
         client.client.updateLibrariesIndex(
           new UpdateLibrariesIndexRequest().setInstance(client.instance)
         ),
-      progressId,
-      onDidProgressEmitter,
+      progressHandler,
       'library-index'
     );
   }
@@ -338,10 +294,10 @@ export class CoreClientProvider {
       | UpdateCoreLibrariesIndexResponse // not used by IDE2
   >(
     responseProvider: () => grpc.ClientReadableStream<R>,
-    progressId: string,
-    onDidProgressEmitter: Emitter<ProgressMessage>,
+    progressHandler: IndexesUpdateProgressHandler,
     task?: string
   ): Promise<void> {
+    const { progressId } = progressHandler;
     return retry(
       () =>
         new Promise<void>((resolve, reject) => {
@@ -350,15 +306,12 @@ export class CoreClientProvider {
               'data',
               InstallWithProgress.createDataCallback({
                 responseService: {
-                  appendToOutput: (message) => {
+                  appendToOutput: ({ chunk: message }) => {
                     console.log(
                       `core-client-provider${task ? ` [${task}]` : ''}`,
-                      message.chunk
+                      message
                     );
-                    onDidProgressEmitter.fire({
-                      message: message.chunk,
-                      progressId,
-                    });
+                    progressHandler.reportProgress(message);
                   },
                 },
                 progressId,
@@ -372,6 +325,26 @@ export class CoreClientProvider {
     );
   }
 
+  private createProgressHandler() {
+    const additionalUrlsCount =
+      this.configService.cliConfiguration?.board_manager?.additional_urls
+        ?.length ?? 0;
+    const progressHandler = new IndexesUpdateProgressHandler(
+      additionalUrlsCount,
+      (progressMessage) =>
+        this.notificationService.notifyIndexUpdateDidProgress(progressMessage),
+      ({ progressId, message }) =>
+        this.notificationService.notifyIndexUpdateDidFail({
+          progressId,
+          message,
+        }),
+      (progressId) =>
+        this.notificationService.notifyIndexWillUpdate(progressId),
+      (progressId) => this.notificationService.notifyIndexDidUpdate(progressId)
+    );
+    return progressHandler;
+  }
+
   private address(port: string): string {
     return `localhost:${port}`;
   }

Diff for: arduino-ide-extension/src/node/grpc-installable.ts

@@ -213,3 +213,54 @@ export namespace InstallWithProgress {
     return undefined;
   }
 }
+
+export class IndexesUpdateProgressHandler {
+  private done = 0;
+  private readonly total: number;
+  readonly progressId: string;
+
+  constructor(
+    additionalUrlsCount: number,
+    private readonly onProgress: (progressMessage: ProgressMessage) => void,
+    private readonly onError?: ({
+      progressId,
+      message,
+    }: {
+      progressId: string;
+      message: string;
+    }) => void,
+    private readonly onStart?: (progressId: string) => void,
+    private readonly onEnd?: (progressId: string) => void
+  ) {
+    this.progressId = v4();
+    this.total = IndexesUpdateProgressHandler.total(additionalUrlsCount);
+    // Note: at this point, the IDE2 backend might not have any connected clients, so this notification is not delivered to anywhere
+    // Hence, clients must handle gracefully when no `willUpdate` is received before any `didProgress`.
+    this.onStart?.(this.progressId);
+  }
+
+  reportEnd(): void {
+    this.onEnd?.(this.progressId);
+  }
+
+  reportProgress(message: string): void {
+    this.onProgress({
+      message,
+      progressId: this.progressId,
+      work: { total: this.total, done: this.done++ },
+    });
+  }
+
+  reportError(message: string): void {
+    this.onError?.({ progressId: this.progressId, message });
+  }
+
+  private static total(additionalUrlsCount: number): number {
+    // +1 for the `package_index.tar.bz2` when updating the platform index.
+    const totalPlatformIndexCount = additionalUrlsCount + 1;
+    // The `library_index.json.gz` and `library_index.json.sig` when running the library index update.
+    const totalLibraryIndexCount = 2;
+    // +1 for better UX (`reportEnd`)
+    return totalPlatformIndexCount + totalLibraryIndexCount + 1;
+  }
+}