Deprioritize passive clients in optimizingclient.get (#748)

* passive clients should always be sorted last in Get's.

client/empty.go

@@ -10,6 +10,8 @@ import (
 
 const emptyClientStringerValue = "EmptyClient"
 
+var errEmptyClientUnsupportedGet = errors.New("not supported")
+
 // EmptyClientWithInfo makes a client that returns the given info but no randomness
 func EmptyClientWithInfo(info *chain.Info) Client {
 	return &emptyClient{info}
@@ -32,7 +34,7 @@ func (m *emptyClient) RoundAt(t time.Time) uint64 {
 }
 
 func (m *emptyClient) Get(ctx context.Context, round uint64) (Result, error) {
-	return nil, errors.New("not supported")
+	return nil, errEmptyClientUnsupportedGet
 }
 
 func (m *emptyClient) Watch(ctx context.Context) <-chan Result {

client/optimizing.go

@@ -27,6 +27,8 @@ const (
 	// is re-open when no context error occurred.
 	defaultWatchRetryInterval = time.Second * 30
 	defaultChannelBuffer      = 5
+
+	maxUnixTime = 1<<63 - 62135596801
 )
 
 // newOptimizingClient creates a drand client that measures the speed of clients
@@ -104,6 +106,13 @@ func (oc *optimizingClient) Start() {
 // MarkPassive must tag clients as passive before `Start` is run.
 func (oc *optimizingClient) MarkPassive(c Client) {
 	oc.passiveClients = append(oc.passiveClients, c)
+	// push passive clients to the back of the list for `Get`s
+	for _, s := range oc.stats {
+		if s.client == c {
+			s.rtt = math.MaxInt64
+			s.startTime = time.Unix(maxUnixTime, 999999999)
+		}
+	}
 }
 
 type optimizingClient struct {
@@ -233,7 +242,12 @@ LOOP:
 				break LOOP
 			}
 			stats = append(stats, rr.stat)
-			res, err = rr.result, rr.err
+			res = rr.result
+			if rr.err != errEmptyClientUnsupportedGet && rr.err != nil {
+				err = fmt.Errorf("%v - %w", err, rr.err)
+			} else if rr.err == nil {
+				err = nil
+			}
 		case <-ctx.Done():
 			oc.updateStats(stats)
 			return nil, ctx.Err()
@@ -244,7 +258,7 @@ LOOP:
 	}
 
 	oc.updateStats(stats)
-	return
+	return res, err
 }
 
 // get calls Get on the passed client and returns a requestResult or nil if the context was canceled.