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-3417): allow calling db() before MongoClient is connected #2889

Merged
merged 1 commit into from
Jul 16, 2021
Merged

fix(NODE-3417): allow calling db() before MongoClient is connected #2889

merged 1 commit into from
Jul 16, 2021

Conversation

vkarpov15
Copy link
Contributor

Description

Removed an error case that will make Automattic/mongoose#8702 much easier.

What changed?

Removed a check that looks unnecessary because the Db() constructor no longer takes a topology param.

Are there any files to ignore?

@nbbeeken nbbeeken changed the title fix(mongo_client): allow calling db() before MongoClient is connected fix(NODE-3417): allow calling db() before MongoClient is connected Jul 7, 2021
@vkarpov15
Copy link
Contributor Author

@nbbeeken any thoughts on merging this now that 4.0 is officially released?

Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea LGTM!

Just for posterity:
Tracking down the origin of this error revealed that the Db class used to take the topology as the second argument to the constructor, that's no longer the case and there's no reliance on the topology existing before grabbing a Db reference.

The UX change here is when you go to run an operation the error occurs when the topology is fetched by getTopology:

Uncaught MongoDriverError: MongoClient must be connected to perform this operation
    at Object.getTopology (lib/utils.js:383:11)
    at Collection.insertOne (lib/collection.js:151:61)

@nbbeeken nbbeeken requested review from dariakp and emadum July 15, 2021 16:55
@nbbeeken nbbeeken added the Team Review Needs review from team label Jul 15, 2021
Copy link
Contributor

@dariakp dariakp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@nbbeeken nbbeeken merged commit 51ea86d into mongodb:4.0 Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants