Skip to content

Memory leak when using fetch and not consuming the response body #51162

Closed

Description

@mcat95

Version

v21.4.0

Platform

Linux 6.6.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 14 Dec 2023 03:45:42 +0000 x86_64 GNU/Linux

Subsystem

fetch API

What steps will reproduce the bug?

Execute the following sample script with node --expose-gc:

const repro = async () => {
  for (let i = 0; i < 1000; i++) {
    await fetch('https://raw.githubusercontent.com/nodejs/node/main/doc/changelogs/CHANGELOG_V20.md');
    console.log(process.memoryUsage().rss);
  }
}

repro()
  .then(() => console.log('Finished'))
  .catch(err => console.log('Error', err));

How often does it reproduce? Is there a required condition?

It always happen

What is the expected behavior? Why is that the expected behavior?

The logged memory usage of the process should be consistent and not increase over every execution of the fetch

What do you see instead?

The memory usage of the process increases with every request, and it's not ever garbage collected (Even with calling global.gc() on every execution.

Additional information

If you add a .then(r => r.text()) or something else that uses the body, the memory leak goes away. I found a similar issue in the node-fetch library (node-fetch/node-fetch#83), but I wasn't able to find this leak in nodejs itself.

For a workaround, I found that using a Finalization Register, I can create a wrapper function that detects evictions of unneeded response objects, and, if the body was unused, empty the body like this:

const registry = new FinalizationRegistry(response => {
  if (!response || response.bodyUsed) return;
  response.arrayBuffer().catch(console.log);
});

Activity

  1. marco-ippolito commented on Dec 15, 2023

    @marco-ippolito
    Member

    cc @nodejs/undici

  2. ronag commented on Dec 15, 2023

    @ronag
    Member
  3. cxcorp commented on Aug 28, 2024

    @cxcorp

    In case anyone else bumps into this, it appears that this behavior has changed and since v20.16.0 the body stream gets closed when the Response is garbage collected. This change was in Undici v6.16.0 via nodejs/undici#3199

  4. Juraj-Masiar commented on Jan 29, 2025

    @Juraj-Masiar

    I'm tracking down a memory leak (I'm using Node v22.13.1) and I'm a bit confused about this fetch behavior.
    Does this mean, that I need to consume also failed requests (when response.ok is false)?

    Let's say I have this helper method for downloading images:

    private async downloadImageFromUrl(imageUrl: string): Promise<Buffer | undefined> {
      try {
        const response = await fetch(imageUrl, getServerFetchOptions());
        if (response.ok) {
          this.logger.log(`image OK status: ${response.status}, Type: ${response.headers.get('content-type')}, Size: ${response.headers.get('content-length')}, ${imageUrl}`);
          return Buffer.from(await response.arrayBuffer());
        }
        this.logger.log(`image [${response.status}]: ${imageUrl}`);
      }
      catch (error) {
        this.logger.log(`image [net err]: ${imageUrl}, ${error instanceof Error ? error.message : error}`);
      }
    }

    How do I fix it?
    If I try to add this code:

    if (response.body) {
      for await (const chunk of response.body) {
        // force consumption of body
      }
    }

    TypeScript will complain:

    TS2504: Type ReadableStream<Uint8Array<ArrayBufferLike>> must have a [Symbol. asyncIterator]() method that returns an async iterator.
    

    Maybe I should rather use:

    await response.bytes();

    Again, I'm super confused about this memory leak...
    Please advice, thank you :)

    UPDATE:
    So, I've captured .heapsnapshot and analyzed it (to the best of my limited abilities), and it looks like it's the fetch API that's leaking:

    Image

    I would guess it wasn't merged into v22.13.1 yet, or maybe the fix doesn't cover my use-cases? (I have some chained fetches).
    In any case, I'm now testing a workaround, where I'm calling this after each fetch:

    const arrayBuffer = await response.arrayBuffer().catch(() => {});

    UPDATE 2:
    Consuming body in all cases didn't helped, the memory leak was still there.
    I've also tried to cancel the request with response.body.cancel(), but the leak (~100MB per day) was still there.
    After trying everything, I've fixed it by using node-fetch:

    import fetch from 'node-fetch';

    Now the memory leak is completely gone - even if I don't consume the body.

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

    fetchIssues and PRs related to the Fetch API.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions