From a3d74b492127d317c14c7b93443ff5990c8060a5 Mon Sep 17 00:00:00 2001 From: Jordan Krage <jmank88@gmail.com> Date: Thu, 5 May 2022 11:48:15 -0500 Subject: [PATCH] 1.4: panic on failover (#6556) * core/chains/evm/client: check for non-nil error instead of nil interface value * core/chains/evm/client: expand test to cover failed EthSubscribe Co-authored-by: Andrei Smirnov <andrei.smirnov@smartcontract.com> (cherry picked from commit de23ce96fd1dfd91326caa9af7157144b84c3ca8) --- core/chains/evm/client/node.go | 13 ++++++------- core/chains/evm/client/node_lifecycle_test.go | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/core/chains/evm/client/node.go b/core/chains/evm/client/node.go index c8c12c56412..56ed567284f 100644 --- a/core/chains/evm/client/node.go +++ b/core/chains/evm/client/node.go @@ -469,6 +469,9 @@ func (n *node) EthSubscribe(ctx context.Context, channel chan<- *evmtypes.Head, lggr.Debug("RPC call: evmclient.Client#EthSubscribe") start := time.Now() sub, err := n.ws.rpc.EthSubscribe(ctx, channel, args...) + if err == nil { + n.registerSub(sub) + } duration := time.Since(start) n.logResult(lggr, err, duration, n.getRPCDomain(), "EthSubscribe", @@ -477,9 +480,6 @@ func (n *node) EthSubscribe(ctx context.Context, channel chan<- *evmtypes.Head, "err", err, ) - if sub != nil { - n.registerSub(sub) - } return sub, err } @@ -904,6 +904,9 @@ func (n *node) SubscribeFilterLogs(ctx context.Context, q ethereum.FilterQuery, lggr.Debug("RPC call: evmclient.Client#SubscribeFilterLogs") start := time.Now() sub, err = n.ws.geth.SubscribeFilterLogs(ctx, q, ch) + if err == nil { + n.registerSub(sub) + } err = n.wrapWS(err) duration := time.Since(start) @@ -913,10 +916,6 @@ func (n *node) SubscribeFilterLogs(ctx context.Context, q ethereum.FilterQuery, "err", err, ) - if sub != nil { - n.registerSub(sub) - } - return } diff --git a/core/chains/evm/client/node_lifecycle_test.go b/core/chains/evm/client/node_lifecycle_test.go index c21c581b2d4..0ff4ef1f6c0 100644 --- a/core/chains/evm/client/node_lifecycle_test.go +++ b/core/chains/evm/client/node_lifecycle_test.go @@ -12,6 +12,7 @@ import ( "go.uber.org/atomic" "go.uber.org/zap" + evmtypes "github.com/smartcontractkit/chainlink/core/chains/evm/types" "github.com/smartcontractkit/chainlink/core/internal/testutils" "github.com/smartcontractkit/chainlink/core/logger" ) @@ -157,10 +158,15 @@ func TestUnit_NodeLifecycle_aliveLoop(t *testing.T) { dial(t, n) defer n.Close() + _, err := n.EthSubscribe(testutils.Context(t), make(chan *evmtypes.Head)) + assert.Error(t, err) + n.wg.Add(1) n.aliveLoop() assert.Equal(t, NodeStateUnreachable, n.State()) + // sc-39341: ensure failed EthSubscribe didn't register a (*rpc.ClientSubscription)(nil) which would lead to a panic on Unsubscribe + assert.Len(t, n.subs, 0) }) t.Run("if remote RPC connection is closed transitions to unreachable", func(t *testing.T) {