Skip to content

Commit 6ad5ca7

Browse files
authored
fix(listener): Limit retries to a maximum of three. (#267)
* fix(listener): Limit retries to a maximum of three. * refactor: run `bun format:fix`
1 parent 6ffc15e commit 6ad5ca7

File tree

2 files changed

+91
-4
lines changed

2 files changed

+91
-4
lines changed

src/listener.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,10 @@ const responseViaResponseObject = async (
131131
let done = false
132132
let currentReadPromise: Promise<ReadableStreamReadResult<Uint8Array>> | undefined = undefined
133133

134-
// In the case of synchronous responses, usually a maximum of two readings is done
135-
for (let i = 0; i < 2; i++) {
136-
currentReadPromise = reader.read()
134+
// In the case of synchronous responses, usually a maximum of two (or three in special cases) readings is done
135+
let maxReadCount = 2
136+
for (let i = 0; i < maxReadCount; i++) {
137+
currentReadPromise ||= reader.read()
137138
const chunk = await readWithoutBlocking(currentReadPromise).catch((e) => {
138139
console.error(e)
139140
done = true
@@ -143,7 +144,7 @@ const responseViaResponseObject = async (
143144
// XXX: In Node.js v24, some response bodies are not read all the way through until the next task queue,
144145
// so wait a moment and retry. (e.g. new Blob([new Uint8Array(contents)]) )
145146
await new Promise((resolve) => setTimeout(resolve))
146-
i--
147+
maxReadCount = 3
147148
continue
148149
}
149150

test/server.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ describe('various response body types', () => {
137137
const largeText = 'a'.repeat(1024 * 1024 * 10)
138138
let server: ServerType
139139
let resolveReadableStreamPromise: () => void
140+
let resolveEventStreamPromise: () => void
141+
let resolveEventStreamWithoutTransferEncodingPromise: () => void
140142
beforeAll(() => {
141143
const app = new Hono()
142144
app.use('*', async (c, next) => {
@@ -183,6 +185,43 @@ describe('various response body types', () => {
183185
})
184186
return new Response(stream)
185187
})
188+
const eventStreamPromise = new Promise<void>((resolve) => {
189+
resolveEventStreamPromise = resolve
190+
})
191+
app.get('/event-stream', () => {
192+
const stream = new ReadableStream({
193+
async start(controller) {
194+
controller.enqueue('data: First!\n\n')
195+
await eventStreamPromise
196+
controller.enqueue('data: Second!\n\n')
197+
controller.close()
198+
},
199+
})
200+
return new Response(stream, {
201+
headers: {
202+
'content-type': 'text/event-stream',
203+
'transfer-encoding': 'chunked',
204+
},
205+
})
206+
})
207+
const eventStreamWithoutTransferEncodingPromise = new Promise<void>((resolve) => {
208+
resolveEventStreamWithoutTransferEncodingPromise = resolve
209+
})
210+
app.get('/event-stream-without-transfer-encoding', () => {
211+
const stream = new ReadableStream({
212+
async start(controller) {
213+
controller.enqueue('data: First!\n\n')
214+
await eventStreamWithoutTransferEncodingPromise
215+
controller.enqueue('data: Second!\n\n')
216+
controller.close()
217+
},
218+
})
219+
return new Response(stream, {
220+
headers: {
221+
'content-type': 'text/event-stream',
222+
},
223+
})
224+
})
186225
app.get('/buffer', () => {
187226
const response = new Response(Buffer.from('Hello Hono!'), {
188227
headers: { 'content-type': 'text/plain' },
@@ -256,6 +295,53 @@ describe('various response body types', () => {
256295
expect(expectedChunks.length).toBe(0) // all chunks are received
257296
})
258297

298+
it('Should return 200 response - GET /event-stream', async () => {
299+
const expectedChunks = ['data: First!\n\n', 'data: Second!\n\n']
300+
const resPromise = request(server)
301+
.get('/event-stream')
302+
.parse((res, fn) => {
303+
// response header should be sent before sending data.
304+
expect(res.headers['transfer-encoding']).toBe('chunked')
305+
resolveEventStreamPromise()
306+
307+
res.on('data', (chunk) => {
308+
const str = chunk.toString()
309+
expect(str).toBe(expectedChunks.shift())
310+
})
311+
res.on('end', () => fn(null, ''))
312+
})
313+
await new Promise((resolve) => setTimeout(resolve, 100))
314+
const res = await resPromise
315+
expect(res.status).toBe(200)
316+
expect(res.headers['content-type']).toMatch('text/event-stream')
317+
expect(res.headers['content-length']).toBeUndefined()
318+
expect(expectedChunks.length).toBe(0) // all chunks are received
319+
})
320+
321+
it('Should return 200 response - GET /event-stream-without-transfer-encoding', async () => {
322+
const expectedChunks = ['data: First!\n\n', 'data: Second!\n\n']
323+
const resPromise = request(server)
324+
.get('/event-stream-without-transfer-encoding')
325+
.parse((res, fn) => {
326+
res.on('data', (chunk) => {
327+
const str = chunk.toString()
328+
expect(str).toBe(expectedChunks.shift())
329+
330+
if (expectedChunks.length === 1) {
331+
// receive first chunk
332+
resolveEventStreamWithoutTransferEncodingPromise()
333+
}
334+
})
335+
res.on('end', () => fn(null, ''))
336+
})
337+
await new Promise((resolve) => setTimeout(resolve, 100))
338+
const res = await resPromise
339+
expect(res.status).toBe(200)
340+
expect(res.headers['content-type']).toMatch('text/event-stream')
341+
expect(res.headers['content-length']).toBeUndefined()
342+
expect(expectedChunks.length).toBe(0) // all chunks are received
343+
})
344+
259345
it('Should return 200 response - GET /buffer', async () => {
260346
const res = await request(server).get('/buffer')
261347
expect(res.status).toBe(200)

0 commit comments

Comments
 (0)