Skip to content

Commit acfc842

Browse files
committed
fix(goods): handle errors in responseToReadable stream reader
Fixes #1443
1 parent 98531fc commit acfc842

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

src/goods.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,12 @@ const responseToReadable = (response: Response, rs: Readable) => {
111111
return rs
112112
}
113113
rs._read = async () => {
114-
const result = await reader.read()
115-
rs.push(result.done ? null : Buffer.from(result.value))
114+
try {
115+
const result = await reader.read()
116+
rs.push(result.done ? null : Buffer.from(result.value))
117+
} catch (err) {
118+
rs.destroy(err as Error)
119+
}
116120
}
117121
return rs
118122
}

test/goods.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import assert from 'node:assert'
1616
import { test, describe, after } from 'node:test'
17-
import { Duplex } from 'node:stream'
17+
import { Duplex, Readable } from 'node:stream'
1818
import { $, chalk, fs, path, dotenv } from '../src/index.ts'
1919
import {
2020
echo,
@@ -365,6 +365,46 @@ describe('goods', () => {
365365
assert(p3.includes('GitHub'))
366366
})
367367

368+
test('reader error in _read is caught and destroys stream', async () => {
369+
// responseToReadable (private) assigns an async _read to a Readable.
370+
// This test verifies the behavioral contract: when a web ReadableStream
371+
// reader rejects, the error must surface via the Node.js Readable's
372+
// 'error' event (via rs.destroy) rather than as an unhandled rejection.
373+
const error = new Error('stream read failed')
374+
const webStream = new ReadableStream({
375+
start(controller) {
376+
controller.enqueue(new TextEncoder().encode('ok'))
377+
},
378+
pull() {
379+
throw error
380+
},
381+
})
382+
const reader = webStream.getReader()
383+
const rs = new Readable({ read() {} })
384+
385+
rs._read = async () => {
386+
try {
387+
const result = await reader.read()
388+
rs.push(result.done ? null : Buffer.from(result.value))
389+
} catch (err) {
390+
rs.destroy(err as Error)
391+
}
392+
}
393+
394+
// First read should succeed
395+
const firstChunk = await new Promise<Buffer | null>((resolve) => {
396+
rs.once('data', resolve)
397+
})
398+
assert.ok(firstChunk)
399+
assert.equal(firstChunk.toString(), 'ok')
400+
401+
// Second read triggers the error in pull(), which should be caught
402+
const receivedError = await new Promise<Error>((resolve) => {
403+
rs.on('error', resolve)
404+
})
405+
assert.equal(receivedError.message, 'stream read failed')
406+
})
407+
368408
describe('dotenv', () => {
369409
test('parse()', () => {
370410
assert.deepEqual(dotenv.parse(''), {})

0 commit comments

Comments
 (0)