Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open handle when using http.Server.listen #8554

Closed
remcohaszing opened this issue Jun 12, 2019 · 10 comments · Fixed by #11429
Closed

Open handle when using http.Server.listen #8554

remcohaszing opened this issue Jun 12, 2019 · 10 comments · Fixed by #11429

Comments

@remcohaszing
Copy link
Contributor

🐛 Bug Report

I think I ran into ladjs/supertest#520. Trying to work around this, I got to a minimal example, which I think is actually a bug in Jest.

Jest detects open handles when an http server is started, even when it is closed afterwards.

To Reproduce

Create the following zero-configuration test case.

const http = require("http");

it("should not timeout", async () => {
  const server = http.createServer();
  await new Promise(resolve => {
    server.listen(() => {
      resolve();
    });
  });
  await new Promise((resolve, reject) => {
    server.close(err => {
      if (err) {
        reject(err);
      } else {
        resolve();
      }
    });
  });
  // await new Promise(resolve => setTimeout(resolve, 0));
});

When jest is run using --detectOpenHandles, this will output the following:

$ jest --detectOpenHandles
 PASS  __tests__/index.js
  ✓ should not timeout (6ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.231s
Ran all test suites.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  TCPSERVERWRAP

      4 |   const server = http.createServer();
      5 |   await new Promise(resolve => {
    > 6 |     server.listen(() => {
        |            ^
      7 |       resolve();
      8 |     });
      9 |   });

      at listen (__tests__/index.js:6:12)
      at Object.<anonymous>.it (__tests__/index.js:5:9)

However, when the “0 ms sleep” line at the end is uncommented, no open handles are detected.

Expected behavior

Jest should not detect any open handles.

Run npx envinfo --preset jest

  System:
    OS: Linux 4.15 Ubuntu 18.04.2 LTS (Bionic Beaver)
    CPU: (8) x64 Intel(R) Core(TM) i7-4720HQ CPU @ 2.60GHz
  Binaries:
    Node: 10.16.0 - /usr/bin/node
    Yarn: 1.16.0 - /usr/bin/yarn
    npm: 6.9.0 - ~/.local/bin/npm
  npmPackages:
    jest: ^24.8.0 => 24.8.0 
@EasonWang01
Copy link

EasonWang01 commented Jun 14, 2019

Close the listening instance can solve the problem, hope this help.

server.js

const server = app.listen(port, function (err) {
  if (err) {
    console.log(err);
  } else {
    console.log('Listening on port ' + port);
  }
});
module.exports = {
  server
};

test.js

const { server } = require('../index');
afterAll(async () => {
  server.close();
});

@gilles-yvetot
Copy link
Contributor

gilles-yvetot commented Sep 12, 2019

@EasonWang01 I am doing this but still got the same error

My only way of solving that was to do this:

afterAll(done => {
    httpServer.close(() => {
      setTimeout(done, 100)
    })
  })

@dotku
Copy link

dotku commented Mar 25, 2020

I have tried all the solutions above, I still get the error:

Jest has detected the following 1 open handle potentially keeping Jest from exiting

anyone with good luck?

@remcohaszing
Copy link
Contributor Author

I switched from supertest to axios-test-instance (written by me). I haven’t ran into this issue since then.

@nhebling
Copy link

nhebling commented May 1, 2020

Somehow I run into the problem that jest "detects" or "has" open handles when using mongo-memory-server and supertest.
I tried various options and for the mongo-memory-server I'm able to just avoid the one test where this problem occurs, but for the tests with supertest it's not possible.
Selecting another framework then supertest, could be a temporary solution, but then I run into the problem at another point. Calling "done" (jest) or "end" (supertest) or completly rewritting the test does not help.
Currently I miss some documentation or feature to help me identifying the root cause why jest does not finish the test. Proposals like "just call --forceExit" or "use another framework" do not solve the root cause.

With my supertest problem, e.g. I tried calling "end" but then supertest says that it's already closed - but jest still thinks there is something open - I just want to understand at which point in time or which part is having the problem. So my "wish" would be to add some support in jest to be able dive deeper into the details.

@Mr0grog
Copy link
Contributor

Mr0grog commented May 5, 2021

Ran into this today and spent some time diving in — it looks like there are 3 intertwined issues here, some in Jest and some in Supertest:

  1. Supertest does not wait for the server is starts to begin listening. It turns out that, if you don’t wait for the callback in server.listen(), the server resources will not have been entirely torn down by the time you receive the callback from server.close() (which Supertest does wait for). This was news to me, and is why you need to add the somewhat long timeout.

  2. If an async resource is destroyed (e.g. closing the server) in the same JS task (even if different microtask, it seems), the async_hooks module won’t have time to call its hooks. If you rsolve issue (1) above, for example, Jest still needs a timeout to catch that the server was destroyed (but it can be an ultra-short timeout of 0 ms). It might be good if Jest waited a tick either when reporting/cleaning up the async hooks: https://github.com/facebook/jest/blob/64d5983d20a628d68644a3a4cd0f510dc304805a/packages/jest-core/src/collectHandles.ts#L103-L113

    Or immediately before calling processResults() in runJest: https://github.com/facebook/jest/blob/64d5983d20a628d68644a3a4cd0f510dc304805a/packages/jest-core/src/runJest.ts#L279-L290

  3. Jest has a false negative for --detectOpenHandles when an open handle was created in a test function that uses a done callback. So you don’t see this warning when using a done function instead of promises, but not because the resource isn’t still open. Jest tries to determine whether to track an async handle based on the call stack where it was opened: https://github.com/facebook/jest/blob/64d5983d20a628d68644a3a4cd0f510dc304805a/packages/jest-core/src/collectHandles.ts#L18-L38

    But if the function has a done callback, it shows up anonymously in the callstack and so doesn’t register as something from which open async handles should be tracked: https://github.com/facebook/jest/blob/bc50e7f360ab1845abbaa0b3ad788caead0d3174/packages/jest-jasmine2/src/jasmineAsyncInstall.ts#L111-L113

    As opposed to when it uses a promise, and is given a name (asyncJestTest) that shows up in the call stack and is checked for in the stackIsFromUser shown above: https://github.com/facebook/jest/blob/bc50e7f360ab1845abbaa0b3ad788caead0d3174/packages/jest-jasmine2/src/jasmineAsyncInstall.ts#L153

(2) seems like the most immediately applicable thing here (wait a tick for any handles that are scheduled for destruction to actually be destroyed).

(3) seems like it should be fixed, too, although it doesn’t really address the immediate problem here (instead, it would just make this warning show up more).

@nfantone
Copy link

I stumbled upon this today. Oddly enough, in my case, it doesn't always happen. If using supertest, the infamous TCPSERVERWRAP error would only sometimes happen, seemingly at random. However, --detectLeaks fails consistently for me (see ladjs/supertest#520 (comment)).

@Mr0grog
Copy link
Contributor

Mr0grog commented May 14, 2021

FWIW, the --detectOpenHandles cases are 100% reproducible for me with current versions of Jest and Supertest, and I feel like I've spent enough time digging under the hood to feel fairly confident I fully understand the issue (if it's unpredictable for you, it could be related to extra, non-predictable time an after* hook, a Jest plugin, or something in Express or your app takes). I haven't looked at --detectLeaks at all, though, so I don't have any useful advice on what might be going wrong there.

@nfantone
Copy link

nfantone commented May 15, 2021

it could be related to extra, non-predictable time an after* hook, a Jest plugin, or something in Express or your app takes

@Mr0grog You might be right here. Take a look at my example on the quoted comment above. There are no other resources that may introduce unpredictable behaviour - however, if you uncomment the afterAll call, effectively waiting one tick for the server to die, the --detectOpenHandles option does not return any errors. This, to me, proves that the issue is time sensitive.

'use strict'
const request = require('supertest')
const express = require('express')

test('should not leak memory', async () => {
  const app = express()
  await request(app).get('/')
})

// afterAll(done => setTimeout(done, 0))

That being said, --detectLeaks, on the other hand, always fails. For every case.

Mr0grog added a commit to Mr0grog/jest that referenced this issue May 20, 2021
Some types of async resources in Node.js are not destroyed until *after* their `close` or `end` or similar callbacks and events run, leading to a situation where the `--detectOpenHandles` option can falsely flag resources that have been cleaned up in user code and are already scheduled for destruction. For example, if a test ends from the callback to `TCPServer.close()` and no other tests or lifecycle functions impose additional delays, `--detectOpenHandles` will flag the server even though it has been closed. This is the main cause of issues people encounter with Supertest (see jestjs#8554).

This addresses the issue by adding a short delay before collecting open handles.

Depends on jestjs#11382.
Mr0grog added a commit to Mr0grog/jest that referenced this issue May 20, 2021
Some types of async resources in Node.js are not destroyed until *after* their `close` or `end` or similar callbacks and events run, leading to a situation where the `--detectOpenHandles` option can falsely flag resources that have been cleaned up in user code and are already scheduled for destruction. For example, if a test ends from the callback to `TCPServer.close()` and no other tests or lifecycle functions impose additional delays, `--detectOpenHandles` will flag the server even though it has been closed. This is the main cause of issues people encounter with Supertest (see jestjs#8554).

This addresses the issue by adding a short delay before collecting open handles.

Depends on jestjs#11382.
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
7 participants