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

fix(NODE-5225): eagerly clear MongoClient.topology in MongoClient.close() #3652

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alecgibson
Copy link
Contributor

Description

At the moment, calling MongoClient.close() in quick succession leads to an error:

TypeError: Cannot read properties of undefined (reading 'close')
  at node-mongodb-native/src/mongo_client.ts:530:16
  at new Promise (<anonymous>)
  at LegacyMongoClient.close (src/mongo_client.ts:529:11)
  at processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async Promise.all (index 1)
  at async Context.<anonymous> (test/integration/node-specific/mongo_client.test.ts:393:5)

This happens because:

  1. The first call attempts to asynchronously close the session pool sessions
  2. While this is happening, the second call passes the topology null check (since this.topology hasn't yet been unset, and tries to also close session pool sessions
  3. Both calls then set const topology = this.topology but the second call will get undefined since the first call unset this.topology
  4. The second call now tries to call close() on an undefined topology and throws

What is changing?

This change fixes the issue by moving the topology unset immediately after the null check, so we know it's always correctly set.

Is there new documentation needed for these changes?

No

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

…se()

At the moment, calling `MongoClient.close()` in quick succession leads
to an error:

```
TypeError: Cannot read properties of undefined (reading 'close')
  at node-mongodb-native/src/mongo_client.ts:530:16
  at new Promise (<anonymous>)
  at LegacyMongoClient.close (src/mongo_client.ts:529:11)
  at processTicksAndRejections (node:internal/process/task_queues:95:5)
  at async Promise.all (index 1)
  at async Context.<anonymous> (test/integration/node-specific/mongo_client.test.ts:393:5)
```

This happens because:

1. The first call attempts to asynchronously [close][1] the session pool
   sessions
2. While this is happening, the second call passes the topology
   [null check][2] (since `this.topology` hasn't yet been [unset][3],
   and tries to also close session pool sessions
3. Both calls then set [`const topology = this.topology`][4] but the
   second call will get undefined since the first call [unset][3]
   `this.topology`
4. The second call now tries to call [`close()`][5] on an `undefined`
   `topology` and throws

This change fixes the issue by moving the `topology` unset immediately
after the `null` check, so we know it's always correctly set.

[1]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L516
[2]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L503
[3]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L527
[4]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L526
[5]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L530
@dariakp dariakp added tracked-in-jira Ticket filed in MongoDB's Jira system External Submission PR submitted from outside the team labels Apr 25, 2023
@ArjixWasTaken
Copy link

Why not do ?.close()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Submission PR submitted from outside the team tracked-in-jira Ticket filed in MongoDB's Jira system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants