[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:
Related

 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:

import (
	"context"
	"fmt"
	"go.mongodb.org/mongo-driver/mongo"
	"go.mongodb.org/mongo-driver/mongo/options"
	"go.mongodb.org/mongo-driver/mongo/readpref"
)
 
func main() {
	err := check("mongodb://localhost:27017,localhost:27017,localhost:30000")
 
	if err != nil {
		fmt.Println(err)
	} else {
		fmt.Println("Success")
	}
 
}
 
func check(uri string) (error) {
	client, err := mongo.NewClient(options.Client().ApplyURI(uri))
 
	// if err is not nil, then client will be nil, and applications should not Disconnect a nil client
	if err != nil {
		return err
	}
 
	// we have a non-nil client and we're about to call Connect, so it's safe to defer a Disconnect
	defer client.Disconnect(context.Background())
 
        // Connect doesn't actually error if any (or all) of the servers are unreachable 
	err = client.Connect(context.Background())
 
	if err != nil {
		return err
	}
 
        // Ensure the primary is actually available by sending it an operation
	err = client.Ping(context.Background(), readpref.Primary())
	if err != nil {
		return err
	}
	return nil
}

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.

Generated at Thu Feb 08 08:35:21 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.