fix(discovery): findCtx is canceled too early by defer

[walldiss]

Overview

If underlying discovery.FindPeers closes the channel, discover call would return calling the defer stack and canceling fincCtx too early. This would result in all routines trying to establish connection being canceled before having time to succeed. Fixed by changing defer stack order of wg.Wait and context.Cancel calls.

The change also fixes flaky get_peer_from_discovery test, since flakiness was actually a bug.

[Wondertan]

Let's comment that the order matters.

[renaynay]

I would just group the defer statements into a

defer func() { 
 wg.Wait()
 findCancel()
}()

so the ordering of exec is explicitly clear.

alternatively make comment but i think if the ordering matters that much, it's better to make explicitly obvious

[codecov-commenter]

Codecov Report

Merging #2180 (5d22111) into main (7f556f0) will decrease coverage by 0.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2180      +/-   ##
==========================================
- Coverage   56.04%   55.83%   -0.22%     
==========================================
  Files         215      215              
  Lines       13918    13922       +4     
==========================================
- Hits         7800     7773      -27     
- Misses       5340     5367      +27     
- Partials      778      782       +4     

Impacted Files	Coverage Δ
share/availability/discovery/discovery.go	73.21% <100.00%> (-2.92%)	⬇️

... and 4 files with indirect coverage changes

[Wondertan]

The order of exec is clear and deterministic with defer as well, but I don't mind grouping them in one defer