Skip to content

Commit 8b534e3

Browse files
authored
changes account hooks to not halt on first failure (#3328)
1 parent b92433e commit 8b534e3

File tree

2 files changed

+39
-1
lines changed

2 files changed

+39
-1
lines changed

worker/pkg/workflows/ee/account_hooks/workflow/workflow.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package accounthook_workflow
22

33
import (
4+
"errors"
45
"fmt"
56
"time"
67

@@ -60,12 +61,16 @@ func ProcessAccountHook(wfctx workflow.Context, req *ProcessAccountHookRequest)
6061
)
6162
}
6263

64+
errs := []error{}
6365
for _, future := range futures {
6466
var execResp *execute_hook_activity.ExecuteHookResponse
6567
if err := future.Get(wfctx, &execResp); err != nil {
66-
return nil, fmt.Errorf("error executing hook: %w", err)
68+
errs = append(errs, fmt.Errorf("error executing hook: %w", err))
6769
}
6870
}
71+
if len(errs) > 0 {
72+
return nil, fmt.Errorf("error executing hooks: %w", errors.Join(errs...))
73+
}
6974

7075
return &ProcessAccountHookResponse{}, nil
7176
}

worker/pkg/workflows/ee/account_hooks/workflow/workflow_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package accounthook_workflow
22

33
import (
4+
"errors"
5+
"strings"
46
"testing"
57

68
accounthook_events "github.com/nucleuscloud/neosync/internal/ee/events"
@@ -42,3 +44,34 @@ func Test_ProcessAccountHook(t *testing.T) {
4244

4345
env.AssertExpectations(t)
4446
}
47+
48+
func Test_ProcessAccountHook_Error(t *testing.T) {
49+
var ts testsuite.WorkflowTestSuite
50+
env := ts.NewTestWorkflowEnvironment()
51+
52+
var hooksByEventActivity *hooks_by_event_activity.Activity
53+
env.OnActivity(hooksByEventActivity.GetAccountHooksByEvent, mock.Anything, mock.Anything).
54+
Return(&hooks_by_event_activity.RunHooksByEventResponse{
55+
HookIds: []string{"hook1", "hook2"},
56+
}, nil).Once()
57+
58+
var executeHookActivity *execute_hook_activity.Activity
59+
env.OnActivity(executeHookActivity.ExecuteAccountHook, mock.Anything, mock.Anything).
60+
Return(nil, errors.New("error"))
61+
62+
env.RegisterWorkflow(ProcessAccountHook)
63+
64+
env.ExecuteWorkflow(ProcessAccountHook, &ProcessAccountHookRequest{
65+
Event: accounthook_events.NewEvent_JobRunCreated("123", "456", "789"),
66+
})
67+
68+
env.AssertExpectations(t)
69+
70+
require.True(t, env.IsWorkflowCompleted())
71+
72+
workflowErr := env.GetWorkflowError()
73+
require.Error(t, workflowErr)
74+
require.Contains(t, workflowErr.Error(), "error executing hook:")
75+
// The way temporal wraps these they show up more than the number of hooks
76+
require.GreaterOrEqual(t, strings.Count(workflowErr.Error(), "error executing hook:"), 2)
77+
}

0 commit comments

Comments
 (0)