fix(connection): make pool not try to reconnect forever when reconnectTries = 0

[vkarpov15]

This is a somewhat strange case. The below script will print an error immediately but continue retrying connecting in the background forever:

const mongodb = require('mongodb');

mongodb.MongoClient.connect('mongodb://thisdoesntexist:27017', { reconnectTries: 0 }).
  then(() => console.log('success'), err => console.log('error', err));

If you set reconnectTries to 1, the script prints an error and then exits.

const mongodb = require('mongodb');

mongodb.MongoClient.connect('mongodb://thisdoesntexist:27017', { reconnectTries: 1 }).
  then(() => console.log('success'), err => console.log('error', err));

This patch fixes that particular issue. But this issue surfaced as part of investigating Automattic/mongoose#6028, which brings up a valid concern: why does the driver continue trying to reconnect and block process exit even after it rejects the MongoClient.connect() promise? I was under the impression that the driver would not try to reconnect after initial connection failed.

[jsardev]

I just came into the same issue 😭 When can we expect a review and merge?

[mbroadst]

LGTM, but I want to include a test. This will have to wait a day, or if someone gets a test in first we can merge immediately 😄

[vkarpov15]

I'll work on putting in a test later today, but I think the most important thing here is to answer the question of whether the driver should try to reconnect in the background even after its rejected the promise and called the callback. I was always under the impression that the driver would not try to reconnect if initial connection failed, so I suspect this behavior is an unintentional quirk in the driver, but I'd like to hear @mbroadst 's thoughts on the matter.

[mbroadst]

@vkarpov15 many of our other drivers have no concept of a connect event, rather you simply create the MongoClient with your connection string, and connections are lazily created on demand. I think what you're witnessing here is a half-committed effort to both approaches, and I agree with you that we should not continue to attempt reconnection in the background once the connect promise has failed.

[vkarpov15]

@mbroadst that's fair. Do you want to remove that or should I? Also, I added a test 👍

[mbroadst]

@vkarpov15 thanks for the test! If I'm not mistaken shouldn't this fix correct the behavior we were discussing? Or rather: what are you referring to removing additionally?

[vkarpov15]

Not exactly. This provides a workaround to prevent the driver from attempting to reconnect if connection failed, but setting reconnectTries: 0 also prevents the driver from trying to reconnect if it loses connection after initial connection is made. But this PR is a good improvement to make for now.

[vkarpov15]

@mbroadst do you have any further comments on this issue?

[daprahamian]

@vkarpov15 sorry for the delay, @mbroadst is on vacation. I will merge this in ASAP.