Creating util function to validate handlers

nodejs · Jul 20, 2021 · e3546d8 · e3546d8
1 parent ef788ee
commit e3546d8
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 41 deletions.
diff --git a/lib/agent.js b/lib/agent.js
@@ -96,12 +96,12 @@ class Agent extends Dispatcher {
 
   dispatch (opts, handler) {
     if (!handler || typeof handler !== 'object') {
-      throw new InvalidArgumentError('handler')
+      throw new InvalidArgumentError('handler must be an object.')
     }
 
     try {
       if (!opts || typeof opts !== 'object') {
-        throw new InvalidArgumentError('opts must be a object.')
+        throw new InvalidArgumentError('opts must be an object.')
       }
 
       let key

diff --git a/lib/client.js b/lib/client.js
@@ -245,12 +245,12 @@ class Client extends Dispatcher {
 
   dispatch (opts, handler) {
     if (!handler || typeof handler !== 'object') {
-      throw new InvalidArgumentError('handler')
+      throw new InvalidArgumentError('handler must be an object')
     }
 
     try {
       if (!opts || typeof opts !== 'object') {
-        throw new InvalidArgumentError('opts must be a object.')
+        throw new InvalidArgumentError('opts must be an object.')
       }
 
       if (this[kDestroyed]) {
@@ -1093,7 +1093,7 @@ function connect (client) {
   let { host, hostname, protocol, port } = client[kUrl]
 
   // Resolve ipv6
-  if (hostname.startsWith('[')) {
+  if (hostname[0] === '[') {
     const idx = hostname.indexOf(']')
 
     assert(idx !== -1)

diff --git a/lib/core/request.js b/lib/core/request.js
@@ -97,35 +97,7 @@ class Request {
       throw new InvalidArgumentError('headers must be an object or an array')
     }
 
-    if (typeof handler.onConnect !== 'function') {
-      throw new InvalidArgumentError('invalid onConnect method')
-    }
-
-    if (typeof handler.onError !== 'function') {
-      throw new InvalidArgumentError('invalid onError method')
-    }
-
-    if (typeof handler.onBodySent !== 'function' && handler.onBodySent !== undefined) {
-      throw new InvalidArgumentError('invalid onBodySent method')
-    }
-
-    if (this.upgrade || this.method === 'CONNECT') {
-      if (typeof handler.onUpgrade !== 'function') {
-        throw new InvalidArgumentError('invalid onUpgrade method')
-      }
-    } else {
-      if (typeof handler.onHeaders !== 'function') {
-        throw new InvalidArgumentError('invalid onHeaders method')
-      }
-
-      if (typeof handler.onData !== 'function') {
-        throw new InvalidArgumentError('invalid onData method')
-      }
-
-      if (typeof handler.onComplete !== 'function') {
-        throw new InvalidArgumentError('invalid onComplete method')
-      }
-    }
+    util.validateHandler(handler, method, upgrade)
 
     this.servername = util.getServerName(this.host)
 

diff --git a/lib/core/util.js b/lib/core/util.js
@@ -188,6 +188,42 @@ function isBuffer (buffer) {
   return buffer instanceof Uint8Array || Buffer.isBuffer(buffer)
 }
 
+function validateHandler (handler, method, upgrade) {
+  if (!handler || typeof handler !== 'object') {
+    throw new InvalidArgumentError('handler must be an object')
+  }
+
+  if (typeof handler.onConnect !== 'function') {
+    throw new InvalidArgumentError('invalid onConnect method')
+  }
+
+  if (typeof handler.onError !== 'function') {
+    throw new InvalidArgumentError('invalid onError method')
+  }
+
+  if (typeof handler.onBodySent !== 'function' && handler.onBodySent !== undefined) {
+    throw new InvalidArgumentError('invalid onBodySent method')
+  }
+
+  if (upgrade || method === 'CONNECT') {
+    if (typeof handler.onUpgrade !== 'function') {
+      throw new InvalidArgumentError('invalid onUpgrade method')
+    }
+  } else {
+    if (typeof handler.onHeaders !== 'function') {
+      throw new InvalidArgumentError('invalid onHeaders method')
+    }
+
+    if (typeof handler.onData !== 'function') {
+      throw new InvalidArgumentError('invalid onData method')
+    }
+
+    if (typeof handler.onComplete !== 'function') {
+      throw new InvalidArgumentError('invalid onComplete method')
+    }
+  }
+}
+
 module.exports = {
   nop,
   parseOrigin,
@@ -203,5 +239,6 @@ module.exports = {
   destroy,
   bodyLength,
   deepClone,
-  isBuffer
+  isBuffer,
+  validateHandler
 }
diff --git a/lib/handler/redirect.js b/lib/handler/redirect.js
@@ -12,6 +12,8 @@ class RedirectHandler {
       throw new InvalidArgumentError('maxRedirections must be a positive number')
     }
 
+    util.validateHandler(handler, opts.method, opts.upgrade)
+
     this.dispatcher = dispatcher
     this.location = null
     this.abort = null

diff --git a/lib/pool.js b/lib/pool.js
@@ -162,7 +162,7 @@ class Pool extends Dispatcher {
 
   dispatch (opts, handler) {
     if (!handler || typeof handler !== 'object') {
-      throw new InvalidArgumentError('handler')
+      throw new InvalidArgumentError('handler must be an object')
     }
 
     try {

diff --git a/test/client-dispatch.js b/test/client-dispatch.js
@@ -18,7 +18,7 @@ test('dispatch invalid opts', (t) => {
     }, null)
   } catch (err) {
     t.type(err, errors.InvalidArgumentError)
-    t.equal(err.message, 'handler')
+    t.equal(err.message, 'handler must be an object')
   }
 
   try {
@@ -29,7 +29,7 @@ test('dispatch invalid opts', (t) => {
     }, 'asd')
   } catch (err) {
     t.type(err, errors.InvalidArgumentError)
-    t.equal(err.message, 'handler')
+    t.equal(err.message, 'handler must be an object')
   }
 
   client.dispatch({

diff --git a/test/mock-agent.js b/test/mock-agent.js
@@ -92,7 +92,7 @@ test('MockAgent - get', t => {
 })
 
 test('MockAgent - dispatch', t => {
-  t.plan(2)
+  t.plan(3)
 
   t.test('should call the dispatch method of the MockPool', (t) => {
     t.plan(1)
@@ -116,7 +116,8 @@ test('MockAgent - dispatch', t => {
     }, {
       onHeaders: (_statusCode, _headers, resume) => resume(),
       onData: () => {},
-      onComplete: () => {}
+      onComplete: () => {},
+      onError: () => {}
     }))
   })
 
@@ -142,9 +143,93 @@ test('MockAgent - dispatch', t => {
     }, {
       onHeaders: (_statusCode, _headers, resume) => resume(),
       onData: () => {},
-      onComplete: () => {}
+      onComplete: () => {},
+      onError: () => {}
     }))
   })
+
+  t.test('should throw if handler is not valid on redirect', (t) => {
+    t.plan(7)
+
+    const baseUrl = 'http://localhost:9999'
+
+    const mockAgent = new MockAgent()
+    t.teardown(mockAgent.close.bind(mockAgent))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: 'INVALID'
+    }), new InvalidArgumentError('invalid onError method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: 'INVALID'
+    }), new InvalidArgumentError('invalid onConnect method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: () => {},
+      onBodySent: 'INVALID'
+    }), new InvalidArgumentError('invalid onBodySent method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'CONNECT'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: () => {},
+      onBodySent: () => {},
+      onUpgrade: 'INVALID'
+    }), new InvalidArgumentError('invalid onUpgrade method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: () => {},
+      onBodySent: () => {},
+      onHeaders: 'INVALID'
+    }), new InvalidArgumentError('invalid onHeaders method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: () => {},
+      onBodySent: () => {},
+      onHeaders: () => {},
+      onData: 'INVALID'
+    }), new InvalidArgumentError('invalid onData method'))
+
+    t.throws(() => mockAgent.dispatch({
+      origin: baseUrl,
+      path: '/foo',
+      method: 'GET'
+    }, {
+      onError: (err) => { throw err },
+      onConnect: () => {},
+      onBodySent: () => {},
+      onHeaders: () => {},
+      onData: () => {},
+      onComplete: 'INVALID'
+    }), new InvalidArgumentError('invalid onComplete method'))
+  })
 })
 
 test('MockAgent - .close should clean up registered pools', async (t) => {