Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 thread
mcollina marked this conversation as resolved.
Outdated
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