surrealdb / surrealdb.js

SurrealDB SDK for JavaScript
https://surrealdb.com
Apache License 2.0
288 stars 47 forks source link

mocha test hangs after calling close() #6

Closed student020341 closed 1 year ago

student020341 commented 2 years ago

Here is a quick test file that hangs with mocha. I ran the "why-is-node-running" library at one point and I think the output from that suggests the websocket is not closing.

import { expect } from "chai";
import surreal from "surrealdb.js";

describe.only("database tests", () => {
  describe("surreal", () => {
    let dbi;
    before(async () => {
      dbi = new surreal("http://localhost:8000/rpc");
      await dbi.signin({
        user: "root",
        pass: "root"
      });

      await dbi.use("test", "test");
    });

    after(async () => {
      await dbi.query(`delete foo`);
      dbi.close();
    });

    it("should create something", async () => {
      const res = await dbi.query(`create foo:stuff set a=1, b="two";`);
      expect(res?.[0]?.result?.[0].b).to.equal("two");
    });
  })
});

Here's the output from "why-is-node-running"

There are 3 handle(s) keeping the process running

# DNSCHANNEL
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/websocket.js:1015                     - return net.connect(options);
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/websocket.js:841                      - req = websocket._req = request(opts);
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/websocket.js:85                       - initAsClient(this, address, protocols, options);
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/classes/socket.js:40
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/index.js:158
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/index.js:79
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/test/sanity.mjs:10
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/mocha/lib/runnable.js:366                    - var result = fn.call(ctx);
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/mocha/lib/runnable.js:354                    - callFn(this.fn);
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/mocha/lib/runner.js:498                      - hook.run(function cbHookRun(err) {
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/mocha/lib/runner.js:559                      - next(0);

# Timeout
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/classes/pinger.js:12
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/index.js:115
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/classes/emitter.js:31
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/classes/emitter.js:30
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/surrealdb.js/src/classes/socket.js:59
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/event-target.js:227                    - listener.call(this, event);
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/websocket.js:237                       - this.emit('open');
/Users/thomasesposito/Desktop/stuff/chain-blocks/server/node_modules/ws/lib/websocket.js:983                       - websocket.setSocket(socket, head, {

# Timeout
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/test/utils.mjs:1
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/test/utils.mjs:1
file:///Users/thomasesposito/Desktop/stuff/chain-blocks/server/test/sanity.mjs:22

I ran an identical test with the postgres client which showed similar behavior if the close call was omitted.

Chooks22 commented 2 years ago

The issue was the listeners (including the one that handles cleanup) are removed before the close event is fired, simply swapping these 2 lines here or just removing the removeAllListeners() call should resolve the issue.

student020341 commented 2 years ago

I still saw the issue if I swapped the 2 lines, but removing this.#ws.removeAllListeners(); did appear to fix the issue. Any potential repercussions of not removing those listeners?

Chooks22 commented 2 years ago

Not much, since it's only storing arrays of listeners. Nothing is kept alive if the emitter goes out of scope