Skip to content

Commit

Permalink
remove all fetchParam event handlers (nodejs#2823)
Browse files Browse the repository at this point in the history
  • Loading branch information
KhafraDev authored and crysmags committed Feb 23, 2024
1 parent e7a1ec8 commit 5edd986
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
11 changes: 5 additions & 6 deletions lib/web/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,6 @@ class Fetch extends EE {
this.connection = null
this.dump = false
this.state = 'ongoing'
// 2 terminated listeners get added per request,
// but only 1 gets removed. If there are 20 redirects,
// 21 listeners will be added.
// See https://github.com/nodejs/undici/issues/1711
// TODO (fix): Find and fix root cause for leaked listener.
this.setMaxListeners(21)
}

terminate (reason) {
Expand Down Expand Up @@ -2046,6 +2040,7 @@ async function httpNetworkFetch (
// 19. Run these steps in parallel:

// 1. Run these steps, but abort when fetchParams is canceled:
fetchParams.controller.onAborted = onAborted
fetchParams.controller.on('terminated', onAborted)
fetchParams.controller.resume = async () => {
// 1. While true
Expand Down Expand Up @@ -2314,6 +2309,10 @@ async function httpNetworkFetch (
fetchParams.controller.off('terminated', this.abort)
}

if (fetchParams.controller.onAborted) {
fetchParams.controller.off('terminated', fetchParams.controller.onAborted)
}

fetchParams.controller.ended = true

this.body.push(null)
Expand Down
33 changes: 33 additions & 0 deletions test/fetch/issue-1711.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict'

const assert = require('node:assert')
const { once } = require('node:events')
const { createServer } = require('node:http')
const { test } = require('node:test')
const { fetch } = require('../..')

test('Redirecting a bunch does not cause a MaxListenersExceededWarning', async (t) => {
let redirects = 0

const server = createServer((req, res) => {
if (redirects === 15) {
res.end('Okay goodbye')
return
}

res.writeHead(302, {
Location: `/${redirects++}`
})
res.end()
}).listen(0)

t.after(server.close.bind(server))
await once(server, 'listening')

process.emitWarning = assert.bind(null, false)

const url = `http://localhost:${server.address().port}`
const response = await fetch(url, { redirect: 'follow' })

assert.deepStrictEqual(response.url, `${url}/${redirects - 1}`)
})

0 comments on commit 5edd986

Please sign in to comment.