Skip to content

Commit 87826c2

Browse files
fix(rush): treat non-terminal statuses as non-terminal (microsoft#5277)
* fix: do not try and sync on cobuild waiting Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * add changeset Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * only assign operations on complete Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * update changeset Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * fix snapshots Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * Update libraries/rush-lib/src/logic/operations/OperationExecutionManager.ts Co-authored-by: David Michon <dmichon@microsoft.com> * fix test cases Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> * Update libraries/rush-lib/src/logic/operations/OperationExecutionManager.ts --------- Signed-off-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> Co-authored-by: Aramis Sennyey <aramissennyeydd@users.noreply.github.com> Co-authored-by: David Michon <dmichon@microsoft.com>
1 parent b9f8849 commit 87826c2

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "Do not run afterExecuteOperation if the operation has not actually completed.",
5+
"type": "none",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "aramissennyeydd@users.noreply.github.com"
11+
}

libraries/rush-lib/src/logic/operations/OperationExecutionManager.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -263,14 +263,19 @@ export class OperationExecutionManager {
263263
const onOperationCompleteAsync: (record: OperationExecutionRecord) => Promise<void> = async (
264264
record: OperationExecutionRecord
265265
) => {
266-
try {
267-
await this._afterExecuteOperation?.(record);
268-
} catch (e) {
269-
this._reportOperationErrorIfAny(record);
270-
record.error = e;
271-
record.status = OperationStatus.Failure;
266+
// If the operation is not terminal, we should _only_ notify the queue to assign operations.
267+
if (!record.isTerminal) {
268+
this._executionQueue.assignOperations();
269+
} else {
270+
try {
271+
await this._afterExecuteOperation?.(record);
272+
} catch (e) {
273+
this._reportOperationErrorIfAny(record);
274+
record.error = e;
275+
record.status = OperationStatus.Failure;
276+
}
277+
this._onOperationComplete(record);
272278
}
273-
this._onOperationComplete(record);
274279
};
275280

276281
const onOperationStartAsync: (
@@ -296,8 +301,8 @@ export class OperationExecutionManager {
296301
const status: OperationStatus = this._hasAnyFailures
297302
? OperationStatus.Failure
298303
: this._hasAnyNonAllowedWarnings
299-
? OperationStatus.SuccessWithWarning
300-
: OperationStatus.Success;
304+
? OperationStatus.SuccessWithWarning
305+
: OperationStatus.Success;
301306

302307
return {
303308
operationResults: this._executionRecords,
@@ -431,13 +436,12 @@ export class OperationExecutionManager {
431436
this._hasAnyNonAllowedWarnings = this._hasAnyNonAllowedWarnings || !runner.warningsAreAllowed;
432437
break;
433438
}
434-
}
435439

436-
if (record.isTerminal) {
437-
// If the operation was not remote, then we can notify queue that it is complete
438-
this._executionQueue.complete(record);
439-
} else {
440-
this._executionQueue.assignOperations();
440+
default: {
441+
throw new InternalError(`Unexpected operation status: ${status}`);
442+
}
441443
}
444+
445+
this._executionQueue.complete(record);
442446
}
443447
}

0 commit comments

Comments
 (0)