Skip to content

Commit

Permalink
Wrap RPC errors (#12638)
Browse files Browse the repository at this point in the history
* Tag errors with msg that RPC Client returned the error and include RPC name

* changeset

* added tag to the changeset
  • Loading branch information
dhaidashenko authored Apr 26, 2024
1 parent 0370aa9 commit bcf7653
Show file tree
Hide file tree
Showing 9 changed files with 275 additions and 119 deletions.
5 changes: 5 additions & 0 deletions .changeset/quick-fishes-heal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---
#changed
Added prefix `RPCClient returned error ({RPC_NAME})` to RPC errors to simplify filtering of RPC related issues.
29 changes: 29 additions & 0 deletions common/client/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,35 @@ var sendTxSevereErrors = []SendTxReturnCode{Fatal, Underpriced, Unsupported, Exc
// sendTxSuccessfulCodes - error codes which signal that transaction was accepted by the node
var sendTxSuccessfulCodes = []SendTxReturnCode{Successful, TransactionAlreadyKnown}

func (c SendTxReturnCode) String() string {
switch c {
case Successful:
return "Successful"
case Fatal:
return "Fatal"
case Retryable:
return "Retryable"
case Underpriced:
return "Underpriced"
case Unknown:
return "Unknown"
case Unsupported:
return "Unsupported"
case TransactionAlreadyKnown:
return "TransactionAlreadyKnown"
case InsufficientFunds:
return "InsufficientFunds"
case ExceedsMaxFee:
return "ExceedsMaxFee"
case FeeOutOfValidRange:
return "FeeOutOfValidRange"
case OutOfCounters:
return "OutOfCounters"
default:
return fmt.Sprintf("SendTxReturnCode(%d)", c)
}
}

type NodeTier int

const (
Expand Down
16 changes: 16 additions & 0 deletions common/client/models_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package client

import (
"strings"
"testing"
)

func TestSendTxReturnCode_String(t *testing.T) {
// ensure all the SendTxReturnCodes have proper name
for c := 1; c < int(sendTxReturnCodeLen); c++ {
strC := SendTxReturnCode(c).String()
if strings.Contains(strC, "SendTxReturnCode(") {
t.Errorf("Expected %s to have a proper string representation", strC)
}
}
}
13 changes: 10 additions & 3 deletions common/client/multi_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,13 @@ func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OP
return n.RPC().PendingSequenceAt(ctx, addr)
}

type sendTxErrors map[SendTxReturnCode][]error

// String - returns string representation of the errors map. Required by logger to properly represent the value
func (errs sendTxErrors) String() string {
return fmt.Sprint(map[SendTxReturnCode][]error(errs))
}

func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OPS, TX_RECEIPT, FEE, HEAD, RPC_CLIENT, BATCH_ELEM]) SendEmptyTransaction(
ctx context.Context,
newTxAttempt func(seq SEQ, feeLimit uint32, fee FEE, fromAddress ADDR) (attempt any, err error),
Expand Down Expand Up @@ -602,7 +609,7 @@ func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OP
ctx, cancel := c.chStop.Ctx(ctx)
defer cancel()
requiredResults := int(math.Ceil(float64(healthyNodesNum) * sendTxQuorum))
errorsByCode := map[SendTxReturnCode][]error{}
errorsByCode := sendTxErrors{}
var softTimeoutChan <-chan time.Time
var resultsCount int
loop:
Expand Down Expand Up @@ -639,7 +646,7 @@ loop:

func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OPS, TX_RECEIPT, FEE, HEAD, RPC_CLIENT, BATCH_ELEM]) reportSendTxAnomalies(tx TX, txResults <-chan sendTxResult) {
defer c.wg.Done()
resultsByCode := map[SendTxReturnCode][]error{}
resultsByCode := sendTxErrors{}
// txResults eventually will be closed
for txResult := range txResults {
resultsByCode[txResult.ResultCode] = append(resultsByCode[txResult.ResultCode], txResult.Err)
Expand All @@ -653,7 +660,7 @@ func (c *multiNode[CHAIN_ID, SEQ, ADDR, BLOCK_HASH, TX, TX_HASH, EVENT, EVENT_OP
}
}

func aggregateTxResults(resultsByCode map[SendTxReturnCode][]error) (txResult error, err error) {
func aggregateTxResults(resultsByCode sendTxErrors) (txResult error, err error) {
severeErrors, hasSevereErrors := findFirstIn(resultsByCode, sendTxSevereErrors)
successResults, hasSuccess := findFirstIn(resultsByCode, sendTxSuccessfulCodes)
if hasSuccess {
Expand Down
20 changes: 11 additions & 9 deletions common/client/multi_node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -796,13 +796,13 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
Name string
ExpectedTxResult string
ExpectedCriticalErr string
ResultsByCode map[SendTxReturnCode][]error
ResultsByCode sendTxErrors
}{
{
Name: "Returns success and logs critical error on success and Fatal",
ExpectedTxResult: "success",
ExpectedCriticalErr: "found contradictions in nodes replies on SendTransaction: got success and severe error",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
Successful: {errors.New("success")},
Fatal: {errors.New("fatal")},
},
Expand All @@ -811,7 +811,7 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
Name: "Returns TransactionAlreadyKnown and logs critical error on TransactionAlreadyKnown and Fatal",
ExpectedTxResult: "tx_already_known",
ExpectedCriticalErr: "found contradictions in nodes replies on SendTransaction: got success and severe error",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
TransactionAlreadyKnown: {errors.New("tx_already_known")},
Unsupported: {errors.New("unsupported")},
},
Expand All @@ -820,7 +820,7 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
Name: "Prefers sever error to temporary",
ExpectedTxResult: "underpriced",
ExpectedCriticalErr: "",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
Retryable: {errors.New("retryable")},
Underpriced: {errors.New("underpriced")},
},
Expand All @@ -829,15 +829,15 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
Name: "Returns temporary error",
ExpectedTxResult: "retryable",
ExpectedCriticalErr: "",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
Retryable: {errors.New("retryable")},
},
},
{
Name: "Insufficient funds is treated as error",
ExpectedTxResult: "",
ExpectedCriticalErr: "",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
Successful: {nil},
InsufficientFunds: {errors.New("insufficientFunds")},
},
Expand All @@ -846,13 +846,13 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
Name: "Logs critical error on empty ResultsByCode",
ExpectedTxResult: "expected at least one response on SendTransaction",
ExpectedCriticalErr: "expected at least one response on SendTransaction",
ResultsByCode: map[SendTxReturnCode][]error{},
ResultsByCode: sendTxErrors{},
},
{
Name: "Zk out of counter error",
ExpectedTxResult: "not enough keccak counters to continue the execution",
ExpectedCriticalErr: "",
ResultsByCode: map[SendTxReturnCode][]error{
ResultsByCode: sendTxErrors{
OutOfCounters: {errors.New("not enough keccak counters to continue the execution")},
},
},
Expand All @@ -870,6 +870,9 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
assert.EqualError(t, txResult, testCase.ExpectedTxResult)
}

logger.Sugared(logger.Test(t)).Info("Map: " + fmt.Sprint(testCase.ResultsByCode))
logger.Sugared(logger.Test(t)).Criticalw("observed invariant violation on SendTransaction", "resultsByCode", testCase.ResultsByCode, "err", err)

if testCase.ExpectedCriticalErr == "" {
assert.NoError(t, err)
} else {
Expand All @@ -884,5 +887,4 @@ func TestMultiNode_SendTransaction_aggregateTxResults(t *testing.T) {
delete(codesToCover, codeToIgnore)
}
assert.Empty(t, codesToCover, "all of the SendTxReturnCode must be covered by this test")

}
Loading

0 comments on commit bcf7653

Please sign in to comment.