Skip to content
Open
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
7 changes: 6 additions & 1 deletion lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,12 @@ function destroy (stream, err) {
stream.socket = null
}

stream.destroy(err)
try {
stream.destroy(err)
} catch {
// stream.destroy may throw on managed sockets (e.g., http2).
// Silently ignore — the socket lifecycle is handled by the subsystem.
}
} else if (err) {
queueMicrotask(() => {
stream.emit('error', err)
Expand Down
9 changes: 1 addition & 8 deletions lib/dispatcher/client-h2.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,6 @@ function connectH2 (client, socket) {

util.addListener(session, 'error', onHttp2SessionError)
util.addListener(session, 'frameError', onHttp2FrameError)
util.addListener(session, 'end', onHttp2SessionEnd)
util.addListener(session, 'goaway', onHttp2SessionGoAway)
util.addListener(session, 'close', onHttp2SessionClose)
util.addListener(session, 'remoteSettings', onHttp2RemoteSettings)
Expand Down Expand Up @@ -505,12 +504,6 @@ function onHttp2FrameError (type, code, id) {
}
}

function onHttp2SessionEnd () {
const err = new SocketError('other side closed', util.getSocketInfo(this[kSocket]))
this.destroy(err)
util.destroy(this[kSocket], err)
}

/**
* This is the root cause of #3011
* We need to handle GOAWAY frames properly, and trigger the session close
Expand Down Expand Up @@ -654,7 +647,7 @@ function onHttp2SocketError (err) {
return
}

this[kClient][kOnError](err)
this[kHTTP2Session]?.[kClient]?.[kOnError](err)
}

function onHttp2SocketEnd () {
Expand Down
70 changes: 70 additions & 0 deletions test/http2-dispatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -1091,3 +1091,73 @@ test('Should not send http2 PING frames after connection is closed', async t =>

await t.completed
})

test('Should not emit uncaughtException when socket closes after abort', async t => {
t = tspl(t, { plan: 2 })

const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))

// Never respond, keeping the stream in-flight.
server.on('stream', (stream) => {
stream.on('error', () => {})
})

after(() => {
server.close()
})
Comment on lines +1105 to +1107

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use t.after

Suggested change
after(() => {
server.close()
})
t.after(() => {
server.close()
})

Ditto at other places.

await once(server.listen(0), 'listening')

// Capture any uncaughtException that fires during the test
let uncaughtErr = null
const originalHandler = process.listeners('uncaughtException').length > 0
? process.listeners('uncaughtException')[0]
: null
process.removeAllListeners('uncaughtException')
const uncaughtHandler = (err) => {
uncaughtErr = err
}
process.on('uncaughtException', uncaughtHandler)
after(() => {
process.removeListener('uncaughtException', uncaughtHandler)
if (originalHandler) {
process.on('uncaughtException', originalHandler)
}
})

const client = new Client(`https://localhost:${server.address().port}`, {
connect: { rejectUnauthorized: false },
allowH2: true
})
after(() => client.close())

const controller = new AbortController()

const requestPromise = client.request({
path: '/',
method: 'GET',
signal: controller.signal
})

// Abort the request to trigger the abort path
controller.abort()

// Wait for the request to reject
try {
await requestPromise
t.fail('request should have rejected')
} catch (err) {
t.ok(err, 'request rejected')
}

// Now close the server to trigger socket 'end' event,
// simulating the CDN closing the connection after abort.
// This used to trigger an uncaughtException via onHttp2SocketError.
server.close()

// Wait for any pending async error handling
await sleep(500)

t.equal(uncaughtErr, null, 'No uncaughtException should have been emitted')

await t.completed
})
Loading