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

[Access] Improve error messages when querying old blocks #6554

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
42 changes: 42 additions & 0 deletions engine/access/rpc/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package backend
import (
"context"
"crypto/md5" //nolint:gosec
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -641,3 +642,44 @@ func chooseFromPreferredENIDs(allENs flow.IdentityList, executorIDs flow.Identif

return chosenIDs
}

// resolveHeightError processes errors returned during height-based queries.
// If the error is due to a block not being found, this function determines whether the queried
// height falls outside the node's accessible range and provides context-sensitive error messages
// based on spork and node root block heights.
//
// Parameters:
// - stateParams: Protocol parameters that contain spork root and node root block heights.
// - height: The queried block height.
// - genericErr: The initial error returned when the block is not found.
//
// Expected errors during normal operation:
// - storage.ErrNotFound - Indicates that the queried block does not exist in the local database.
func resolveHeightError(
stateParams protocol.Params,
height uint64,
genericErr error,
) error {
if !errors.Is(genericErr, storage.ErrNotFound) {
return genericErr
}

sporkRootBlockHeight := stateParams.SporkRootBlockHeight()
nodeRootBlockHeader := stateParams.SealedRoot().Height

if height < sporkRootBlockHeight {
return fmt.Errorf("block height %d is less than the spork root block height %d. Try to use a historic node: %w",
height,
sporkRootBlockHeight,
genericErr,
)
} else if height < nodeRootBlockHeader {
return fmt.Errorf("block height %d is less than the node's root block height %d. Try to use a different Access node: %w",
height,
nodeRootBlockHeader,
genericErr,
)
} else {
return genericErr
}
}
10 changes: 5 additions & 5 deletions engine/access/rpc/backend/backend_accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (b *backendAccounts) GetAccountAtBlockHeight(
) (*flow.Account, error) {
blockID, err := b.headers.BlockIDByHeight(height)
if err != nil {
return nil, rpc.ConvertStorageError(err)
return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

account, err := b.getAccountAtBlock(ctx, address, blockID, height)
Expand Down Expand Up @@ -108,7 +108,7 @@ func (b *backendAccounts) GetAccountBalanceAtBlockHeight(
) (uint64, error) {
blockID, err := b.headers.BlockIDByHeight(height)
if err != nil {
return 0, rpc.ConvertStorageError(err)
return 0, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

balance, err := b.getAccountBalanceAtBlock(ctx, address, blockID, height)
Expand Down Expand Up @@ -176,7 +176,7 @@ func (b *backendAccounts) GetAccountKeyAtBlockHeight(
) (*flow.AccountPublicKey, error) {
blockID, err := b.headers.BlockIDByHeight(height)
if err != nil {
return nil, rpc.ConvertStorageError(err)
return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

accountKey, err := b.getAccountKeyAtBlock(ctx, address, keyIndex, blockID, height)
Expand All @@ -196,7 +196,7 @@ func (b *backendAccounts) GetAccountKeysAtBlockHeight(
) ([]flow.AccountPublicKey, error) {
blockID, err := b.headers.BlockIDByHeight(height)
if err != nil {
return nil, rpc.ConvertStorageError(err)
return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

accountKeys, err := b.getAccountKeysAtBlock(ctx, address, blockID, height)
Expand Down Expand Up @@ -400,7 +400,7 @@ func (b *backendAccounts) getAccountFromLocalStorage(
// make sure data is available for the requested block
account, err := b.scriptExecutor.GetAccountAtBlockHeight(ctx, address, height)
if err != nil {
return nil, convertAccountError(err, address, height)
return nil, convertAccountError(resolveHeightError(b.state.Params(), height, err), address, height)
}
return account, nil
}
Expand Down
8 changes: 8 additions & 0 deletions engine/access/rpc/backend/backend_accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ func (s *BackendAccountsSuite) TestGetAccountFromStorage_Fails() {
scriptExecutor.On("GetAccountAtBlockHeight", mock.Anything, s.failingAddress, s.block.Header.Height).
Return(nil, tt.err).Times(3)

s.state.On("Params").Return(s.params).Times(3)

s.Run(fmt.Sprintf("GetAccount - fails with %v", tt.err), func() {
s.testGetAccount(ctx, backend, tt.statusCode)
})
Expand All @@ -247,6 +249,9 @@ func (s *BackendAccountsSuite) TestGetAccountFromStorage_Fails() {
})

s.Run(fmt.Sprintf("GetAccountAtBlockHeight - fails with %v", tt.err), func() {
s.params.On("SporkRootBlockHeight").Return(s.block.Header.Height-10, nil)
s.params.On("SealedRoot").Return(s.block.Header, nil)

s.testGetAccountAtBlockHeight(ctx, backend, tt.statusCode)
})
}
Expand Down Expand Up @@ -279,6 +284,9 @@ func (s *BackendAccountsSuite) TestGetAccountFromFailover_HappyPath() {
})

s.Run(fmt.Sprintf("GetAccountAtBlockHeight - happy path - recovers %v", errToReturn), func() {
s.params.On("SporkRootBlockHeight").Return(s.block.Header.Height-10, nil)
s.params.On("SealedRoot").Return(s.block.Header, nil)

s.testGetAccountAtBlockHeight(ctx, backend, codes.OK)
})
}
Expand Down
2 changes: 1 addition & 1 deletion engine/access/rpc/backend/backend_block_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (b *backendBlockDetails) GetBlockByID(ctx context.Context, id flow.Identifi
func (b *backendBlockDetails) GetBlockByHeight(ctx context.Context, height uint64) (*flow.Block, flow.BlockStatus, error) {
block, err := b.blocks.ByHeight(height)
if err != nil {
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

stat, err := b.getBlockStatus(ctx, block)
Expand Down
2 changes: 1 addition & 1 deletion engine/access/rpc/backend/backend_block_headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (b *backendBlockHeaders) GetBlockHeaderByID(ctx context.Context, id flow.Id
func (b *backendBlockHeaders) GetBlockHeaderByHeight(ctx context.Context, height uint64) (*flow.Header, flow.BlockStatus, error) {
header, err := b.headers.ByHeight(height)
if err != nil {
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(err)
return nil, flow.BlockStatusUnknown, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), height, err))
}

stat, err := b.getBlockStatus(ctx, header)
Expand Down
2 changes: 1 addition & 1 deletion engine/access/rpc/backend/backend_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (b *backendEvents) GetEventsForHeightRange(
// and avoids calculating header.ID() for each block.
blockID, err := b.headers.BlockIDByHeight(i)
if err != nil {
return nil, rpc.ConvertStorageError(fmt.Errorf("failed to get blockID for %d: %w", i, err))
return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), i, err))
}
header, err := b.headers.ByBlockID(blockID)
if err != nil {
Expand Down
33 changes: 33 additions & 0 deletions engine/access/rpc/backend/backend_events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,39 @@ func (s *BackendEventsSuite) TestGetEventsForHeightRange_HandlesErrors() {
s.Assert().Equal(codes.OutOfRange, status.Code(err))
s.Assert().Nil(response)
})

s.state.On("Params").Return(s.params)

s.Run("returns error for startHeight < spork root height", func() {
backend := s.defaultBackend()

sporkRootHeight := s.blocks[0].Header.Height - 10
startHeight := sporkRootHeight - 1

s.params.On("SporkRootBlockHeight").Return(sporkRootHeight).Once()
s.params.On("SealedRoot").Return(s.rootHeader, nil).Once()

response, err := backend.GetEventsForHeightRange(ctx, targetEvent, startHeight, endHeight, encoding)
s.Assert().Equal(codes.NotFound, status.Code(err))
s.Assert().ErrorContains(err, "Try to use a historic node")
s.Assert().Nil(response)
})

s.Run("returns error for startHeight < node root height", func() {
backend := s.defaultBackend()

sporkRootHeight := s.blocks[0].Header.Height - 10
nodeRootHeader := unittest.BlockHeaderWithHeight(s.blocks[0].Header.Height)
startHeight := nodeRootHeader.Height - 5

s.params.On("SporkRootBlockHeight").Return(sporkRootHeight).Once()
s.params.On("SealedRoot").Return(nodeRootHeader, nil).Once()

response, err := backend.GetEventsForHeightRange(ctx, targetEvent, startHeight, endHeight, encoding)
s.Assert().Equal(codes.NotFound, status.Code(err))
s.Assert().ErrorContains(err, "Try to use a different Access node")
s.Assert().Nil(response)
})
}

func (s *BackendEventsSuite) TestGetEventsForBlockIDs_HandlesErrors() {
Expand Down
2 changes: 1 addition & 1 deletion engine/access/rpc/backend/backend_scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (b *backendScripts) ExecuteScriptAtBlockHeight(
) ([]byte, error) {
header, err := b.headers.ByHeight(blockHeight)
if err != nil {
return nil, rpc.ConvertStorageError(err)
return nil, rpc.ConvertStorageError(resolveHeightError(b.state.Params(), blockHeight, err))
}

return b.executeScript(ctx, newScriptExecutionRequest(header.ID(), blockHeight, script, arguments))
Expand Down
82 changes: 79 additions & 3 deletions engine/access/rpc/backend/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
execproto "github.com/onflow/flow/protobuf/go/flow/execution"
"github.com/rs/zerolog"
"github.com/sony/gobreaker"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -1493,7 +1492,7 @@ func (suite *Suite) TestGetExecutionResultByID() {
// execute request
_, err = backend.GetExecutionResultByID(ctx, nonexistingID)

assert.Error(suite.T(), err)
suite.Assert().Error(err)
})

suite.Run("existing execution result id", func() {
Expand Down Expand Up @@ -1555,7 +1554,7 @@ func (suite *Suite) TestGetExecutionResultByBlockID() {
// execute request
_, err = backend.GetExecutionResultForBlockID(ctx, nonexistingBlockID)

assert.Error(suite.T(), err)
suite.Assert().Error(err)
})

suite.Run("existing execution results", func() {
Expand Down Expand Up @@ -2241,3 +2240,80 @@ func (suite *Suite) defaultBackendParams() Params {
VersionControl: suite.versionControl,
}
}

// TestResolveHeightError tests the resolveHeightError function for various scenarios where the block height
// is below the spork root height, below the node root height, above the node root height, or when a different
// error is provided. It validates that resolveHeightError returns an appropriate error message for each case.
//
// Test cases:
// 1) If height is below the spork root height, it suggests using a historic node.
// 2) If height is below the node root height, it suggests using a different Access node.
// 3) If height is above the node root height, it returns the original error without modification.
// 4) If a non-storage-related error is provided, it returns the error as is.
func (suite *Suite) TestResolveHeightError() {
tests := []struct {
name string
height uint64
sporkRootHeight uint64
nodeRootHeight uint64
genericErr error
expectedErrorMsg string
expectOriginalErr bool
}{
{
name: "height below spork root height",
height: uint64(50),
sporkRootHeight: uint64(100),
nodeRootHeight: uint64(200),
genericErr: storage.ErrNotFound,
expectedErrorMsg: "block height %d is less than the spork root block height 100. Try to use a historic node: %v"},
{
name: "height below node root height",
height: uint64(150),
sporkRootHeight: uint64(100),
nodeRootHeight: uint64(200),
genericErr: storage.ErrNotFound,
expectedErrorMsg: "block height %d is less than the node's root block height 200. Try to use a different Access node: %v",
expectOriginalErr: false,
},
{
name: "height above node root height",
height: uint64(205),
sporkRootHeight: uint64(100),
nodeRootHeight: uint64(200),
genericErr: storage.ErrNotFound,
expectedErrorMsg: "%v",
expectOriginalErr: true,
},
{
name: "non-storage related error",
height: uint64(150),
sporkRootHeight: uint64(100),
nodeRootHeight: uint64(200),
genericErr: fmt.Errorf("some other error"),
expectedErrorMsg: "%v",
expectOriginalErr: true,
},
}

for _, test := range tests {
suite.T().Run(test.name, func(t *testing.T) {
stateParams := protocol.NewParams(suite.T())

if errors.Is(test.genericErr, storage.ErrNotFound) {
stateParams.On("SporkRootBlockHeight").Return(test.sporkRootHeight).Once()
sealedRootHeader := unittest.BlockHeaderWithHeight(test.nodeRootHeight)
stateParams.On("SealedRoot").Return(sealedRootHeader, nil).Once()
}

err := resolveHeightError(stateParams, test.height, test.genericErr)

if test.expectOriginalErr {
suite.Assert().True(errors.Is(err, test.genericErr))
} else {
expectedError := fmt.Sprintf(test.expectedErrorMsg, test.height, test.genericErr)
suite.Assert().Equal(err.Error(), expectedError)
}
})
}
}
Loading