From e627a0e1090c58b13c81fe17c5c9eff2faa19596 Mon Sep 17 00:00:00 2001 From: Lautaro Fernandez Date: Fri, 27 Mar 2026 01:16:10 -0300 Subject: [PATCH 1/3] remote capabilities errors are properly handled as caperrors --- .../executable/request/client_request.go | 34 +++++++++++- .../executable/request/client_request_test.go | 53 +++++++++++++++++++ .../workflows/v2/capability_executor.go | 3 +- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/core/capabilities/remote/executable/request/client_request.go b/core/capabilities/remote/executable/request/client_request.go index 5afda38e4e2..6ec4eb4502c 100644 --- a/core/capabilities/remote/executable/request/client_request.go +++ b/core/capabilities/remote/executable/request/client_request.go @@ -16,6 +16,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/beholder" commoncap "github.com/smartcontractkit/chainlink-common/pkg/capabilities" + caperrors "github.com/smartcontractkit/chainlink-common/pkg/capabilities/errors" "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-protos/workflows/go/events" @@ -27,6 +28,32 @@ import ( p2ptypes "github.com/smartcontractkit/chainlink/v2/core/services/p2p/types" ) +// errRemoteCapabilityExecute preserves the legacy "TRANSPORT : ErrorMsg" string from the +// remote executable client while wrapping a deserialized caperrors.Error so callers can +// errors.As into caperrors.Error after RPC (see capability_executor metrics). +type errRemoteCapabilityExecute struct { + s string + wrap caperrors.Error +} + +func (e *errRemoteCapabilityExecute) Error() string { return e.s } + +func (e *errRemoteCapabilityExecute) Unwrap() error { return e.wrap } + +func newRemoteCapabilityExecuteError(transportErr types.Error, errMsg string) error { + return &errRemoteCapabilityExecute{ + s: fmt.Sprintf("%s : %s", transportErr, errMsg), + wrap: caperrors.DeserializeErrorFromString(errMsg), + } +} + +func newRemoteCapabilityExecuteErrorWithMessage(display string, errMsg string) error { + return &errRemoteCapabilityExecute{ + s: display, + wrap: caperrors.DeserializeErrorFromString(errMsg), + } +} + type clientResponse struct { Result []byte Err error @@ -351,9 +378,12 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err } if c.errorCount[msg.ErrorMsg] == c.requiredIdenticalResponses { - c.sendResponse(clientResponse{Err: fmt.Errorf("%s : %s", msg.Error, msg.ErrorMsg)}) + c.sendResponse(clientResponse{Err: newRemoteCapabilityExecuteError(msg.Error, msg.ErrorMsg)}) } else if c.totalErrorCount == c.remoteNodeCount-c.requiredIdenticalResponses+1 { - c.sendResponse(clientResponse{Err: fmt.Errorf("received %d errors, last error %s : %s", c.totalErrorCount, msg.Error, msg.ErrorMsg)}) + c.sendResponse(clientResponse{Err: newRemoteCapabilityExecuteErrorWithMessage( + fmt.Sprintf("received %d errors, last error %s : %s", c.totalErrorCount, msg.Error, msg.ErrorMsg), + msg.ErrorMsg, + )}) } } return nil diff --git a/core/capabilities/remote/executable/request/client_request_test.go b/core/capabilities/remote/executable/request/client_request_test.go index 69bc64cef7c..a6d8d2dfe60 100644 --- a/core/capabilities/remote/executable/request/client_request_test.go +++ b/core/capabilities/remote/executable/request/client_request_test.go @@ -14,6 +14,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/beholder/beholdertest" commoncap "github.com/smartcontractkit/chainlink-common/pkg/capabilities" + caperrors "github.com/smartcontractkit/chainlink-common/pkg/capabilities/errors" "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb" "github.com/smartcontractkit/chainlink-protos/cre/go/values" "github.com/smartcontractkit/chainlink-protos/workflows/go/events" @@ -229,6 +230,58 @@ func Test_ClientRequest_MessageValidation(t *testing.T) { response := <-req.ResponseChan() assert.Equal(t, fmt.Sprintf("%s : %s", types.Error_INTERNAL_ERROR, assert.AnError.Error()), response.Err.Error()) + + var capErr caperrors.Error + require.True(t, errors.As(response.Err, &capErr)) + assert.Equal(t, caperrors.OriginSystem, capErr.Origin(), "non-serialized ErrorMsg falls back to private system capability error") + assert.Equal(t, caperrors.VisibilityPrivate, capErr.Visibility()) + assert.Equal(t, caperrors.Unknown, capErr.Code()) + }) + + t.Run("Error response with serialized caperrors unwraps correctly as usererror", func(t *testing.T) { + ctx := t.Context() + capabilityPeers, capDonInfo, capInfo := capabilityDon(t, 4, 1) + + dispatcher := &clientRequestTestDispatcher{msgs: make(chan *types.MessageBody, 100)} + req, err := request.NewClientExecuteRequest(ctx, logger.Test(t), capabilityRequest, capInfo, + workflowDonInfo, dispatcher, 10*time.Minute, nil, "") + require.NoError(t, err) + defer req.Cancel(errors.New("test end")) + + <-dispatcher.msgs + <-dispatcher.msgs + assert.Empty(t, dispatcher.msgs) + + serialized := caperrors.NewPublicUserError(errors.New("rpc error: EVM error invalid argument"), caperrors.FailedPrecondition).SerializeToRemoteString() + msgWithError := &types.MessageBody{ + CapabilityId: capInfo.ID, + CapabilityDonId: capDonInfo.ID, + CallerDonId: workflowDonInfo.ID, + Method: types.MethodExecute, + Payload: rawResponse, + MessageId: []byte("messageID"), + Error: types.Error_INTERNAL_ERROR, + ErrorMsg: serialized, + } + + msgWithError.Sender = capabilityPeers[0][:] + err = req.OnMessage(ctx, msgWithError) + require.NoError(t, err) + + msgWithError.Sender = capabilityPeers[1][:] + err = req.OnMessage(ctx, msgWithError) + require.NoError(t, err) + + response := <-req.ResponseChan() + + wantDisplay := fmt.Sprintf("%s : %s", types.Error_INTERNAL_ERROR, serialized) + assert.Equal(t, wantDisplay, response.Err.Error(), "It should be equal to 'Public:User:FailedPrecondition:rpc error: EVM error invalid argument'") + + var capErr caperrors.Error + require.True(t, errors.As(response.Err, &capErr)) + assert.Equal(t, caperrors.OriginUser, capErr.Origin()) + assert.Equal(t, caperrors.VisibilityPublic, capErr.Visibility()) + assert.Equal(t, caperrors.FailedPrecondition, capErr.Code()) }) t.Run("Send three messages with different errors", func(t *testing.T) { diff --git a/core/services/workflows/v2/capability_executor.go b/core/services/workflows/v2/capability_executor.go index 244e6bbb72a..9b1e798d960 100644 --- a/core/services/workflows/v2/capability_executor.go +++ b/core/services/workflows/v2/capability_executor.go @@ -226,7 +226,8 @@ func (c *ExecutionHelper) callCapability(ctx context.Context, request *sdkpb.Cap execLogger.Debugw("Capability execution failed", "err", err) _ = events.EmitCapabilityFinishedEvent(ctx, loggerLabels, c.WorkflowExecutionID, request.Id, meteringRef, store.StatusErrored, request.Method, err) - c.metrics.With(platform.KeyCapabilityID, request.Id, platform.KeyCapabilityErrorCode, caperrors.Unknown.String()).IncrementCapabilityFailureCounter(ctx) + //TODO shouldn't all capabilities *always* return a typed error, and if so shouldn't the following metric alert us there's a bug we need to fix? + c.metrics.With(platform.KeyCapabilityID, request.Id, platform.KeyCapabilityErrorCode, "BUG").IncrementCapabilityFailureCounter(ctx) c.metrics.IncrementTotalWorkflowStepErrorsCounter(ctx) return nil, fmt.Errorf("failed to execute capability: %w", err) } From fc16e88c2988f92dc45df627bbdba44a56524249 Mon Sep 17 00:00:00 2001 From: Lautaro Fernandez Date: Fri, 27 Mar 2026 01:25:45 -0300 Subject: [PATCH 2/3] lint --- .../remote/executable/request/client_request.go | 10 +++++----- .../remote/executable/request/client_request_test.go | 4 ++-- core/services/workflows/v2/capability_executor.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/core/capabilities/remote/executable/request/client_request.go b/core/capabilities/remote/executable/request/client_request.go index 6ec4eb4502c..f7c061eaeaf 100644 --- a/core/capabilities/remote/executable/request/client_request.go +++ b/core/capabilities/remote/executable/request/client_request.go @@ -31,24 +31,24 @@ import ( // errRemoteCapabilityExecute preserves the legacy "TRANSPORT : ErrorMsg" string from the // remote executable client while wrapping a deserialized caperrors.Error so callers can // errors.As into caperrors.Error after RPC (see capability_executor metrics). -type errRemoteCapabilityExecute struct { +type errRemoteCapabilityExecuteError struct { s string wrap caperrors.Error } -func (e *errRemoteCapabilityExecute) Error() string { return e.s } +func (e *errRemoteCapabilityExecuteError) Error() string { return e.s } -func (e *errRemoteCapabilityExecute) Unwrap() error { return e.wrap } +func (e *errRemoteCapabilityExecuteError) Unwrap() error { return e.wrap } func newRemoteCapabilityExecuteError(transportErr types.Error, errMsg string) error { - return &errRemoteCapabilityExecute{ + return &errRemoteCapabilityExecuteError{ s: fmt.Sprintf("%s : %s", transportErr, errMsg), wrap: caperrors.DeserializeErrorFromString(errMsg), } } func newRemoteCapabilityExecuteErrorWithMessage(display string, errMsg string) error { - return &errRemoteCapabilityExecute{ + return &errRemoteCapabilityExecuteError{ s: display, wrap: caperrors.DeserializeErrorFromString(errMsg), } diff --git a/core/capabilities/remote/executable/request/client_request_test.go b/core/capabilities/remote/executable/request/client_request_test.go index a6d8d2dfe60..d35f8c11082 100644 --- a/core/capabilities/remote/executable/request/client_request_test.go +++ b/core/capabilities/remote/executable/request/client_request_test.go @@ -232,7 +232,7 @@ func Test_ClientRequest_MessageValidation(t *testing.T) { assert.Equal(t, fmt.Sprintf("%s : %s", types.Error_INTERNAL_ERROR, assert.AnError.Error()), response.Err.Error()) var capErr caperrors.Error - require.True(t, errors.As(response.Err, &capErr)) + require.ErrorAs(t, response.Err, &capErr) assert.Equal(t, caperrors.OriginSystem, capErr.Origin(), "non-serialized ErrorMsg falls back to private system capability error") assert.Equal(t, caperrors.VisibilityPrivate, capErr.Visibility()) assert.Equal(t, caperrors.Unknown, capErr.Code()) @@ -278,7 +278,7 @@ func Test_ClientRequest_MessageValidation(t *testing.T) { assert.Equal(t, wantDisplay, response.Err.Error(), "It should be equal to 'Public:User:FailedPrecondition:rpc error: EVM error invalid argument'") var capErr caperrors.Error - require.True(t, errors.As(response.Err, &capErr)) + require.ErrorAs(t, response.Err, &capErr) assert.Equal(t, caperrors.OriginUser, capErr.Origin()) assert.Equal(t, caperrors.VisibilityPublic, capErr.Visibility()) assert.Equal(t, caperrors.FailedPrecondition, capErr.Code()) diff --git a/core/services/workflows/v2/capability_executor.go b/core/services/workflows/v2/capability_executor.go index 9b1e798d960..54bb97a4d29 100644 --- a/core/services/workflows/v2/capability_executor.go +++ b/core/services/workflows/v2/capability_executor.go @@ -226,7 +226,7 @@ func (c *ExecutionHelper) callCapability(ctx context.Context, request *sdkpb.Cap execLogger.Debugw("Capability execution failed", "err", err) _ = events.EmitCapabilityFinishedEvent(ctx, loggerLabels, c.WorkflowExecutionID, request.Id, meteringRef, store.StatusErrored, request.Method, err) - //TODO shouldn't all capabilities *always* return a typed error, and if so shouldn't the following metric alert us there's a bug we need to fix? + // TODO shouldn't all capabilities *always* return a typed error, and if so shouldn't the following metric alert us there's a bug we need to fix? c.metrics.With(platform.KeyCapabilityID, request.Id, platform.KeyCapabilityErrorCode, "BUG").IncrementCapabilityFailureCounter(ctx) c.metrics.IncrementTotalWorkflowStepErrorsCounter(ctx) return nil, fmt.Errorf("failed to execute capability: %w", err) From fab3ef226af60964738b147190425d34c31f3f4b Mon Sep 17 00:00:00 2001 From: Juan Lautaro Fernandez Date: Fri, 27 Mar 2026 02:58:35 -0300 Subject: [PATCH 3/3] Update core/capabilities/remote/executable/request/client_request.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- core/capabilities/remote/executable/request/client_request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/capabilities/remote/executable/request/client_request.go b/core/capabilities/remote/executable/request/client_request.go index f7c061eaeaf..a771be2ad7c 100644 --- a/core/capabilities/remote/executable/request/client_request.go +++ b/core/capabilities/remote/executable/request/client_request.go @@ -28,7 +28,7 @@ import ( p2ptypes "github.com/smartcontractkit/chainlink/v2/core/services/p2p/types" ) -// errRemoteCapabilityExecute preserves the legacy "TRANSPORT : ErrorMsg" string from the +// errRemoteCapabilityExecuteError preserves the legacy "TRANSPORT : ErrorMsg" string from the // remote executable client while wrapping a deserialized caperrors.Error so callers can // errors.As into caperrors.Error after RPC (see capability_executor metrics). type errRemoteCapabilityExecuteError struct {