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.

Sign up for GitHub

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

P2P AppError handling #2248

Merged
merged 13 commits into from
Dec 12, 2023
+1,070 −886
Merged

P2P AppError handling #2248

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
aed63eb
AppError handling
joshua-kim Oct 31, 2023
89f47cc
nit
joshua-kim Dec 6, 2023
28b3cec
nit
joshua-kim Dec 6, 2023
0c54f1d
nit
joshua-kim Dec 6, 2023
2e5596c
lint
joshua-kim Dec 6, 2023
bb857dc
tests
joshua-kim Dec 7, 2023
de6ebed
nit
joshua-kim Dec 7, 2023
3a8d3c8
fix tests
joshua-kim Dec 7, 2023
7226dcd
Merge branch 'dev' into vm-app-error
StephenButtolph Dec 12, 2023
e70c46e
nit
joshua-kim Dec 12, 2023
e626b42
nit add test
StephenButtolph Dec 12, 2023
26f28c3
Merge branch 'vm-app-error' of github.com:ava-labs/avalanchego into v…
StephenButtolph Dec 12, 2023
18a05d5
merged
StephenButtolph Dec 12, 2023
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/DataDog/zstd v1.5.2
github.com/Microsoft/go-winio v0.5.2
github.com/NYTimes/gziphandler v1.1.1
github.com/ava-labs/coreth v0.12.10-rc.0
github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76
github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34
github.com/btcsuite/btcd/btcutil v1.1.3
github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah
github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/ava-labs/coreth v0.12.10-rc.0 h1:qmuom7rtH5hc1E3lnqrMFNLFL1TMnEVa/2O8poB1YLU=
github.com/ava-labs/coreth v0.12.10-rc.0/go.mod h1:plFm/xzvWmx1+qJ3JQSTzF8+FdaA2xu7GgY/AdaZDfk=
github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76 h1:0hYwcuOfndE1I61SvWv2dEa7N3QRJL3RpB9fmJy0sr0=
github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76/go.mod h1:dH0LOVIEm1uHKozwhDV223BOyP0ElVsYclizRJzEY4s=
github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 h1:mg9Uw6oZFJKytJxgxnl3uxZOs/SB8CVHg6Io4Tf99Zc=
github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34/go.mod h1:pJxaT9bUgeRNVmNRgtCHb7sFDIRKy7CzTQVi8gGNT6g=
github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=
Expand Down
20 changes: 20 additions & 0 deletions message/inbound_msg_builder.go
View file
Open in desktop
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,26 @@ func InboundAppRequest(
}
}

func InboundAppError(
nodeID ids.NodeID,
chainID ids.ID,
requestID uint32,
errorCode int32,
errorMessage string,
) InboundMessage {
return &inboundMessage{
nodeID: nodeID,
op: AppRequestFailedOp,
message: &p2p.AppError{
ChainId: chainID[:],
RequestId: requestID,
ErrorCode: errorCode,
ErrorMessage: errorMessage,
},
expiration: mockable.MaxTime,
}
}

func InboundAppResponse(
chainID ids.ID,
requestID uint32,
Expand Down
46 changes: 6 additions & 40 deletions message/internal_msg_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ var (
_ requestIDGetter = (*QueryFailed)(nil)
_ engineTypeGetter = (*QueryFailed)(nil)

_ fmt.Stringer = (*AppRequestFailed)(nil)
_ chainIDGetter = (*AppRequestFailed)(nil)
_ requestIDGetter = (*AppRequestFailed)(nil)

_ fmt.Stringer = (*CrossChainAppRequest)(nil)
_ sourceChainIDGetter = (*CrossChainAppRequest)(nil)
_ chainIDGetter = (*CrossChainAppRequest)(nil)
Expand Down Expand Up @@ -365,42 +361,6 @@ func InternalQueryFailed(
}
}

type AppRequestFailed struct {
ChainID ids.ID `json:"chain_id,omitempty"`
RequestID uint32 `json:"request_id,omitempty"`
}

func (m *AppRequestFailed) String() string {
return fmt.Sprintf(
"ChainID: %s RequestID: %d",
m.ChainID, m.RequestID,
)
}

func (m *AppRequestFailed) GetChainId() []byte {
return m.ChainID[:]
}

func (m *AppRequestFailed) GetRequestId() uint32 {
return m.RequestID
}

func InternalAppRequestFailed(
nodeID ids.NodeID,
chainID ids.ID,
requestID uint32,
) InboundMessage {
return &inboundMessage{
nodeID: nodeID,
op: AppRequestFailedOp,
message: &AppRequestFailed{
ChainID: chainID,
RequestID: requestID,
},
expiration: mockable.MaxTime,
}
}

type CrossChainAppRequest struct {
SourceChainID ids.ID `json:"source_chain_id,omitempty"`
DestinationChainID ids.ID `json:"destination_chain_id,omitempty"`
Expand Down Expand Up @@ -452,6 +412,8 @@ type CrossChainAppRequestFailed struct {
SourceChainID ids.ID `json:"source_chain_id,omitempty"`
DestinationChainID ids.ID `json:"destination_chain_id,omitempty"`
RequestID uint32 `json:"request_id,omitempty"`
ErrorCode int32 `json:"error_code,omitempty"`
ErrorMessage string `json:"error_message,omitempty"`
}

func (m *CrossChainAppRequestFailed) String() string {
Expand All @@ -478,6 +440,8 @@ func InternalCrossChainAppRequestFailed(
sourceChainID ids.ID,
destinationChainID ids.ID,
requestID uint32,
errorCode int32,
errorMessage string,
) InboundMessage {
return &inboundMessage{
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
nodeID: nodeID,
Expand All @@ -486,6 +450,8 @@ func InternalCrossChainAppRequestFailed(
SourceChainID: sourceChainID,
DestinationChainID: destinationChainID,
RequestID: requestID,
ErrorCode: errorCode,
ErrorMessage: errorMessage,
},
expiration: mockable.MaxTime,
}
Expand Down
5 changes: 2 additions & 3 deletions network/p2p/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import (
)

var (
ErrAppRequestFailed = errors.New("app request failed")
joshua-kim marked this conversation as resolved.
Show resolved Hide resolved
ErrRequestPending = errors.New("request pending")
ErrNoPeers = errors.New("no peers")
ErrRequestPending = errors.New("request pending")
ErrNoPeers = errors.New("no peers")
)

// AppResponseCallback is called upon receiving an AppResponse for an AppRequest
Expand Down
8 changes: 4 additions & 4 deletions network/p2p/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ func (n *Network) AppResponse(ctx context.Context, nodeID ids.NodeID, requestID
return n.router.AppResponse(ctx, nodeID, requestID, response)
}

func (n *Network) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32) error {
return n.router.AppRequestFailed(ctx, nodeID, requestID)
func (n *Network) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32, appErr *common.AppError) error {
return n.router.AppRequestFailed(ctx, nodeID, requestID, appErr)
}

func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msg []byte) error {
Expand All @@ -103,8 +103,8 @@ func (n *Network) CrossChainAppResponse(ctx context.Context, chainID ids.ID, req
return n.router.CrossChainAppResponse(ctx, chainID, requestID, response)
}

func (n *Network) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32) error {
return n.router.CrossChainAppRequestFailed(ctx, chainID, requestID)
func (n *Network) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32, appErr *common.AppError) error {
return n.router.CrossChainAppRequestFailed(ctx, chainID, requestID, appErr)
}

func (n *Network) Connected(_ context.Context, nodeID ids.NodeID, _ *version.Application) error {
Expand Down
17 changes: 11 additions & 6 deletions network/p2p/network_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ import (
"github.com/ava-labs/avalanchego/version"
)

var errFoo = &common.AppError{
Code: 123,
Message: "foo",
}

func TestAppRequestResponse(t *testing.T) {
handlerID := uint64(0x0)
request := []byte("request")
Expand Down Expand Up @@ -85,7 +90,7 @@ func TestAppRequestResponse(t *testing.T) {
sender.SendAppRequestF = func(ctx context.Context, nodeIDs set.Set[ids.NodeID], requestID uint32, request []byte) error {
for range nodeIDs {
go func() {
require.NoError(t, network.AppRequestFailed(ctx, nodeID, requestID))
require.NoError(t, network.AppRequestFailed(ctx, nodeID, requestID, errFoo))
}()
}

Expand All @@ -95,7 +100,7 @@ func TestAppRequestResponse(t *testing.T) {
callback := func(_ context.Context, actualNodeID ids.NodeID, actualResponse []byte, err error) {
defer wg.Done()

require.ErrorIs(t, err, ErrAppRequestFailed)
require.ErrorIs(t, err, errFoo)
require.Equal(t, nodeID, actualNodeID)
require.Nil(t, actualResponse)
}
Expand Down Expand Up @@ -140,14 +145,14 @@ func TestAppRequestResponse(t *testing.T) {
requestFunc: func(t *testing.T, network *Network, client *Client, sender *common.SenderTest, handler *mocks.MockHandler, wg *sync.WaitGroup) {
sender.SendCrossChainAppRequestF = func(ctx context.Context, chainID ids.ID, requestID uint32, request []byte) {
go func() {
require.NoError(t, network.CrossChainAppRequestFailed(ctx, chainID, requestID))
require.NoError(t, network.CrossChainAppRequestFailed(ctx, chainID, requestID, errFoo))
}()
}

callback := func(_ context.Context, actualChainID ids.ID, actualResponse []byte, err error) {
defer wg.Done()

require.ErrorIs(t, err, ErrAppRequestFailed)
require.ErrorIs(t, err, errFoo)
require.Equal(t, chainID, actualChainID)
require.Nil(t, actualResponse)
}
Expand Down Expand Up @@ -273,7 +278,7 @@ func TestNetworkDropMessage(t *testing.T) {
{
name: "drop unrequested app request failed",
requestFunc: func(network *Network) error {
return network.AppRequestFailed(context.Background(), ids.GenerateTestNodeID(), 0)
return network.AppRequestFailed(context.Background(), ids.GenerateTestNodeID(), 0, errFoo)
},
err: ErrUnrequestedResponse,
},
Expand All @@ -287,7 +292,7 @@ func TestNetworkDropMessage(t *testing.T) {
{
name: "drop unrequested cross-chain request failed",
requestFunc: func(network *Network) error {
return network.CrossChainAppRequestFailed(context.Background(), ids.GenerateTestID(), 0)
return network.CrossChainAppRequestFailed(context.Background(), ids.GenerateTestID(), 0, errFoo)
},
err: ErrUnrequestedResponse,
},
Expand Down
8 changes: 4 additions & 4 deletions network/p2p/router.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,15 +223,15 @@ func (r *router) AppRequest(ctx context.Context, nodeID ids.NodeID, requestID ui
//
// Any error condition propagated outside Handler application logic is
// considered fatal
func (r *router) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32) error {
func (r *router) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32, appErr *common.AppError) error {
start := time.Now()
pending, ok := r.clearAppRequest(requestID)
if !ok {
// we should never receive a timeout without a corresponding requestID
return ErrUnrequestedResponse
}

pending.AppResponseCallback(ctx, nodeID, nil, ErrAppRequestFailed)
pending.AppResponseCallback(ctx, nodeID, nil, appErr)
pending.appRequestFailedTime.Observe(float64(time.Since(start)))
return nil
}
Expand Down Expand Up @@ -316,15 +316,15 @@ func (r *router) CrossChainAppRequest(
//
// Any error condition propagated outside Handler application logic is
// considered fatal
func (r *router) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32) error {
func (r *router) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32, appErr *common.AppError) error {
start := time.Now()
pending, ok := r.clearCrossChainAppRequest(requestID)
if !ok {
// we should never receive a timeout without a corresponding requestID
return ErrUnrequestedResponse
}

pending.CrossChainAppResponseCallback(ctx, chainID, nil, ErrAppRequestFailed)
pending.CrossChainAppResponseCallback(ctx, chainID, nil, appErr)
pending.crossChainAppRequestFailedTime.Observe(float64(time.Since(start)))
return nil
}
Expand Down
Loading
Loading