fixup

nodejs · Sep 22, 2024 · 4736a1b · 4736a1b
1 parent 33172c3
commit 4736a1b
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 19 deletions.
diff --git a/lib/handler/decorator-handler.js b/lib/handler/decorator-handler.js
@@ -30,17 +30,21 @@ module.exports = class DecoratorHandler {
 
   onRequestStart (reserved, abort) {
     if (this.#handler.onRequestStart) {
-      this.#handler.onRequestStart(null, abort)
+      this.#handler.onRequestStart(reserved, abort)
+    }
+
+    if (this.#handler.onConnect) {
+      this.#handler.onConnect(abort)
     }
   }
 
   onResponseStart (resume) {
-    this.#resume = resume
-
     if (this.#handler.onResponseStart) {
       return this.#handler.onResponseStart(resume)
     }
 
+    this.#resume = resume
+
     return true
   }
 
@@ -49,8 +53,8 @@ module.exports = class DecoratorHandler {
       this.#handler.onResponseHeaders(headers, statusCode)
     }
 
-    if (this.#handler.onConnect) {
-      this.#handler.onConnect(statusCode, toRawHeaders(headers), this.#resume)
+    if (this.#handler.onHeaders) {
+      this.#handler.onHeaders(statusCode, toRawHeaders(headers), this.#resume)
     }
 
     return true
@@ -136,21 +140,27 @@ module.exports = class DecoratorHandler {
     assert(!this.#onCompleteCalled)
     assert(!this.#onErrorCalled)
 
-    let paused
+    let ret
 
     if (this.#handler.onResponseStart) {
-      paused ||= this.#handler.onResponseStart(args[2]) === false
+      if (this.#handler.onResponseStart(args[2]) === false) {
+        // TODO (fix): Strictly speaking we should not call onRepsonseHeaders
+        // after this...
+        ret = false
+      }
     }
 
     if (this.#handler.onResponseHeaders) {
-      paused ||= this.#handler.onResponseHeaders(util.parseHeaders(args[1]), args[0]) === false
+      if (this.#handler.onResponseHeaders(util.parseHeaders(args[1]), args[0]) === false) {
+        ret = false
+      }
     }
 
     if (this.#handler.onHeaders) {
-      paused ||= this.#handler.onHeaders(...args) === false
+      ret ??= this.#handler.onHeaders(...args)
     }
 
-    return paused
+    return ret
   }
 
   onData (...args) {

diff --git a/test/decorator-handler.js b/test/decorator-handler.js
@@ -39,6 +39,32 @@ describe('DecoratorHandler', () => {
       t.equal(typeof decorator[method], 'function')
     })
 
+    test(`should delegate ${method}-method`, (t) => {
+      t = tspl(t, { plan: 1 })
+      const handler = { [method]: () => method }
+      const decorator = new DecoratorHandler(handler)
+      t.equal(decorator[method](), method)
+    })
+
+    test(`should delegate ${method}-method with arguments`, (t) => {
+      t = tspl(t, { plan: 1 })
+      const handler = { [method]: (...args) => args }
+      const decorator = new DecoratorHandler(handler)
+      t.deepStrictEqual(decorator[method](1, 2, 3), [1, 2, 3])
+    })
+
+    test(`can be extended and should delegate ${method}-method`, (t) => {
+      t = tspl(t, { plan: 1 })
+
+      class ExtendedHandler extends DecoratorHandler {
+        [method] () {
+          return method
+        }
+      }
+      const decorator = new ExtendedHandler({})
+      t.equal(decorator[method](), method)
+    })
+
     test(`calling the method ${method}-method should not throw if the method is not defined in the handler`, (t) => {
       t = tspl(t, { plan: 1 })
       const decorator = new DecoratorHandler({})

diff --git a/test/issue-2065.js b/test/issue-2065.js
@@ -21,9 +21,8 @@ test('undici.request with a FormData body should set content-length header', asy
   const body = new FormData()
   body.set('file', new File(['abc'], 'abc.txt'))
 
-  const res = await request(`http://localhost:${server.address().port}`, {
+  await request(`http://localhost:${server.address().port}`, {
     method: 'POST',
     body
   })
-  await res.body.dump()
 })
diff --git a/test/stream-compat.js b/test/stream-compat.js
@@ -72,7 +72,6 @@ test('IncomingMessage', async (t) => {
         body: 'hello world'
       }, (err, data) => {
         t.ifError(err)
-        data.body.resume()
       })
     })
   })

diff --git a/types/dispatcher.d.ts b/types/dispatcher.d.ts
@@ -214,12 +214,12 @@ declare namespace Dispatcher {
   }
   export type StreamFactory<TOpaque = null> = (data: StreamFactoryData<TOpaque>) => Writable
   export interface DispatchHandlers {
-    onRequestStart(reserved: null, abort: (err?: Error) => void): void;
-    onResponseStart(resume: () => void): boolean;
-    onResponseHeaders(headers: Record<string, string>, statusCode: number): boolean;
-    onResponseData(chunk: Buffer): boolean;
-    onResponseEnd(trailers: Record<string, string>): void;
-    onResponseError(err: Error) : void;
+    onRequestStart?(reserved: null, abort: (err?: Error) => void): void;
+    onResponseStart?(resume: () => void): boolean;
+    onResponseHeaders?(headers: Record<string, string>, statusCode: number): boolean;
+    onResponseData?(chunk: Buffer): boolean;
+    onResponseEnd?(trailers: Record<string, string>): void;
+    onResponseError?(err: Error) : void;
     /** @deprecated Invoked before request is dispatched on socket. May be invoked multiple times when a request is retried when the request at the head of the pipeline fails. */
     onConnect?(abort: (err?: Error) => void): void;
     /** @deprecated Invoked when an error has occurred. */