Skip to content

Commit a5fcadf

Browse files
committed
fix(cli): chain progress-log writes so every write is awaited
Awaiting only the latest progressWrite left earlier chunks' writes fire-and-forget, so a trailing write could still recreate the progress log after cleanup. Chain each write onto the previous so settling awaits every write.
1 parent 2025f51 commit a5fcadf

2 files changed

Lines changed: 31 additions & 31 deletions

File tree

‎src/core/cliManager.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -805,50 +805,50 @@ export class CliManager {
805805
: (buffer.byteLength / contentLength) * 100,
806806
});
807807
if (onProgress) {
808-
progressWrite = onProgress(
809-
written,
810-
Number.isNaN(contentLength) ? null : contentLength,
811-
).catch((error) => {
812-
this.output.warn(
813-
"Failed to write progress log:",
814-
errToStr(error),
815-
);
816-
});
808+
// Chain so awaiting the final progressWrite awaits every
809+
// progress-log write, not just the last one.
810+
progressWrite = progressWrite
811+
.then(() =>
812+
onProgress(
813+
written,
814+
Number.isNaN(contentLength) ? null : contentLength,
815+
),
816+
)
817+
.catch((error) => {
818+
this.output.warn(
819+
"Failed to write progress log:",
820+
errToStr(error),
821+
);
822+
});
817823
}
818824
});
819825
});
820826

821827
// Wait for the stream to end or error.
822828
return new Promise<boolean>((resolve, reject) => {
829+
// fs emits "close" only after every pending write callback, so
830+
// settling after it cannot race the trailing progress-log write.
831+
const settle = (settleFn: () => void): void => {
832+
writeStream.once("close", () => {
833+
void progressWrite.then(settleFn);
834+
});
835+
};
836+
const downloadError = (error: unknown): Error =>
837+
new Error(
838+
`Unable to download binary: ${errToStr(error, "no reason given")}`,
839+
);
840+
823841
writeStream.on("error", (error) => {
824842
readStream.destroy();
825-
void progressWrite.then(() =>
826-
reject(
827-
new Error(
828-
`Unable to download binary: ${errToStr(error, "no reason given")}`,
829-
),
830-
),
831-
);
843+
settle(() => reject(downloadError(error)));
832844
});
833845
readStream.on("error", (error) => {
834846
writeStream.close();
835-
void progressWrite.then(() =>
836-
reject(
837-
new Error(
838-
`Unable to download binary: ${errToStr(error, "no reason given")}`,
839-
),
840-
),
841-
);
847+
settle(() => reject(downloadError(error)));
842848
});
843849
readStream.on("close", () => {
844850
writeStream.close();
845-
void progressWrite.then(() => {
846-
if (cancelled) {
847-
resolve(false);
848-
} else {
849-
resolve(true);
850-
}
851-
});
851+
settle(() => resolve(!cancelled));
852852
});
853853
});
854854
},

‎test/unit/core/cliManagerHarness.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export function setupCliManager(basePath: string = BASE_PATH) {
124124
writeStream.write = ((chunk: Buffer, callback?: () => void) => {
125125
memfs.appendFileSync(String(writePath), chunk);
126126
if (callback) {
127-
pendingWrites.push(callback);
127+
pendingWrites.push(callback);
128128
}
129129
return true;
130130
}) as fs.WriteStream["write"];

0 commit comments

Comments
 (0)