Skip to content

Commit

Permalink
Update panic handling (#1119)
Browse files Browse the repository at this point in the history
Recover panics from validation by converting to PanicError - this will
fail the update but not the WFT and will not trigger the
WorkflowPanicPolicy. Panics from update execution still propagate out
and will be handled by the task processing loop in accordance with the
WorkflowPanicPolicy.
  • Loading branch information
Matt McShane committed May 25, 2023
1 parent 720dab0 commit ed13374
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
6 changes: 6 additions & 0 deletions internal/internal_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ func newUpdateHandler(

// validate invokes the update's validation function.
func (h *updateHandler) validate(ctx Context, input []interface{}) (err error) {
defer func() {
if p := recover(); p != nil {
st := getStackTraceRaw("update validator [panic]:", 7, 0)
err = newPanicError(fmt.Sprintf("update validator panic: %v", p), st)
}
}()
_, err = executeFunctionWithWorkflowContext(ctx, h.validateFn, input)
return err
}
Expand Down
13 changes: 8 additions & 5 deletions internal/internal_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ var testSDKFlags = newSDKFlags(
&workflowservice.GetSystemInfoResponse_Capabilities{SdkMetadata: true},
)

func TestUpdateHandlerPanicsPropagate(t *testing.T) {
func TestUpdateHandlerPanicHandling(t *testing.T) {
t.Parallel()

env := &workflowEnvironmentImpl{
Expand All @@ -107,12 +107,15 @@ func TestUpdateHandlerPanicsPropagate(t *testing.T) {
in := UpdateInput{Name: t.Name(), Args: []interface{}{}}

t.Run("ValidateUpdate", func(t *testing.T) {
defer func() { require.NotNil(t, recover(), "expected a panic") }()
_ = interceptor.inboundInterceptor.ValidateUpdate(ctx, &in)
err = interceptor.inboundInterceptor.ValidateUpdate(ctx, &in)
var panicerr *PanicError
require.ErrorAs(t, err, &panicerr,
"panic during validate should be converted to an error to fail the update")
})
t.Run("ExecuteUpdate", func(t *testing.T) {
defer func() { require.NotNil(t, recover(), "expected a panic") }()
_, _ = interceptor.inboundInterceptor.ExecuteUpdate(ctx, &in)
require.Panics(t, func() {
_, _ = interceptor.inboundInterceptor.ExecuteUpdate(ctx, &in)
}, "panic during execution should be propagated to reach the WorkflowPanicPolicy")
})
}

Expand Down

0 comments on commit ed13374

Please sign in to comment.