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 <[email protected]> (cherry picked from commit de23ce9)
smartcontractkit · May 5, 2022 · a3d74b4 · a3d74b4
1 parent a5f089f
commit a3d74b4
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git 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
@@ -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) {