Skip to content

Commit

Permalink
fix: dispatch error handling (#2459)
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag authored Nov 26, 2023
1 parent 17c7a73 commit ac30400
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 24 deletions.
31 changes: 12 additions & 19 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -917,11 +917,9 @@ class Parser {
socket[kReset] = true
}

let pause
try {
pause = request.onHeaders(statusCode, headers, this.resume, statusText) === false
} catch (err) {
util.destroy(socket, err)
const pause = request.onHeaders(statusCode, headers, this.resume, statusText) === false

if (request.aborted) {
return -1
}

Expand Down Expand Up @@ -968,13 +966,8 @@ class Parser {

this.bytesRead += buf.length

try {
if (request.onData(buf) === false) {
return constants.ERROR.PAUSED
}
} catch (err) {
util.destroy(socket, err)
return -1
if (request.onData(buf) === false) {
return constants.ERROR.PAUSED
}
}

Expand Down Expand Up @@ -1015,11 +1008,7 @@ class Parser {
return -1
}

try {
request.onComplete(headers)
} catch (err) {
errorRequest(client, request, err)
}
request.onComplete(headers)

client[kQueue][client[kRunningIdx]++] = null

Expand Down Expand Up @@ -1805,13 +1794,17 @@ function writeH2 (client, session, request) {
})

stream.on('data', (chunk) => {
if (request.onData(chunk) === false) stream.pause()
if (request.onData(chunk) === false) {
stream.pause()
}
})

stream.once('close', () => {
h2State.openStreams -= 1
// TODO(HTTP/2): unref only if current streams count is 0
if (h2State.openStreams === 0) session.unref()
if (h2State.openStreams === 0) {
session.unref()
}
})

stream.once('error', function (err) {
Expand Down
34 changes: 29 additions & 5 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,11 @@ class Request {

onBodySent (chunk) {
if (this[kHandler].onBodySent) {
return this[kHandler].onBodySent(chunk)
try {
return this[kHandler].onBodySent(chunk)
} catch (err) {
this.abort(err)
}
}
}

Expand All @@ -239,7 +243,11 @@ class Request {
}

if (this[kHandler].onRequestSent) {
return this[kHandler].onRequestSent()
try {
return this[kHandler].onRequestSent()
} catch (err) {
this.abort(err)
}
}
}

Expand All @@ -263,14 +271,23 @@ class Request {
channels.headers.publish({ request: this, response: { statusCode, headers, statusText } })
}

return this[kHandler].onHeaders(statusCode, headers, resume, statusText)
try {
return this[kHandler].onHeaders(statusCode, headers, resume, statusText)
} catch (err) {
this.abort(err)
}
}

onData (chunk) {
assert(!this.aborted)
assert(!this.completed)

return this[kHandler].onData(chunk)
try {
return this[kHandler].onData(chunk)
} catch (err) {
this.abort(err)
return false
}
}

onUpgrade (statusCode, headers, socket) {
Expand All @@ -289,7 +306,13 @@ class Request {
if (channels.trailers.hasSubscribers) {
channels.trailers.publish({ request: this, trailers })
}
return this[kHandler].onComplete(trailers)

try {
return this[kHandler].onComplete(trailers)
} catch (err) {
// TODO (fix): This might be a bad idea?
this.onError(err)
}
}

onError (error) {
Expand All @@ -303,6 +326,7 @@ class Request {
return
}
this.aborted = true

return this[kHandler].onError(error)
}

Expand Down

0 comments on commit ac30400

Please sign in to comment.