fix(stack-files): record download metric when response closes after a full read (#1282)

The file-download handler recorded its in-process download metric off the
source stream's end/close events. When the response consumer closed right
after receiving the whole file, those source events could be dropped and the
metric was never recorded, so the per-node download counter undercounted
successful reads under load.

Record the success directly on response close once the file has been fully
read, instead of waiting on source events that may never arrive. The abort
path (partial read) is unchanged and the recorder stays idempotent, so a
later source end/close cannot double-count. Adds a deterministic regression
test for the response-close-after-full-read path.

backend/src/__tests__/stack-files-routes.test.ts

@@ -502,6 +502,35 @@ describe('GET /api/stacks/:stackName/files/download', () => {
     }
   }
 
+  class ResponseCloseAfterFullReadStream extends Readable {
+    public bytesRead = 0;
+    public destroyCalls = 0;
+    public emittedResponseClose = false;
+
+    constructor(private readonly payload: Buffer) {
+      super();
+    }
+
+    _read(): void {}
+
+    override destroy(error?: Error): this {
+      this.destroyCalls += 1;
+      return super.destroy(error);
+    }
+
+    override pipe<T extends NodeJS.WritableStream>(destination: T): T {
+      destination.write(this.payload);
+      this.bytesRead = this.payload.length;
+      // The response closes after the whole body was received, and the source
+      // stream never emits its own end/close because the consumer is already
+      // gone. Mirrors the real fs.ReadStream path under load that previously
+      // left the download metric unrecorded.
+      this.emittedResponseClose = destination.emit('close');
+      destination.end();
+      return destination;
+    }
+  }
+
   class ErrorThenCloseStream extends Readable {
     _read(): void {}
 
@@ -681,6 +710,24 @@ describe('GET /api/stacks/:stackName/files/download', () => {
     await expectDownloadMetricCounts(1, 1, 0);
   });
 
+  it('records a success when the response closes after a full read and the source emits no end/close', async () => {
+    await resetFileExplorerMetrics();
+    const payload = Buffer.from('services: {}\n');
+    const stream = new ResponseCloseAfterFullReadStream(payload);
+    await mockDownloadStream(stream, payload.length);
+
+    const res = await request(app)
+      .get(`/api/stacks/${STACK}/files/download`)
+      .query({ path: 'compose.yaml' })
+      .set('Cookie', adminCookie);
+
+    expect(res.status).toBe(200);
+    expect(res.text).toBe(payload.toString('utf-8'));
+    expect(stream.emittedResponseClose).toBe(true);
+    expect(stream.destroyCalls).toBe(0);
+    await expectDownloadMetricCounts(1, 1, 0);
+  });
+
   it('records stream error followed by close exactly once as a failed download', async () => {
     await resetFileExplorerMetrics();
     const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);

backend/src/routes/stacks.ts

@@ -1504,12 +1504,12 @@ stacksRouter.get('/:stackName/files/download', async (req: Request, res: Respons
     const encodedFilename = encodeURIComponent(result.filename);
     const safeFilename = result.filename.replace(/[\\"]/g, '');
     res.setHeader('Content-Disposition', `attachment; filename="${safeFilename}"; filename*=UTF-8''${encodedFilename}`);
-    // Track normal download completion off the file stream's lifecycle, not
-    // the request lifecycle. Under the in-process supertest transport, request
-    // close events can race ahead of normal stream completion; response close
-    // is handled below only as delayed abort cleanup.
-    // End/error are the durable completion signals, and close can still count
-    // as success only when the stream reports it read the full file.
+    // Track download completion off both the file stream's lifecycle and the
+    // response close. Under the in-process supertest transport, request close
+    // events can race ahead of normal stream completion. End/error are the
+    // durable source signals; a close (source or response) counts as success
+    // only when the stream reports it read the full file, and as an abort
+    // otherwise.
     let downloadRecorded = false;
     const streamWithBytes = result.stream as typeof result.stream & { bytesRead?: number };
     const hasReadFullFile = (): boolean => (
@@ -1541,13 +1541,20 @@ stacksRouter.get('/:stackName/files/download', async (req: Request, res: Respons
     result.stream.on('end', () => recordDownloadOnce(hasReadFullFile()));
     result.stream.on('close', () => recordDownloadOnce(hasReadFullFile()));
     res.on('close', () => {
-      if (downloadRecorded || hasReadFullFile()) return;
-      // At this point, response close is treated as an abort signal. It can
-      // beat the source stream's final events in the in-process test transport.
-      // Give a same-turn clean source completion a chance to win before cleanup.
+      if (downloadRecorded) return;
+      // This op measures a server-side file read, so once the source has
+      // streamed the whole file a response close is a successful completion.
+      // Record it here rather than waiting on the source stream's end/close,
+      // which can be dropped once the response consumer is gone, leaving the
+      // op unrecorded.
+      if (hasReadFullFile()) { recordDownloadOnce(true); return; }
+      // Otherwise response close is an abort signal. It can beat the source
+      // stream's final events in the in-process test transport, so give a
+      // same-turn clean source completion a chance to win before cleanup.
       abortCleanupHandle = setImmediate(() => {
         abortCleanupHandle = null;
-        if (downloadRecorded || hasReadFullFile()) return;
+        if (downloadRecorded) return;
+        if (hasReadFullFile()) { recordDownloadOnce(true); return; }
         recordDownloadOnce(false);
         result.stream.destroy();
       });