fix(1270): throw descriptive error when opts dispatcher passed instance methods (#5007)

Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>

Author: rozzilla

index.js

@@ -105,14 +105,14 @@ function makeDispatcher (fn) {
       url = util.parseURL(url)
     }
 
-    const { agent, dispatcher = getGlobalDispatcher() } = opts
+    const { agent, dispatcher = getGlobalDispatcher(), ...restOpts } = opts
 
     if (agent) {
       throw new InvalidArgumentError('unsupported opts.agent. Did you mean opts.client?')
     }
 
     return fn.call(dispatcher, {
-      ...opts,
+      ...restOpts,
       origin: url.origin,
       path: url.search ? `${url.pathname}${url.search}` : url.pathname,
       method: opts.method || (opts.body ? 'PUT' : 'GET')

lib/dispatcher/dispatcher-base.js

@@ -138,6 +138,10 @@ class DispatcherBase extends Dispatcher {
         throw new InvalidArgumentError('opts must be an object.')
       }
 
+      if (opts.dispatcher) {
+        throw new InvalidArgumentError('opts.dispatcher is not supported by instance methods. Pass opts.dispatcher to the top-level undici functions or call the dispatcher instance method directly.')
+      }
+
       if (this[kDestroyed] || this[kOnDestroyed]) {
         throw new ClientDestroyedError()
       }

test/issue-1270.js

@@ -0,0 +1,110 @@
+'use strict'
+
+const { tspl } = require('@matteo.collina/tspl')
+const { test, after } = require('node:test')
+const { createServer } = require('node:http')
+const { once } = require('node:events')
+const {
+  Pool,
+  Client,
+  Agent,
+  request,
+  errors
+} = require('..')
+
+// https://github.com/nodejs/undici/issues/1270
+
+test('Pool.request() throws if opts.dispatcher is provided', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+    res.end('ok')
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  after(async () => {
+    server.close()
+    await once(server, 'close')
+  })
+
+  const pool = new Pool(`http://localhost:${server.address().port}`)
+  after(() => pool.close())
+
+  const otherAgent = new Agent()
+  after(() => otherAgent.close())
+
+  try {
+    await pool.request({
+      path: '/',
+      method: 'GET',
+      dispatcher: otherAgent
+    })
+    t.fail('should have thrown')
+  } catch (err) {
+    t.ok(err instanceof errors.InvalidArgumentError)
+    t.ok(err.message.includes('opts.dispatcher is not supported'))
+  }
+})
+
+test('Client.request() throws if opts.dispatcher is provided', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+    res.end('ok')
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  after(async () => {
+    server.close()
+    await once(server, 'close')
+  })
+
+  const client = new Client(`http://localhost:${server.address().port}`)
+  after(() => client.close())
+
+  const otherPool = new Pool(`http://localhost:${server.address().port}`)
+  after(() => otherPool.close())
+
+  try {
+    await client.request({
+      path: '/',
+      method: 'GET',
+      dispatcher: otherPool
+    })
+    t.fail('should have thrown')
+  } catch (err) {
+    t.ok(err instanceof errors.InvalidArgumentError)
+    t.ok(err.message.includes('opts.dispatcher is not supported'))
+  }
+})
+
+test('Top-level request() still works with opts.dispatcher', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
+    res.end('hello')
+  })
+
+  server.listen(0)
+  await once(server, 'listening')
+
+  const pool = new Pool(`http://localhost:${server.address().port}`)
+
+  after(async () => {
+    await pool.close()
+    server.close()
+    await once(server, 'close')
+  })
+
+  const { statusCode, body } = await request(`http://localhost:${server.address().port}`, {
+    method: 'GET',
+    dispatcher: pool
+  })
+
+  t.equal(statusCode, 200)
+  t.equal(await body.text(), 'hello')
+})