Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/listener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ export const getRequestListener = (
} else if (!outgoing.writableFinished) {
req[abortControllerKey].abort('Client connection prematurely closed.')
}

incoming.destroy()
})

res = fetchCallback(req, { incoming, outgoing } as HttpBindings) as
Expand Down
88 changes: 88 additions & 0 deletions test/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,40 @@
app.post('/posts', (c) => {
return c.redirect('/posts')
})
app.post('/no-body-consumed', (c) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these tests fail in the main? I've tried to do it, but these have not failed.

I want to know how we reproduce issue #246. I tried some cases. I can confirm that the case you pointed out with the Body Limit middleware has failed.

#246 (comment)

However, I couldn't find another case that failed due to the same issue. Is my understanding correct that only the case above fails?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yusukebe
Thank you for pointing that out!

You're right, I originally added it in #250 to ensure coverage, but in this branch, it is indeed a “repeat of tests that were successful in main.”

As I mentioned in the following comment, this issue cannot be tested with supertest because it requires an API that is aware of sockets at a low level.

#246 (comment)

I will add another test, so please bear with me for a little longer.

if (!c.req.raw.body) {
// force create new request object
throw new Error('No body consumed')
}
return c.text('No body consumed')
})
app.post('/body-cancelled', (c) => {
if (!c.req.raw.body) {
// force create new request object
throw new Error('No body consumed')
}
c.req.raw.body.cancel()
return c.text('No body consumed')
})
app.post('/partially-consumed', async (c) => {
if (!c.req.raw.body) {
// force create new request object
throw new Error('No body consumed')
}
const reader = c.req.raw.body.getReader()
await reader.read() // read only one chunk
return c.text('No body consumed')
})
app.post('/partially-consumed-and-cancelled', async (c) => {
if (!c.req.raw.body) {
// force create new request object
throw new Error('No body consumed')
}
const reader = c.req.raw.body.getReader()
await reader.read() // read only one chunk
reader.cancel()
return c.text('No body consumed')
})
app.delete('/posts/:id', (c) => {
return c.text(`DELETE ${c.req.param('id')}`)
})
Expand Down Expand Up @@ -82,6 +116,60 @@
expect(res.headers['location']).toBe('/posts')
})

it('Should return 200 response - POST /no-body-consumed', async () => {
const res = await request(server).post('/no-body-consumed').send('')
expect(res.status).toBe(200)
expect(res.text).toBe('No body consumed')
})

it('Should return 200 response - POST /body-cancelled', async () => {
const res = await request(server).post('/body-cancelled').send('')
expect(res.status).toBe(200)
expect(res.text).toBe('No body consumed')
})

it('Should return 200 response - POST /partially-consumed', async () => {
const buffer = Buffer.alloc(1024 * 10) // large buffer
const res = await new Promise<any>((resolve, reject) => {

Check warning on line 133 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (18.x)

Unexpected any. Specify a different type

Check warning on line 133 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (22.x)

Unexpected any. Specify a different type

Check warning on line 133 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (20.x)

Unexpected any. Specify a different type
const req = request(server)
.post('/partially-consumed')
.set('Content-Length', buffer.length.toString())

req.write(buffer)
req.end((err, res) => {
if (err) {
reject(err)
} else {
resolve(res)
}
})
})

expect(res.status).toBe(200)
expect(res.text).toBe('No body consumed')
})

it('Should return 200 response - POST /partially-consumed-and-cancelled', async () => {
const buffer = Buffer.alloc(1) // A large buffer will not make the test go far, so keep it small because it won't go far.
const res = await new Promise<any>((resolve, reject) => {

Check warning on line 154 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (18.x)

Unexpected any. Specify a different type

Check warning on line 154 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (22.x)

Unexpected any. Specify a different type

Check warning on line 154 in test/server.test.ts

View workflow job for this annotation

GitHub Actions / ci (20.x)

Unexpected any. Specify a different type
const req = request(server)
.post('/partially-consumed-and-cancelled')
.set('Content-Length', buffer.length.toString())

req.write(buffer)
req.end((err, res) => {
if (err) {
reject(err)
} else {
resolve(res)
}
})
})

expect(res.status).toBe(200)
expect(res.text).toBe('No body consumed')
})

it('Should return 201 response - DELETE /posts/123', async () => {
const res = await request(server).delete('/posts/123')
expect(res.status).toBe(200)
Expand Down