Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added for non websocket usage #909

xen-HendryZheng · 2023-05-01T07:25:03Z

Checks

I understand project setup issues should be asked on StackOverflow or in GitHub Discussions.
I updated to latest http-proxy-middleware.

Describe the bug (be clear and concise)

I had an issue with my project where the memory is steadily increasing. When I turned on the profiling through chrome inspector, I get a warning of this

And when I checked and console.log the req.socket where the code of the library is creating the listener at, on every single request being made which result into invoking createHttpProxyMiddleware, it will always add an event listener to server.onClose, and the object was never cleared by the Garbage Collector.

Please see screenshot below

I've created a runner which triggered 50request at a time, and it created 50 close events without being cleared.

Step-by-step reproduction instructions

1. Turn on node --inspect on your nodejs application
2. Open chrome://inspect and make sure you are on debug mode
3. Fire 50 or more request, you shall see a warning which I've described above.

Expected behavior (be clear and concise)

Since request.connection has been deprected, hence request.socket which used in this line should only applicable for HPM with mode ws: true. For ws: false which is non websocket, should not need this kind of listener.

How is http-proxy-middleware used in your project?

****-***-*******@1.0.0 ***/***/****
├─┬ ***/***/****
│ └── [email protected] deduped
└── [email protected]

What http-proxy-middleware configuration are you using?

this.proxy = createProxyMiddleware({
            target: this.url,
            ws: false,
            pathRewrite: () => (prefix ? `${prefix}${path}` : `${path}`),
            onProxyReq: (proxyReq: ClientRequest, req: Request) => {
                proxyReq.setHeader('service-id', ServiceIdSnapApiGateway);
                if (this.xApiKey) {
                    proxyReq.setHeader('x-api-key', this.xApiKey);
                }
                if (req.body) {
                    const contentType: string = proxyReq.getHeader('Content-Type') as string;

                    let bodyData;
                    if (contentType && contentType.includes('application/json')) {
                        bodyData = JSON.stringify(req.body);
                    } else if (contentType && contentType.includes('application/x-www-form-urlencoded')) {
                        bodyData = queryString.stringify(req.body);
                    }

                    if (bodyData) {
                        proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData));
                        proxyReq.write(bodyData);
                    }
                }
            },
            onProxyRes: extendProxyRes,
            changeOrigin: true,
            logLevel: 'silent'
        });

What OS/version and node/version are you seeing the problem?

Node 18.16.0
macOS Ventura v13.2.1
Memory 8gb
Chip Apple M1

Additional context (optional)

No response

The text was updated successfully, but these errors were encountered:

taozhi8833998 · 2023-07-06T12:45:02Z

got the same issue

chimurai · 2023-07-06T20:31:22Z

Try to create the proxy once, instead of on every request.

Similar issue: #108 (comment)

Could you share what server are you using?

RadekKpc · 2023-09-20T09:05:59Z

Is anyone working on that?

tak1n · 2023-09-26T23:41:25Z

We are facing the same issue on a NestJS Application trying to leverage http-proxy-middleware through a Nest Middleware.

// proxy.middleware.ts
import {
  Inject,
  NestMiddleware,
} from '@nestjs/common';
import { Request, Response } from 'express';
import { createProxyMiddleware } from 'http-proxy-middleware';
import { ConfigurationService } from '../configuration/configuration.service';
import { randomUUID } from 'node:crypto';

export class ProxyMiddleware implements NestMiddleware {
  constructor(
    private configService: ConfigurationService,
  ) {}

  async use(req: Request, res: Response, next: () => void): Promise<any> {
    const upstreamInfo = <method to get jwt etc>;
    const correlationId = randomUUID();

    const proxy = createProxyMiddleware({
      target: this.configService.getProxyTarget(),
      headers: {
        host: this.configService.getUpstreamHost(),
        authorization: `Bearer ${upstreamInfo.jwt}`,
        'x-organization-id': upstreamInfo.organizationId,
        'x-correlation-id': correlationId,
      },
      changeOrigin: false,
      xfwd: true,
    });

    proxy(req, res, next);
  }
}

// proxy.module.ts
import {
  MiddlewareConsumer,
  Module,
  NestModule,
  RequestMethod,
} from '@nestjs/common';
import { ConfigurationModule } from '../configuration/configuration.module';
import { ProxyMiddleware } from './proxy.middleware';

@Module({
  providers: [],
  imports: [
    ConfigurationModule,
  ],
})
export class ProxyModule implements NestModule {
  configure(consumer: MiddlewareConsumer): any {
    consumer
      .apply(ProxyMiddleware)
      .exclude(
        { path: 'ready', method: RequestMethod.GET },
        { path: 'live', method: RequestMethod.GET },
      )
      .forRoutes('');
  }
}

Try to create the proxy once, instead of on every request.

Is there a way to create the proxy instance only once but still be able to dynamically set headers for the proxied request? I know of onProxyReq to manipulate requests, but having a hard time figuring out a way to use that with a Nest Middleware that uses dependencies for various required functionalities (issue JWT, etc.).

tak1n · 2023-09-27T17:19:48Z

Leaving this here for anyone facing the same issue with a NestJS application. The solution was like suggested by @chimurai to only call createProxyMiddleware once.

// proxy.middleware.ts
import {
  Inject,
  NestMiddleware,
} from '@nestjs/common';
import { Request, Response } from 'express';
import { RequestHandler, createProxyMiddleware } from 'http-proxy-middleware';
import { ConfigurationService } from '../configuration/configuration.service';
import { randomUUID } from 'node:crypto';

export class ProxyMiddleware implements NestMiddleware {
  private proxy: RequestHandler;

  constructor(
    private configService: ConfigurationService,
  ) {
    this.proxy = createProxyMiddleware({
      target: this.configService.getProxyTarget(),
      headers: {
        host: this.configService.getUpstreamHost(),
      },
      changeOrigin: false,
      xfwd: true,
    });
  }

  async use(req: Request, res: Response, next: () => void): Promise<any> {
    const upstreamInfo = <method to get jwt etc>;
    const correlationId = randomUUID();

    req.headers = {
      ...req.headers,
      authorization: `Bearer ${upstreamInfo.jwt}`,
      'x-organization-id': upstreamInfo.organizationId,
      'x-correlation-id': correlationId,
    }

    this.proxy(req, res, next);
  }
}

xen-HendryZheng changed the title ~~Memory leak detected on the EventEmitter wutg http-proxy-middleware.js where server.on('close') listener is added~~ Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added May 1, 2023

xen-HendryZheng linked a pull request May 1, 2023 that will close this issue

Fix:server on close on websocket only condition #910

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added for non websocket usage #909

Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added for non websocket usage #909

xen-HendryZheng commented May 1, 2023

taozhi8833998 commented Jul 6, 2023

chimurai commented Jul 6, 2023 •

edited

Loading

RadekKpc commented Sep 20, 2023

tak1n commented Sep 26, 2023

tak1n commented Sep 27, 2023 •

edited

Loading

Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added for non websocket usage #909

Memory leak detected on the EventEmitter with http-proxy-middleware.js where server.on('close') listener is added for non websocket usage #909

Comments

xen-HendryZheng commented May 1, 2023

Checks

Describe the bug (be clear and concise)

Step-by-step reproduction instructions

Expected behavior (be clear and concise)

How is http-proxy-middleware used in your project?

What http-proxy-middleware configuration are you using?

What OS/version and node/version are you seeing the problem?

Additional context (optional)

taozhi8833998 commented Jul 6, 2023

chimurai commented Jul 6, 2023 • edited Loading

RadekKpc commented Sep 20, 2023

tak1n commented Sep 26, 2023

tak1n commented Sep 27, 2023 • edited Loading

chimurai commented Jul 6, 2023 •

edited

Loading

tak1n commented Sep 27, 2023 •

edited

Loading