[heft] Require a logging name for every operation (followup)

apps/heft/src/cli/HeftActionRunner.ts

@@ -333,8 +333,8 @@ export class HeftActionRunner {
       executeAsync: (state: IWatchLoopState): Promise<OperationStatus> => {
         return this._executeOnceAsync(executionManager, state.abortSignal, state.requestRun);
       },
-      onRequestRun: (requestor?: string) => {
-        terminal.writeLine(Colorize.bold(`New run requested by ${requestor || 'unknown task'}`));
+      onRequestRun: (requestor: string) => {
+        terminal.writeLine(Colorize.bold(`New run requested by ${requestor}`));
       },
       onAbort: () => {
         terminal.writeLine(Colorize.bold(`Cancelling incremental build...`));
@@ -346,7 +346,7 @@ export class HeftActionRunner {
   private async _executeOnceAsync(
     executionManager: OperationExecutionManager,
     abortSignal: AbortSignal,
-    requestRun?: (requestor?: string) => void
+    requestRun?: (requestor: string) => void
   ): Promise<OperationStatus> {
     // Record this as the start of task execution.
     this._metricsCollector.setStartTime();
@@ -447,6 +447,7 @@ function _getOrCreatePhaseOperation(
     // Only create the operation. Dependencies are hooked up separately
     operation = new Operation({
       groupName: phase.phaseName,
+      operationName: `${phase.phaseName} phase`,
       runner: new PhaseOperationRunner({ phase, internalHeftSession })
     });
     operations.set(key, operation);
@@ -466,6 +467,7 @@ function _getOrCreateTaskOperation(
   if (!operation) {
     operation = new Operation({
       groupName: task.parentPhase.phaseName,
+      operationName: `${task.taskName} task`,
       runner: new TaskOperationRunner({
         internalHeftSession,
         task

apps/heft/src/operations/runners/PhaseOperationRunner.ts

@@ -23,7 +23,7 @@ export class PhaseOperationRunner implements IOperationRunner {
   private readonly _options: IPhaseOperationRunnerOptions;
   private _isClean: boolean = false;
 
-  public get name(): string {
+  public get operationName(): string {
     return `Phase ${JSON.stringify(this._options.phase.phaseName)}`;
   }
 

apps/heft/src/operations/runners/TaskOperationRunner.ts

@@ -55,7 +55,7 @@ export class TaskOperationRunner implements IOperationRunner {
 
   public readonly silent: boolean = false;
 
-  public get name(): string {
+  public get operationName(): string {
     const { taskName, parentPhase } = this._options.task;
     return `Task ${JSON.stringify(taskName)} of phase ${JSON.stringify(parentPhase.phaseName)}`;
   }

common/changes/@rushstack/heft/octogonz-heft-issue-4467_2024-10-01-08-08.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/heft",
+      "comment": "Require a logging name for every operation.",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@rushstack/heft"
+}

common/changes/@rushstack/operation-graph/octogonz-heft-issue-4467_2024-10-01-08-08.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@rushstack/operation-graph",
+      "comment": "(BREAKING API CHANGE) Require an operationName for every operation to improve logging.",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@rushstack/operation-graph"
+}

common/reviews/api/operation-graph.api.md

@@ -33,7 +33,7 @@ export interface IExecuteOperationContext extends Omit<IOperationRunnerContext,
     afterExecute(operation: Operation, state: IOperationState): void;
     beforeExecute(operation: Operation, state: IOperationState): void;
     queueWork(workFn: () => Promise<OperationStatus>, priority: number): Promise<OperationStatus>;
-    requestRun?: (requestor?: string) => void;
+    requestRun?: (requestor: string) => void;
     terminal: ITerminal;
 }
 
@@ -50,23 +50,23 @@ export interface IOperationExecutionOptions {
     // (undocumented)
     parallelism: number;
     // (undocumented)
-    requestRun?: (requestor?: string) => void;
+    requestRun?: (requestor: string) => void;
     // (undocumented)
     terminal: ITerminal;
 }
 
 // @beta
 export interface IOperationOptions {
     groupName?: string | undefined;
-    name?: string | undefined;
+    operationName: string;
     runner?: IOperationRunner | undefined;
     weight?: number | undefined;
 }
 
 // @beta
 export interface IOperationRunner {
     executeAsync(context: IOperationRunnerContext): Promise<OperationStatus>;
-    readonly name: string;
+    readonly operationName: string;
     silent: boolean;
 }
 
@@ -99,7 +99,7 @@ export interface IRequestRunEventMessage {
     // (undocumented)
     event: 'requestRun';
     // (undocumented)
-    requestor?: string;
+    requestor: string;
 }
 
 // @beta
@@ -127,20 +127,20 @@ export interface IWatchLoopOptions {
     executeAsync: (state: IWatchLoopState) => Promise<OperationStatus>;
     onAbort: () => void;
     onBeforeExecute: () => void;
-    onRequestRun: (requestor?: string) => void;
+    onRequestRun: (requestor: string) => void;
 }
 
 // @beta
 export interface IWatchLoopState {
     // (undocumented)
     get abortSignal(): AbortSignal;
     // (undocumented)
-    requestRun: (requestor?: string) => void;
+    requestRun: (requestor: string) => void;
 }
 
 // @beta
 export class Operation implements IOperationStates {
-    constructor(options?: IOperationOptions);
+    constructor(options: IOperationOptions);
     // (undocumented)
     addDependency(dependency: Operation): void;
     readonly consumers: Set<Operation>;
@@ -152,7 +152,7 @@ export class Operation implements IOperationStates {
     _executeAsync(context: IExecuteOperationContext): Promise<OperationStatus>;
     readonly groupName: string | undefined;
     lastState: IOperationState | undefined;
-    readonly name: string | undefined;
+    readonly operationName: string;
     // (undocumented)
     reset(): void;
     runner: IOperationRunner | undefined;
@@ -231,7 +231,7 @@ export class Stopwatch {
 export class WatchLoop implements IWatchLoopState {
     constructor(options: IWatchLoopOptions);
     get abortSignal(): AbortSignal;
-    requestRun: (requestor?: string) => void;
+    requestRun: (requestor: string) => void;
     runIPCAsync(host?: IPCHost): Promise<void>;
     runUntilAbortedAsync(abortSignal: AbortSignal, onWaiting: () => void): Promise<void>;
     runUntilStableAsync(abortSignal: AbortSignal): Promise<OperationStatus>;

libraries/operation-graph/src/IOperationRunner.ts

@@ -81,7 +81,7 @@ export interface IOperationRunner {
   /**
    * Name of the operation, for logging.
    */
-  readonly name: string;
+  readonly operationName: string;
 
   /**
    * Indicates that this runner is architectural and should not be reported on.

libraries/operation-graph/src/Operation.ts

@@ -22,7 +22,7 @@ export interface IOperationOptions {
   /**
    * The name of this operation, for logging.
    */
-  name?: string | undefined;
+  operationName: string;
 
   /**
    * The group that this operation belongs to. Will be used for logging and duration tracking.
@@ -68,7 +68,7 @@ export interface IExecuteOperationContext extends Omit<IOperationRunnerContext,
    * A callback to the overarching orchestrator to request that the operation be invoked again.
    * Used in watch mode to signal that inputs have changed.
    */
-  requestRun?: (requestor?: string) => void;
+  requestRun?: (requestor: string) => void;
 
   /**
    * Terminal to write output to.
@@ -101,7 +101,7 @@ export class Operation implements IOperationStates {
   /**
    * The name of this operation, for logging.
    */
-  public readonly name: string | undefined;
+  public readonly operationName: string;
 
   /**
    * When the scheduler is ready to process this `Operation`, the `runner` implements the actual work of
@@ -174,11 +174,11 @@ export class Operation implements IOperationStates {
    */
   private _runPending: boolean = true;
 
-  public constructor(options?: IOperationOptions) {
+  public constructor(options: IOperationOptions) {
     this.groupName = options?.groupName;
     this.runner = options?.runner;
     this.weight = options?.weight || 1;
-    this.name = options?.name;
+    this.operationName = options.operationName;
   }
 
   public addDependency(dependency: Operation): void {
@@ -280,7 +280,9 @@ export class Operation implements IOperationStates {
                 // The requestRun callback is assumed to remain constant
                 // throughout the lifetime of the process, so it is safe
                 // to capture here.
-                return requestRun(this.name);
+                return requestRun(this.operationName);
+              case undefined:
+                throw new InternalError(`The operation state is undefined`);
               default:
                 // This line is here to enforce exhaustiveness
                 const currentStatus: undefined = this.state?.status;

libraries/operation-graph/src/OperationExecutionManager.ts

@@ -21,7 +21,7 @@ export interface IOperationExecutionOptions {
   parallelism: number;
   terminal: ITerminal;
 
-  requestRun?: (requestor?: string) => void;
+  requestRun?: (requestor: string) => void;
 }
 
 /**
@@ -77,8 +77,8 @@ export class OperationExecutionManager {
       for (const dependency of consumer.dependencies) {
         if (!operations.has(dependency)) {
           throw new Error(
-            `Operation ${JSON.stringify(consumer.name)} declares a dependency on operation ` +
-              `${JSON.stringify(dependency.name)} that is not in the set of operations to execute.`
+            `Operation ${JSON.stringify(consumer.operationName)} declares a dependency on operation ` +
+              `${JSON.stringify(dependency.operationName)} that is not in the set of operations to execute.`
           );
         }
       }

libraries/operation-graph/src/OperationGroupRecord.ts

@@ -54,7 +54,7 @@ export class OperationGroupRecord {
 
   public setOperationAsComplete(operation: Operation, state: IOperationState): void {
     if (!this._remainingOperations.has(operation)) {
-      throw new InternalError(`Operation ${operation.name} is not in the group ${this.name}`);
+      throw new InternalError(`Operation ${operation.operationName} is not in the group ${this.name}`);
     }
 
     if (state.status === OperationStatus.Aborted) {

libraries/operation-graph/src/WatchLoop.ts

@@ -31,7 +31,7 @@ export interface IWatchLoopOptions {
   /**
    * Logging callback when a run is requested (and hasn't already been).
    */
-  onRequestRun: (requestor?: string) => void;
+  onRequestRun: (requestor: string) => void;
   /**
    * Logging callback when a run is aborted.
    */
@@ -45,7 +45,7 @@ export interface IWatchLoopOptions {
  */
 export interface IWatchLoopState {
   get abortSignal(): AbortSignal;
-  requestRun: (requestor?: string) => void;
+  requestRun: (requestor: string) => void;
 }
 
 /**
@@ -60,7 +60,7 @@ export class WatchLoop implements IWatchLoopState {
   private _isRunning: boolean;
   private _runRequested: boolean;
   private _requestRunPromise: Promise<string | undefined>;
-  private _resolveRequestRun!: (requestor?: string) => void;
+  private _resolveRequestRun!: (requestor: string) => void;
 
   public constructor(options: IWatchLoopOptions) {
     this._options = options;
@@ -147,7 +147,7 @@ export class WatchLoop implements IWatchLoopState {
         }
       }
 
-      function requestRunFromHost(requestor?: string): void {
+      function requestRunFromHost(requestor: string): void {
         if (runRequestedFromHost) {
           return;
         }
@@ -192,9 +192,12 @@ export class WatchLoop implements IWatchLoopState {
 
             try {
               status = await this.runUntilStableAsync(abortController.signal);
-              // ESLINT: "Promises must be awaited, end with a call to .catch, end with a call to .then ..."
-              // eslint-disable-next-line @typescript-eslint/no-floating-promises
-              this._requestRunPromise.finally(requestRunFromHost);
+              this._requestRunPromise
+                // the reject callback in the promise is discarded so we ignore errors
+                .catch(() => {})
+                .finally(() => {
+                  requestRunFromHost('runIPCAsync');
+                });
             } catch (err) {
               status = OperationStatus.Failure;
               return reject(err);
@@ -225,7 +228,7 @@ export class WatchLoop implements IWatchLoopState {
   /**
    * Requests that a new run occur.
    */
-  public requestRun: (requestor?: string) => void = (requestor?: string) => {
+  public requestRun: (requestor: string) => void = (requestor: string) => {
     if (!this._runRequested) {
       this._options.onRequestRun(requestor);
       this._runRequested = true;

libraries/operation-graph/src/calculateCriticalPath.ts

@@ -2,7 +2,7 @@
 // See LICENSE in the project root for license information.
 
 export interface ISortableOperation<T extends ISortableOperation<T>> {
-  name: string | undefined;
+  operationName: string | undefined;
   criticalPathLength?: number | undefined;
   weight: number;
   consumers: Set<T>;
@@ -54,7 +54,9 @@ export function calculateShortestPath<T extends ISortableOperation<T>>(
   }
 
   if (!finalParent) {
-    throw new Error(`Could not find a path from "${startOperation.name}" to "${endOperation.name}"`);
+    throw new Error(
+      `Could not find a path from "${startOperation.operationName}" to "${endOperation.operationName}"`
+    );
   }
 
   // Walk back up the path from the end operation to the start operation
@@ -81,7 +83,7 @@ export function calculateCriticalPathLength<T extends ISortableOperation<T>>(
 
     throw new Error(
       'A cyclic dependency was encountered:\n  ' +
-        shortestPath.map((visitedTask) => visitedTask.name).join('\n  -> ')
+        shortestPath.map((visitedTask) => visitedTask.operationName).join('\n  -> ')
     );
   }
 

libraries/operation-graph/src/protocol.types.ts

@@ -10,7 +10,7 @@ import type { OperationStatus } from './OperationStatus';
  */
 export interface IRequestRunEventMessage {
   event: 'requestRun';
-  requestor?: string;
+  requestor: string;
 }
 
 /**