feat(NODE-6633): MongoClient.close closes active cursors #4372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addaleax
reviewed
Jan 16, 2025
src/cursor/abstract_cursor.ts
Outdated
| this.cursorClient.s.activeCursors.add(this); | ||
| if (!this.listeners('close').includes(removeActiveCursor)) { | ||
| this.once('close', removeActiveCursor); | ||
| } |
There was a problem hiding this comment.
Any chance that this and the constructor should share more code? This change here feels like something that could have been really easy to miss if you didn't happen to think of it :)
There was a problem hiding this comment.
Yea "setupTracking" helper of sorts seems worth having, will do, ty
nbbeeken
force-pushed
the
NODE-6633-track-cursors
branch
from
cb798ec to
567fb98
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What is changing?
Is there new documentation needed for these changes?
What is the motivation for this change?
In an effort to make MongoClient.close more effective and align with other existing close semantics in the ecosystem our close method is being improved to ensure the client when closed cleans up any resources it was responsible for creating. This allows users to rely on the paradigm that a closed client is no longer performing operations, which is currently not true and is likely surprising.
Release Highlight
MongoClient.close now closes any outstanding cursors
Previously, cursors could somewhat live beyond the client they came from. What this meant was depending on timing you would learn of the client's (and by proxy, the cursor's) demise via an assertion that the associated session had expired. This only occurred if your cursor needed to use the session, which only happens when it is attempting to run a
getMoreoperation to obtain another batch of documents.Practically speaking a cursor that lives beyond a client is an exception waiting to happen, the connection pools are closed, the sessions are ended, last call has been served 🍻, it is only a matter of timing and event firing until the cursor learns of its fate and informs you via whatever interaction is being used (
.toArray(),for-await,.next()).To make the expected state of cursors clearer in this scenario
MongoClient'swill now close any associated cursor upon itsclose()-ing reducing the risk of leaving behind server-side resources.Double check the following
npm run check:lintscripttype(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript