Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap RPC errors #12638

Merged
merged 5 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading