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

[Keystone] Standard error message for remote execution errors #15615

Merged
merged 1 commit into from
Dec 10, 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
6 changes: 3 additions & 3 deletions core/capabilities/remote/executable/endtoend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func Test_RemoteExecutionCapability_CapabilityError(t *testing.T) {

methods = append(methods, func(ctx context.Context, caller commoncap.ExecutableCapability) {
executeCapability(ctx, t, caller, transmissionSchedule, func(t *testing.T, responseCh commoncap.CapabilityResponse, responseError error) {
assert.Equal(t, "error executing request: failed to execute capability: an error", responseError.Error())
assert.Equal(t, "error executing request: failed to execute capability", responseError.Error())
})
})

Expand All @@ -102,12 +102,12 @@ func Test_RemoteExecutableCapability_RandomCapabilityError(t *testing.T) {

methods = append(methods, func(ctx context.Context, caller commoncap.ExecutableCapability) {
executeCapability(ctx, t, caller, transmissionSchedule, func(t *testing.T, responseCh commoncap.CapabilityResponse, responseError error) {
assert.Equal(t, "error executing request: request expired by executable client", responseError.Error())
assert.Equal(t, "error executing request: failed to execute capability", responseError.Error())
})
})

for _, method := range methods {
testRemoteExecutableCapability(ctx, t, capability, 10, 9, 10*time.Millisecond, 10, 9, 10*time.Minute,
testRemoteExecutableCapability(ctx, t, capability, 10, 9, 1*time.Second, 10, 9, 10*time.Minute,
method)
}
}
Expand Down
13 changes: 9 additions & 4 deletions core/capabilities/remote/executable/request/server_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package request

import (
"context"
"errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -48,6 +49,8 @@ type ServerRequest struct {
lggr logger.Logger
}

var errExternalErrorMsg = errors.New("failed to execute capability")

func NewServerRequest(capability capabilities.ExecutableCapability, method string, capabilityID string, capabilityDonID uint32,
capabilityPeerID p2ptypes.PeerID,
callingDon commoncap.DON, requestID string,
Expand Down Expand Up @@ -228,20 +231,22 @@ func executeCapabilityRequest(ctx context.Context, lggr logger.Logger, capabilit
payload []byte) ([]byte, error) {
capabilityRequest, err := pb.UnmarshalCapabilityRequest(payload)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal capability request: %w", err)
lggr.Errorw("failed to unmarshal capability request", "err", err)
return nil, errExternalErrorMsg
}

lggr.Debugw("executing capability", "metadata", capabilityRequest.Metadata)
capResponse, err := capability.Execute(ctx, capabilityRequest)

if err != nil {
lggr.Debugw("received execution error", "workflowExecutionID", capabilityRequest.Metadata.WorkflowExecutionID, "error", err)
return nil, fmt.Errorf("failed to execute capability: %w", err)
lggr.Errorw("received execution error", "workflowExecutionID", capabilityRequest.Metadata.WorkflowExecutionID, "error", err)
return nil, errExternalErrorMsg
}

responsePayload, err := pb.MarshalCapabilityResponse(capResponse)
if err != nil {
return nil, fmt.Errorf("failed to marshal capability response: %w", err)
lggr.Errorw("failed to marshal capability request", "err", err)
return nil, errExternalErrorMsg
}

lggr.Debugw("received execution results", "workflowExecutionID", capabilityRequest.Metadata.WorkflowExecutionID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,9 @@ func Test_ServerRequest_MessageValidation(t *testing.T) {
require.NoError(t, err)
assert.Len(t, dispatcher.msgs, 2)
assert.Equal(t, types.Error_INTERNAL_ERROR, dispatcher.msgs[0].Error)
assert.Equal(t, "failed to execute capability: an error", dispatcher.msgs[0].ErrorMsg)
assert.Equal(t, "failed to execute capability", dispatcher.msgs[0].ErrorMsg)
assert.Equal(t, types.Error_INTERNAL_ERROR, dispatcher.msgs[1].Error)
assert.Equal(t, "failed to execute capability: an error", dispatcher.msgs[1].ErrorMsg)
assert.Equal(t, "failed to execute capability", dispatcher.msgs[1].ErrorMsg)
})

t.Run("Execute capability", func(t *testing.T) {
Expand Down
Loading