feat(NODE-6392): add timeoutMS support to ClientEncryption helpers part 1

[nbbeeken]

Description

What is changing?

• Adds timeoutMS support to CE helpers

Is there new documentation needed for these changes?

No

What is the motivation for this change?

The timeoutMS setting given to CE will now be passed to the various helpers.

Double check the following

• Ran npm run check:lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
• New TODOs have a related JIRA ticket

[baileympearson]

Just some test questions!

[baileympearson]

I'm not opposed to adjusting our usages of client.options to client.s.options but unless we have some sort of mechanism of enforcing it, I worry that its a losing battle and that usages of client.options will slip in (like many imports from bson got past us in review until we linted for it).

If this is something we care about - maybe we could defer this and see if we can write a lint rule? We could probably do this with a typescript-eslint plugin.

Or deprecate options in favor of a non-Object.freeze() getter and then switch to the other getter?

[nbbeeken]

Unlike the BSON import which is just a 'nice-to-have' single location for all BSON interactions, the options getter on the client has real cost spreading and freezing on every access, we fixed a ~10% performance regression when we reverted its introduction to the Session constructor, so it is worth correcting it where we notice it.

I do agree though we should be able to remove the s.options and change the getter to just storing the options directly on the client without breaking anything.

This specific case is a bug: https://jira.mongodb.org/browse/NODE-6449

[nbbeeken]

I did consider making these changes to main, but most of the usages are introduced for CSOT, maybe this will close the small gap in perf we saw. fyi: @W-A-James

[baileympearson]

I don't understand how this fixes the bug you reported. We still instantiate an auto encrypter before adding the options to client.s - all you've done is explicitly default to {}.

Unlike sessions, none of the places you fixed in this PR are hot code paths (this usage included). I disagree that this is burning and that important. I'm trying to say that, if you believe this needs to be addressed, please file a ticket for it so we can fix it somehow and put guardrails around this so we can't make this mistake. Otherwise more usages of client.options will continue to pop up (for example - there are literally more occurrences of this in Aditi's PR).

edit: except cursors

[nbbeeken]

I don't understand how this fixes the bug you reported.

Because it doesn't! 😄 The ticket I filed is for us to fix the bug, I did change the code to be explicit as a reminder for anyone who drops by this line of code.

there are literally more occurrences of this in Aditi's PR

We should change the occurrences we can catch because of what we know to be true about the cost of using the incorrect API rather than giving up any attempt to improve what we are implementing.

I filed a ticket to fix this particular bug but at that time we can make sure we're not making the same mistake elsewhere and potentially lint or just correct the API altogether

[baileympearson]

Is there a ticket to either 1. find a tooling solution to prevent us from using client.options or 2. remove the freeze or 3. some other fix that ensures we don't make that mistake going forward?

[baileympearson]

just a minor test title fix