[GODRIVER-966] Document whether it's necessary to call client.Disconnect() if client.Connect() returns error Created: 15/Apr/19 Updated: 01/Aug/22 |
|
| Status: | Backlog |
| Project: | Go Driver |
| Component/s: | Documentation |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Timothy Olsen (Inactive) | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | neweng, techdebt | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||
| Description |
|
I am wondering if I need to call client.Disconnect() if client.Connect() returns an error. It was requested that I open a ticket so that the answer to this question can be documented. |
| Comments |
| Comment by Kristofer Brandow (Inactive) [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
No, mongo.Client.Connect is not synchronous as per the SDAM specification. While it's technically not a constructor and we could do a check, we opted to recommend that users instead use the Ping method, which they can pass a read preference to. This enables the user to determine if what they actually need is available, e.g. a primary or a secondary. | |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Timothy Olsen (Inactive) [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
What if MongoDB is down or auth fails? Does mongo.Client.Connect() not return an error? | |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Kristofer Brandow (Inactive) [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
Currently, you'll only get an error from a call to mongo.Client.Connect if the mongo.Client is already connected or in a disconnecting state, so calling Disconnect would (likely) itself result in an error (because the mongo.Client is already disconnected). | |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
I spoke to soon. There are two Connect methods. There may be a problem with the second one (the one you are referring to, Dan), since if it returns an error it returns a nil Client, so you can't call Disconnect on it. It should probably call Disconnect itself before returning nil, err. kris.brandow what do you think? | |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
Ah, John Morales had the same question. It's not Connect that returns a client, it's NewClient. Here's a code sample to make it more explicit:
| |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Daniel Gottlieb (Inactive) [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
Should `client.Disconnect` be called unconditionally, or only when the returned client returned by mongo.Connect is non-nil? | |||||||||||||||||||||||||||||||||||||||||||||
| Comment by Jeffrey Yemin [ 16/Apr/19 ] | |||||||||||||||||||||||||||||||||||||||||||||
|
Short answer until we document: definitely call Disconnect after calling Connect, regardless of whether Connect returns a non-nil error. |