Merge pull request #411 from onflow/gregor/improve-tx-errors

Improve error handling and logging

api/api.go

@@ -19,14 +19,13 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/sethvargo/go-limiter"
 
-	errs "github.com/onflow/flow-evm-gateway/api/errors"
 	"github.com/onflow/flow-evm-gateway/config"
 	"github.com/onflow/flow-evm-gateway/metrics"
 	"github.com/onflow/flow-evm-gateway/models"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 	"github.com/onflow/flow-evm-gateway/services/logs"
 	"github.com/onflow/flow-evm-gateway/services/requester"
 	"github.com/onflow/flow-evm-gateway/storage"
-	storageErrs "github.com/onflow/flow-evm-gateway/storage/errors"
 )
 
 const maxFeeHistoryBlockCount = 1024
@@ -1017,27 +1016,20 @@ func (b *BlockChainAPI) getBlockNumber(blockNumberOrHash *rpc.BlockNumberOrHash)
 // empty type.
 func handleError[T any](err error, log zerolog.Logger, collector metrics.Collector) (T, error) {
 	var (
-		zero              T
-		errGasPriceTooLow *errs.GasPriceTooLowError
-		revertError       *errs.RevertError
+		zero        T
+		revertedErr *errs.RevertError
 	)
 
 	switch {
 	// as per specification returning nil and nil for not found resources
-	case errors.Is(err, storageErrs.ErrNotFound):
+	case errors.Is(err, errs.ErrNotFound):
 		return zero, nil
-	case errors.As(err, &revertError):
-		return zero, revertError
-	case errors.Is(err, storageErrs.ErrInvalidRange):
-		return zero, err
-	case errors.Is(err, models.ErrInvalidEVMTransaction):
-		return zero, err
-	case errors.Is(err, requester.ErrOutOfRange):
-		return zero, err
 	case errors.Is(err, errs.ErrInvalid):
 		return zero, err
-	case errors.As(err, &errGasPriceTooLow):
-		return zero, errGasPriceTooLow
+	case errors.Is(err, errs.ErrFailedTransaction):
+		return zero, err
+	case errors.As(err, &revertedErr):
+		return zero, revertedErr
 	default:
 		collector.ApiErrorOccurred()
 		log.Error().Err(err).Msg("api error")

api/encode_transaction.go

@@ -6,7 +6,7 @@ import (
 
 	"github.com/onflow/go-ethereum/core/types"
 
-	errs "github.com/onflow/flow-evm-gateway/api/errors"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 )
 
 const blockGasLimit uint64 = 15_000_000

api/models.go

@@ -6,8 +6,8 @@ import (
 	"fmt"
 	"math/big"
 
-	errs "github.com/onflow/flow-evm-gateway/api/errors"
 	"github.com/onflow/flow-evm-gateway/models"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 
 	"github.com/onflow/go-ethereum/common"
 	"github.com/onflow/go-ethereum/common/hexutil"
@@ -40,7 +40,9 @@ type TransactionArgs struct {
 func (txArgs TransactionArgs) Validate() error {
 	// Prevent accidental erroneous usage of both 'input' and 'data' (show stopper)
 	if txArgs.Data != nil && txArgs.Input != nil && !bytes.Equal(*txArgs.Data, *txArgs.Input) {
-		return errors.New(`ambiguous request: both "data" and "input" are set and are not identical`)
+		return errs.InvalidTransaction(
+			errors.New(`ambiguous request: both "data" and "input" are set and are not identical`),
+		)
 	}
 
 	// Place data on 'data'
@@ -62,11 +64,13 @@ func (txArgs TransactionArgs) Validate() error {
 		if txDataLen == 0 {
 			// Prevent sending ether into black hole (show stopper)
 			if txArgs.Value.ToInt().Cmp(big.NewInt(0)) > 0 {
-				return errors.New("transaction will create a contract with value but empty code")
+				return errs.InvalidTransaction(
+					errors.New("transaction will create a contract with value but empty code"),
+				)
 			}
 
 			// No value submitted at least, critically Warn, but don't blow up
-			return errors.New("transaction will create a contract with empty code")
+			return errs.InvalidTransaction(errors.New("transaction will create a contract with empty code"))
 		}
 	}
 
@@ -75,7 +79,7 @@ func (txArgs TransactionArgs) Validate() error {
 		to := common.NewMixedcaseAddress(*txArgs.To)
 
 		if bytes.Equal(to.Address().Bytes(), common.Address{}.Bytes()) {
-			return errors.New("transaction recipient is the zero address")
+			return errs.InvalidTransaction(errors.New("transaction recipient is the zero address"))
 		}
 	}
 

api/pull.go

@@ -17,9 +17,9 @@ import (
 
 	"github.com/onflow/flow-evm-gateway/config"
 	"github.com/onflow/flow-evm-gateway/models"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 	"github.com/onflow/flow-evm-gateway/services/logs"
 	"github.com/onflow/flow-evm-gateway/storage"
-	errs "github.com/onflow/flow-evm-gateway/storage/errors"
 )
 
 // maxFilters limits the max active filters at any time to prevent abuse and OOM

api/ratelimiter.go

@@ -6,8 +6,9 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/sethvargo/go-limiter"
 
-	errs "github.com/onflow/flow-evm-gateway/api/errors"
 	"github.com/onflow/go-ethereum/rpc"
+
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 )
 
 // rateLimit will limit requests with the provider limiter, in case the limit

api/server.go

@@ -24,9 +24,9 @@ import (
 	"github.com/rs/zerolog"
 	slogzerolog "github.com/samber/slog-zerolog"
 
-	errs "github.com/onflow/flow-evm-gateway/api/errors"
 	"github.com/onflow/flow-evm-gateway/config"
 	"github.com/onflow/flow-evm-gateway/metrics"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 )
 
 type rpcHandler struct {
@@ -437,7 +437,7 @@ func (w *loggingResponseWriter) Write(data []byte) (int, error) {
 		// don't error log known handled errors
 		if !errorIs(err, errs.ErrRateLimit) &&
 			!errorIs(err, errs.ErrInvalid) &&
-			!errorIs(err, errs.ErrInternal) &&
+			!errorIs(err, errs.ErrFailedTransaction) &&
 			!errorIs(err, errs.ErrNotSupported) {
 			l = w.logger.Error()
 		}

bootstrap/bootstrap.go

@@ -17,11 +17,11 @@ import (
 	"github.com/onflow/flow-evm-gateway/config"
 	"github.com/onflow/flow-evm-gateway/metrics"
 	"github.com/onflow/flow-evm-gateway/models"
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 	"github.com/onflow/flow-evm-gateway/services/ingestion"
 	"github.com/onflow/flow-evm-gateway/services/requester"
 	"github.com/onflow/flow-evm-gateway/services/traces"
 	"github.com/onflow/flow-evm-gateway/storage"
-	storageErrs "github.com/onflow/flow-evm-gateway/storage/errors"
 	"github.com/onflow/flow-evm-gateway/storage/pebble"
 )
 
@@ -81,7 +81,7 @@ func Start(ctx context.Context, cfg *config.Config) error {
 	}
 
 	// if database is not initialized require init height
-	if _, err := blocks.LatestCadenceHeight(); errors.Is(err, storageErrs.ErrNotInitialized) {
+	if _, err := blocks.LatestCadenceHeight(); errors.Is(err, errs.ErrNotInitialized) {
 		cadenceHeight := cfg.InitCadenceHeight
 		cadenceBlock, err := client.GetBlockHeaderByHeight(ctx, cadenceHeight)
 		if err != nil {

models/engine.go

@@ -2,9 +2,13 @@ package models
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"github.com/rs/zerolog"
 	"time"
+
+	"github.com/rs/zerolog"
+
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
 )
 
 // Engine defines a processing unit
@@ -85,7 +89,7 @@ func (r *RestartableEngine) Run(ctx context.Context) error {
 			// don't restart if no error is returned, normal after stop procedure is done
 			return nil
 		}
-		if !IsRecoverableError(err) {
+		if !errors.Is(err, errs.ErrRecoverable) {
 			r.logger.Error().Err(err).Msg("received unrecoverable error")
 			// if error is not recoverable just die
 			return err

models/engine_test.go

@@ -6,11 +6,13 @@ import (
 	"testing"
 	"time"
 
-	"github.com/onflow/flow-evm-gateway/models/mocks"
 	"github.com/rs/zerolog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+
+	errs "github.com/onflow/flow-evm-gateway/models/errors"
+	"github.com/onflow/flow-evm-gateway/models/mocks"
 )
 
 func Test_RestartableEngine(t *testing.T) {
@@ -61,13 +63,13 @@ func Test_RestartableEngine(t *testing.T) {
 				// make sure time diff increases with each retry
 				assert.True(t, prevDiff < curDiff)
 				prevDiff = curDiff
-				return ErrDisconnected
+				return errs.ErrDisconnected
 			}).
 			Times(int(retries))
 
 		r := NewRestartableEngine(mockEngine, retries, zerolog.New(zerolog.NewTestWriter(t)))
 		err := r.Run(context.Background())
-		require.EqualError(t, ErrDisconnected, err.Error())
+		require.EqualError(t, errs.ErrDisconnected, err.Error())
 	})
 
 	t.Run("should restart when recoverable error is returned but then return nil after error is no longer returned", func(t *testing.T) {
@@ -83,7 +85,7 @@ func Test_RestartableEngine(t *testing.T) {
 					}).
 					Once()
 
-				return ErrDisconnected
+				return errs.ErrDisconnected
 			}).
 			Once()