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

Adapter not closed on server closed #9

Closed
jmnsf opened this issue Jan 10, 2023 · 3 comments
Closed

Adapter not closed on server closed #9

jmnsf opened this issue Jan 10, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@jmnsf
Copy link

jmnsf commented Jan 10, 2023

Unsure if this is an issue in the adapter itself or upstream, but when calling io.close(), the adapter's .close() function is never called, so the connection with MongoDB stays up. I suppose this isn't too serious when closing down a server since everything's torn down anyway, but can be annoying in CI.

Problem:

afterAll((done) => {
  io.close(done);
});

One would expect this to shut down cleanly, but after the suite's done:

Test Suites: 1 passed, 1 total
Tests:       5 passed, 5 total
Snapshots:   0 total
Time:        5.166 s, estimated 6 s
Ran all test suites matching /src\/__tests__\/io.test.js/i.
/.../node_modules/mongoose/lib/drivers/node-mongodb-native/collection.js:153
        throw new Error('Collection method ' + i + ' is synchronous');
        ^

Error: Collection method watch is synchronous
    at NativeCollection.<computed> [as watch] (/.../node_modules/mongoose/lib/drivers/node-mongodb-native/collection.js:153:15)
    at initChangeStream (/.../node_modules/@socket.io/mongo-adapter/dist/index.js:80:40)
    at Timeout._onTimeout (/.../node_modules/@socket.io/mongo-adapter/dist/index.js:99:17)
    at listOnTimeout (node:internal/timers:557:17)
    at processTimers (node:internal/timers:500:7)
error Command failed with exit code 1.

Which isn't great. A workaround is going into the default namespace and tearing down the adapter manually:

afterAll((done) => {
  io.close((err) => {
    io.of('/').adapter.close();
    done(err);
  });
});

Would be nice if this weren't necessary (or at least documented somewhere).

@darrachequesne
Copy link
Member

@jmnsf that's a good point 👍 we will include this in the next release.

@darrachequesne darrachequesne added the enhancement New feature or request label Jan 16, 2023
@fhp
Copy link

fhp commented Jan 10, 2024

If you use multiple namespaces, use this to close them all:

const namespaces = io._nsps.keys();
for (const namespace of namespaces) {
    void io.of(namespace).adapter.close();
}

For Google visibility: in my case, this lead to the following error while running tests with Jest:

MongoNotConnectedError: Client must be connected before running operations

@darrachequesne
Copy link
Member

For future readers:

This should be fixed by socketio/socket.io@bf64870, included in socket.io@4.7.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants