fix: make artifact inventory consumption deterministic on download (#1073)

Artifact downloads consumed the inventory entry only from the source
stream's 'end' / response 'finish' handlers. But the response carries an
explicit content-length, so a client's fetch completes as soon as that
many body bytes arrive - which happens right after the final chunk is
written to the socket, while the server's read stream still needs one
more threadpool read to detect EOF and emit 'end'. Under CPU contention
(e.g. the instrumented Coverage CI job) that threadpool completion can
land after the client's follow-up GET /artifacts, which then still lists
the consumed artifact.

Consume the entry as soon as the stream has emitted content-length
bytes, before pipe() writes the final chunk to the client. Any client
that observed a complete download is now guaranteed to observe the
entry as consumed. The 'end'/'finish'/'close' handlers stay as backstops
(idempotent via didCleanupArtifact).
This commit is contained in:
Michał Pierzchała
2026-07-04 10:25:31 +02:00
committed by GitHub
parent 50a6a3533a
commit 1009086145
+8
View File
@@ -113,6 +113,14 @@ async function handleArtifactDownload(
didCleanupArtifact = true;
cleanupDownloadableArtifact(artifactId);
};
// Consume before the final chunk is written to the client: fetch treats a
// download as complete once content-length bytes arrive, which can happen
// before this stream emits 'end' (that needs one more threadpool read).
let bytesSent = 0;
stream.on('data', (chunk: string | Buffer) => {
bytesSent += chunk.length;
if (bytesSent >= artifact.sizeBytes) cleanupCompletedDownload();
});
stream.on('end', cleanupCompletedDownload);
res.on('finish', cleanupCompletedDownload);
res.on('close', () => {