search

tsedio / tsed

:triangular_ruler: Ts.ED is a Node.js and TypeScript framework on top of Express to write your application with TypeScript (or ES6). It provides a lot of decorators and guideline to make your code more readable and less error-prone. ⭐️ Star to support our work!
https://tsed.io/
MIT License
2.83k stars 286 forks source link

[BUG] Testing with jest using more and more heap memory. #2253

Closed Krzysztof-Nerwinski closed 1 year ago

Krzysztof-Nerwinski commented 1 year ago

Information

First I'd like to thank everyone involved in this very cool framework. The reason I'm createing this issue is because me and my team stumbled on a problem after adding ~500 tests most of them intergration tests.

We're getting the following error in our pipelines: FATAL ERROR: MarkCompactCollector: young object promotion failed Allocation failed - JavaScript heap out of memory.

To test if same issue occurs on a new project I've created an unmodofied projectwith tsed init . command. Below are the options I've chosen: ? Choose the target platform: Express.js ? Choose the architecture for your project: Ts.ED ? Choose the convention file styling: Ts.ED ? Check the features needed for your project Swagger, Testing, Linter, Bundler ? Choose unit framework Jest ? Choose linter tools framework EsLint ? Choose extra linter tools ? Choose your bundler (Use arrow keys) ❯ Babel

After installing components-scan I've tried running node --expose-gc ./node_modules/.bin/jest --logHeapUsage --detect-leaks without any modification to tests created on init.

It returns that the default tests are leaking:

EXPERIMENTAL FEATURE!
    Your test suite is leaking memory. Please ensure all references are cleaned.

    There is a number of things that can leak memory:
      - Async operations that have not finished (e.g. fs.readFile).
      - Timers not properly mocked (e.g. setInterval, setTimeout).
      - Keeping references to the global scope.

      at onResult (node_modules/@jest/core/build/TestScheduler.js:150:18)
      at node_modules/@jest/core/build/TestScheduler.js:254:19
      at node_modules/emittery/index.js:363:13
          at Array.map (<anonymous>)
      at Emittery.emit (node_modules/emittery/index.js:361:23)

Even without --detect-leaks you can observe heap size growing with every test that bootstraps Server application.

 PASS  src/controllers/rest/HelloWorldController.spec.ts (160 MB heap size)
 PASS  src/Server.integration.spec.ts (209 MB heap size)
 PASS  src/controllers/rest/HelloWorldController.integration.spec.ts (209 MB heap size)

Do you know about this issue and maybe have a solution we could implement?

Romakita commented 1 year ago

Hello @Krzysztof-Nerwinski

thanks for this issue.

Ts.ED use a lot of integration test but I haven’t constated that. But it’s totally possible. Memory leaks in express app is easy to create it but complicated to solve it.

I haven’t a lot of experience to found memory leaks. I already tried to optimize the framework as is it possible but it’s very complicated. For this issue I need help.

I’ll be happy to help and give the all details needed around the framework architecture and solve this issue.

I’ll make this issue as top priority!

See yoy

Romakita commented 1 year ago

https://github.com/Romakita/tsed-jest-memory-leaks

Romakita commented 1 year ago

Interesting point. With Koa, Leaks isn't presents: https://github.com/Romakita/tsed-jest-memory-leaks-koa

So the problem is around the PlatformExpress.ts :D

Krzysztof-Nerwinski commented 1 year ago

Nice, that narrows it down, thanks!

I didn't have much time last couple of days. I tried debugging it with chrome inspector but no luck for now.

Romakita commented 1 year ago

It seems to be related to the runInContext function combined with Express.js here (https://github.com/tsedio/tsed/tree/production/packages/platform/platform-express/src/components/PlatformExpress.ts):

class PlatformExpress {
  useContext(): this {
    const app = this.getPlatformApplication();
    const invoke = createContext(this.injector);

    this.injector.logger.debug("Mount app context");

    app.use(async (request: any, response: any, next: any) => {
      const $ctx = await invoke({request, response});
      await $ctx.start();

      $ctx.response.getRes().on("finish", () => $ctx.finish());

// if we send response immediately here, there is no memory leaks:
// response.send("hello") 

// when it's called, the memory leaks occurs
      return runInContext($ctx, next);
// Note:
// - maybe the memory leaks is under the handlers executed by runInContext? 
// - runInContext in Koa run handlers also and haven't memory leaks
    });

    return this;
  }
}

Here the equivalent in PlatformKoa (https://github.com/tsedio/tsed/tree/production/packages/platform/platform-koa/src/components/PlatformKoa.ts):

  useContext(): this {
    const app = this.getPlatformApplication();
    const invoke = createContext(this.injector);
    const platformExceptions = this.injector.get<PlatformExceptions>(PlatformExceptions);

    this.injector.logger.debug("Mount app context");

    app.use(async (koaContext: Context, next: Next) => {
      const $ctx = await invoke({
        request: koaContext.request as any,
        response: koaContext.response as any,
        koaContext
      });

      return runInContext($ctx, async () => {
        try {
          await $ctx.start();
          await next();
          const status = koaContext.status || 404;

          if (status === 404 && !$ctx.isDone()) {
            platformExceptions?.resourceNotFound($ctx);
          }
        } catch (error) {
          platformExceptions?.catch(error, $ctx);
        } finally {
          await $ctx.finish();
        }
      });
    });

    return this;
  }

In koa, the next function returning a promise will in express next return nothing. So cleaning cleaning resource is more simple in Koa.

Also Ts.ED create circular ref between PlatformContext/PlatformRequest/PlatformResponse (like Koa do).

that's all i've analyzed so far ^^ See you

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 year ago

🎉 Are you happy?

If you appreciated the support, know that it is free and is carried out on personal time ;)

A support, even a little bit makes a difference for me and continues to bring you answers!

github opencollective