minor hook error handling

nodejs · Mar 31, 2024 · c81718e · c81718e
1 parent 7fb8232
commit c81718e
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 19 deletions.
diff --git a/lib/core/request.js b/lib/core/request.js
@@ -223,7 +223,11 @@ class Request {
       abort(this.error)
     } else {
       this.abort = abort
-      return this[kHandler].onConnect(abort)
+      try {
+        return this[kHandler].onConnect(abort)
+      } catch (err) {
+        abort(err)
+      }
     }
   }
 
@@ -254,15 +258,18 @@ class Request {
       return this[kHandler].onData(chunk)
     } catch (err) {
       this.abort(err)
-      return false
     }
   }
 
   onUpgrade (statusCode, headers, socket) {
     assert(!this.aborted)
     assert(!this.completed)
 
-    return this[kHandler].onUpgrade(statusCode, headers, socket)
+    try {
+      return this[kHandler].onUpgrade(statusCode, headers, socket)
+    } catch (err) {
+      this.abort(err)
+    }
   }
 
   onComplete (trailers) {
@@ -295,6 +302,7 @@ class Request {
     }
     this.aborted = true
 
+    // TODO (fix): What if onError throws?
     return this[kHandler].onError(error)
   }
 

diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js
@@ -325,6 +325,7 @@ class Parser {
     if (!request) {
       return -1
     }
+
     request.onResponseStarted()
   }
 
@@ -408,11 +409,7 @@ class Parser {
     client[kQueue][client[kRunningIdx]++] = null
     client.emit('disconnect', client[kUrl], [client], new InformationalError('upgrade'))
 
-    try {
-      request.onUpgrade(statusCode, headers, socket)
-    } catch (err) {
-      util.destroy(socket, err)
-    }
+    request.onUpgrade(statusCode, headers, socket)
 
     client[kResume]()
   }
@@ -556,6 +553,10 @@ class Parser {
     if (request.onData(buf) === false) {
       return constants.ERROR.PAUSED
     }
+
+    if (request.aborted) {
+      return -1
+    }
   }
 
   onMessageComplete () {
@@ -597,6 +598,10 @@ class Parser {
 
     request.onComplete(headers)
 
+    if (request.aborted) {
+      return -1
+    }
+
     client[kQueue][client[kRunningIdx]++] = null
 
     if (socket[kWriting]) {
@@ -915,22 +920,21 @@ function writeH1 (client, request) {
 
   const socket = client[kSocket]
 
-  try {
-    request.onConnect((err) => {
-      if (request.aborted || request.completed) {
-        return
-      }
+  request.onConnect((err) => {
+    if (request.aborted || request.completed) {
+      return
+    }
 
-      errorRequest(client, request, err || new RequestAbortedError())
+    errorRequest(client, request, err ?? new RequestAbortedError())
 
+    if (request.upgrade) {
+      util.destroy(socket, err ?? new RequestAbortedError('aborted'))
+    } else {
       util.destroy(socket, new InformationalError('aborted'))
-    })
-  } catch (err) {
-    errorRequest(client, request, err)
-  }
+    }
+  })
 
   if (request.aborted) {
-    util.destroy(body)
     return false
   }