Stop stubbing dns module

[arthurschreiber]

This removes the stubbing of dns.lookup we had in place in some of our test cases.

This stubbing was error prone and lead to a whole bunch of flaky test failures on CI.

Instead of stubbing functions, this changes relevant code to allow passing in a custom lookup function, but falling back to dns.lookup if that function is undefined.

Closes: #988

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@afc4cab). Click here to learn what that means.
The diff coverage is 82.60%.

@@            Coverage Diff            @@
##             master    #1103   +/-   ##
=========================================
  Coverage          ?   79.81%           
=========================================
  Files             ?       86           
  Lines             ?     4394           
  Branches          ?      802           
=========================================
  Hits              ?     3507           
  Misses            ?      628           
  Partials          ?      259           

Impacted Files	Coverage Δ
src/instance-lookup.ts	90.00% <66.66%> (ø)
src/connector.ts	100.00% <100.00%> (ø)
src/sender.ts	100.00% <100.00%> (ø)
src/data-types/binary.ts	76.47% <0.00%> (ø)
src/value-parser.ts	82.29% <0.00%> (ø)
src/data-types/decimal.ts	92.85% <0.00%> (ø)
src/data-types/bigint.ts	89.47% <0.00%> (ø)
src/data-types/time.ts	97.22% <0.00%> (ø)
src/tracking-buffer/writable-tracking-buffer.ts	71.81% <0.00%> (ø)
src/transaction.ts	82.75% <0.00%> (ø)
... and 79 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afc4cab...da58b30. Read the comment docs.

[arthurschreiber]

I think this fixes the flakiness that #988 also tried to fix, but in a different way. 😅

The problem that #988 worked around was that sometimes the callback given to the InstanceLookup or Connector classes could be fired twice. On the first call, the current test would fail (because it did not expect an error), and then after some time the callback was called again causing whatever test was running at that time to fail as well. #988 worked around this by moving the assertions in the test into the non-async part of the test case.

This fixed the test flakyness, but did not fix the problems in the underlying code.

I believe the changes in this PR in the src/instance-lookup.ts fix the problem in the code because they slightly change how and when setTimeout/clearTimeout is called, and this should prevent the double callback execution.

But I think there is still problems with this code in general.

There is no way to cancel any of the in-flight operations (e.g. the dns lookups) if the connectTimeout has been reached or the connection is prematurely closed. So what users sometimes see is that the timeout fires, and then some time later another error is thrown again. I'll try to fix this in follow up PRs.

[github-actions]

🎉 This PR is included in version 8.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀