Skip to content

Commit f0bc616

Browse files
committed
fix: address Copilot review comments (round 5)
- Track last error instead of first error in AsyncSerializer: clear the error after each successful execution so the cycle only rejects if the final execution fails. A later coalesced success no longer causes a spurious rejection. - Update @returns JSDoc to precisely document the two-contract behavior: cycle-starters observe resolve/reject of the full cycle; coalesced callers only observe acknowledgement and do not see success/failure. - Update the coalesced-error test to reflect that promiseA now resolves (not rejects) when the coalesced re-run succeeds.
1 parent 766a20e commit f0bc616

2 files changed

Lines changed: 20 additions & 16 deletions

File tree

src/asyncSerializer.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,16 @@ export class AsyncSerializer {
3232
* re-execution.
3333
*
3434
* @param fn The async operation to execute
35-
* @returns A promise that resolves when this call has been acknowledged
36-
* (either executed immediately or queued for a coalesced re-run). Awaiting
37-
* this promise does not guarantee a distinct execution of {@link fn} for
38-
* this call; coalesced calls share execution cycles. The promise rejects
39-
* if any execution performed during this cycle throws, and rethrows the
40-
* first error encountered after pending executions have been processed.
35+
* @returns A promise whose meaning depends on whether this call starts a new
36+
* execution cycle. If no execution is in progress, the promise resolves
37+
* after the current execution cycle completes, including any coalesced
38+
* re-execution triggered while it is running, and rejects if the final
39+
* execution of that cycle fails. If an execution is already in progress,
40+
* the promise resolves once this call has been acknowledged and {@link fn}
41+
* has been stored as the latest pending operation; in that case, the
42+
* returned promise does not observe the eventual success or failure of the
43+
* running/coalesced execution and does not guarantee a distinct execution
44+
* of {@link fn}.
4145
*/
4246
async execute(fn: () => Promise<void>): Promise<void> {
4347
// If already executing, store latest fn for the coalesced re-run and return immediately
@@ -47,7 +51,7 @@ export class AsyncSerializer {
4751
}
4852

4953
this._isExecuting = true;
50-
let firstError: unknown | undefined;
54+
let lastError: unknown | undefined;
5155
try {
5256
// Keep executing while new requests come in, always using the latest queued fn
5357
let nextFn: (() => Promise<void>) | undefined = fn;
@@ -56,11 +60,9 @@ export class AsyncSerializer {
5660
this._pendingFn = undefined;
5761
try {
5862
await toRun();
63+
lastError = undefined; // Clear error if a subsequent execution succeeds
5964
} catch (error) {
60-
// Record the first error but continue processing pending executions
61-
if (firstError === undefined) {
62-
firstError = error;
63-
}
65+
lastError = error;
6466
}
6567
nextFn = this._pendingFn;
6668
}
@@ -69,8 +71,8 @@ export class AsyncSerializer {
6971
this._pendingFn = undefined;
7072
}
7173

72-
if (firstError !== undefined) {
73-
throw firstError;
74+
if (lastError !== undefined) {
75+
throw lastError;
7476
}
7577
}
7678
}

src/test/unit/asyncSerializer.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,17 @@ describe('AsyncSerializer', () => {
128128
}
129129
};
130130

131-
// Start call A (will error after barrier), then dispatch B and C which get coalesced
131+
// Start call A (will error partway through but coalesced re-run succeeds),
132+
// then dispatch B and C which get coalesced
132133
const promiseA = serializer.execute(operation);
133134
const promiseB = serializer.execute(operation);
134135
const promiseC = serializer.execute(operation);
135136

136137
release();
137138

138-
// A rejects; B and C already resolved immediately (coalesced)
139-
await promiseA.catch(() => {});
139+
// promiseA resolves (not rejects) because the coalesced re-run succeeded,
140+
// clearing the error from the first execution
141+
await promiseA;
140142
await promiseB;
141143
await promiseC;
142144

0 commit comments

Comments
 (0)