Skip to content

redirecting >11 times causes "MaxListenersExceededWarning" #1711

Description

@KhafraDev

Bug Description

Each redirect adds one terminated listener to fetchParams and it's never removed.

Reproducible By

import { once } from 'events'
import { createServer } from 'http'
import { fetch } from 'undici'

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)

await once(server, 'listening')

const url = `http://localhost:${server.address().port}`

fetch(url, { redirect: 'follow' })

Expected Behavior

Logs & Screenshots

MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 terminated listeners added to [Fetch]. Use emitter.setMaxListeners() to increase limit

Environment

Additional context

Activity

  1. jonseymour commented on Feb 23, 2024

    @jonseymour

    Right, but if you have to lift the warning so high that it masks actual leaks you cease to get any benefit from the warning since if there is an actual leak in one of these event sources the warning never fires or if does fires once due a spurious condition then, when an actual leak occurs later, the code fails to emit a second warning because it has already emitted a spurious warning.

    Maybe the most pragmatic thing would be to place a rate limit on the number of warnings, but ensure they are issued periodically along with the current number of listeners. That way, an actual leak could be observed (by observing a steady increase in the watermark) without necessarily polluting the logs with too many spurious warnings.

    edit: of course, this would likely an upstream change.

  2. Uzlopak commented on Feb 23, 2024

    @Uzlopak
    Contributor

    The problem is solved by @KhafraDev. You can open an issue in node core and talk there about more strategies to improve this.

    From the undici perspective the bug will be gone when #2823 is merged.

  3. jonseymour commented on Feb 23, 2024

    @jonseymour

    Ok, good to see the root cause is being addressed now, thanks.

  4. daveyarwood commented on Apr 15, 2024

    @daveyarwood

    I've been running into what I think is the same or similar issue to what was reported in this issue - lots of MaxListenersExceededWarnings being logged, and my service eventually runs out of memory and dies. When I run with --trace-warnings, the stacktrace points to Undici.

    It looks like the fix (#2823) was released in Undici 6.7.0, and I was hopeful that upgrading the dependency in my project would fix my issue. But after updating Undici to 6.13.0, I'm still getting a lot of MaxListenersExceededWarnings:

    (node:14480) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 101 abort listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit
        at _addListener (node:events:588:17)
        at EventEmitter.addListener (node:events:606:10)
        at addAbortListener (/Users/daveyarwood/code/spark/node_modules/undici/lib/core/util.js:443:10)
        at addSignal (/Users/daveyarwood/code/spark/node_modules/undici/lib/api/abort-signal.js:36:3)
        at new RequestHandler (/Users/daveyarwood/code/spark/node_modules/undici/lib/api/api-request.js:66:5)
        at Pool.request (/Users/daveyarwood/code/spark/node_modules/undici/lib/api/api-request.js:171:25)
        at /Users/daveyarwood/code/spark/node_modules/undici/lib/api/api-request.js:164:15
        at new Promise (<anonymous>)
        at Pool.request (/Users/daveyarwood/code/spark/node_modules/undici/lib/api/api-request.js:163:12)
    
  5. daveyarwood commented on Apr 15, 2024

    @daveyarwood

    Another note: I did try increasing the default max event listeners to a high number:

    EventEmitter.defaultMaxListeners = 5000

    And I'm still seeing the warnings come up. Granted, my service is doing a lot of work in parallel (many promises being fired off), so I'm not sure if the problem might be user error. Are there best practices about using this library in many promises concurrently?

  6. daveyarwood commented on Apr 15, 2024

    @daveyarwood

    For context, I am using undici transitively via Elasticsearch.

  7. KhafraDev commented on Apr 15, 2024

    @KhafraDev
    MemberAuthor

    It doesn't look like elasticsearch is using fetch, which is what #2823 fixed the issue for. I would recommend opening up a new issue for this and, if possible, provide a minimal reproducible example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions