Review streaming response cancellation

Review exercise · TypeScript

Advanced45 min

Review brief

An analytics API now proxies large generated reports without buffering them in memory. Clients frequently cancel downloads or lose their connection, and the upstream generator is capacity-constrained. Review the stream lifecycle, flow control, and completion accounting.

Behavioral contract

Opening a report must receive the request AbortSignal, and client disconnect must promptly abort upstream generation and close the active async iterator.
When response.write returns false, wait for the next drain event before reading another upstream chunk.
After headers are sent, an upstream read failure must destroy the response so clients observe a truncated transfer rather than a successful end.
Record completed usage only after response.finished resolves to flushed; a closed or failed response must not be counted as delivered.
The source may throw before or during iteration, and its iterator return method is safe to call more than once.

Review instructions

1Review only the changed stream implementation and tests against the stated lifecycle contract.
2Trace normal completion, backpressure, upstream failure, and client disconnect independently.
3Prioritize no more than five findings, including meaningful test gaps.

Learning objectives

• Propagate cancellation across a streaming pipeline
• Respect writable backpressure without buffering unbounded data
• Distinguish response initiation from completed delivery

src/stream-report-types.tsTypeScript

@@ -0,0 +1,34 @@

1+export interface ReportRequest {

2+ reportId: string;

3+ signal: AbortSignal;

4+}

6+export interface ReportResponse {

7+ writableEnded: boolean;

8+ finished: Promise<"flushed" | "closed">;

9+ writeHead(status: number, headers: Record<string, string>): void;

10+ write(chunk: Uint8Array): boolean;

11+ once(event: "drain", listener: () => void): void;

12+ end(): void;

13+ destroy(error?: Error): void;

14+ on(event: "close", listener: () => void): void;

15+}

16+

17+export interface ReportSource {

18+ open(

19+ reportId: string,

20+ options?: { signal: AbortSignal },

21+ ): Promise<AsyncIterable<Uint8Array>>;

22+}

23+

24+export interface UsageLedger {

25+ recordCompleted(input: {

26+ reportId: string;

27+ deliveredBytes: number;

28+ }): Promise<void>;

29+}

30+

31+export interface StreamDependencies {

32+ reports: ReportSource;

33+ usage: UsageLedger;

34+}

Reference solution

The findings below are ordered by priority.

Findings

1. Client cancellation never reaches the report generator

Location: src/stream-report.ts:11-20

The source API accepts an AbortSignal option, but the handler calls open with only the report ID and never observes either the request signal or the response close event. A disconnected client therefore leaves the capacity-constrained generator and the for await loop running until the entire report is produced.

Pass the request signal in the source's open options, combine it with cancellation triggered by the response close lifecycle, and explicitly close the active iterator when cancellation wins.

2. The loop ignores writable backpressure

Location: src/stream-report.ts:19-22

response.write can return false, but the loop immediately asks the source for another chunk. A slow client can make the process buffer a large report in memory instead of applying backpressure upstream.

When a write returns false, wait for the next drain notification, while still allowing cancellation, before reading the next chunk.

3. Upstream iteration failures do not terminate the partial response deliberately

Location: src/stream-report.ts:19-24

An exception from the async iterable escapes after the success headers have been sent. The handler never destroys the response, so behavior depends on an outer framework and may leave the client with an ambiguous or apparently complete transfer.

Catch iteration failures after headers, destroy the response with the error, close the iterator in a finally path, and rethrow only if the framework still needs the failure.

4. Usage is recorded before delivery is known to have flushed

Location: src/stream-report.ts:24-28

Calling end merely initiates the final write. The ledger is updated without awaiting response.finished, so a socket that closes while buffered bytes are flushing is counted as a completed download.

Await the finish outcome and record usage only for flushed; skip the completion record for closed or failed streams.

5. The test covers only an unconstrained successful response

Location: src/stream-report.test.ts:6-43

The fake always accepts writes, resolves the finish state immediately, and never cancels or throws. Its interaction assertion should check only that opening occurred rather than pinning the obsolete one-argument call shape; as written in the corrected target it still cannot protect cancellation propagation, iterator cleanup, drain waiting, truncated-response handling, or completion accounting.

Add focused cases for a false write result, client close, an upstream exception, and a close-before-flush outcome, asserting both resource cleanup and ledger calls.

Reasonable non-findings

Streaming chunks without concatenating them is the correct memory model once flow control is honored.
application/octet-stream and private no-store caching are appropriate for the stated report format.
Counting the byte lengths supplied by the source is reasonable when the finish outcome confirms that the transfer flushed.
Awaiting the usage ledger after successful delivery is acceptable; the contract does not require that accounting be detached from the request.