-
Notifications
You must be signed in to change notification settings - Fork 208
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
Error for unused Update operation #1655
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -746,7 +746,7 @@ type ( | |
// workflow completion callback. Only settable by the SDK - e.g. [temporalnexus.workflowRunOperation]. | ||
callbacks []*commonpb.Callback | ||
// links. Only settable by the SDK - e.g. [temporalnexus.workflowRunOperation]. | ||
links []*commonpb.Link | ||
links []*commonpb.Link | ||
} | ||
|
||
// WithStartWorkflowOperation is a type of operation that can be executed as part of a workflow start. | ||
|
@@ -1082,6 +1082,9 @@ func (op *UpdateWithStartWorkflowOperation) Get(ctx context.Context) (WorkflowUp | |
case <-op.doneCh: | ||
return op.handle, op.err | ||
case <-ctx.Done(): | ||
if !op.executed.Load() { | ||
return nil, fmt.Errorf("%v: %v", ctx.Err(), fmt.Errorf("operation was not executed")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for error wrapping shouldn't one of these be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed! I just learned that Go 1.20 supports multiple |
||
} | ||
return nil, ctx.Err() | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1027,6 +1027,35 @@ func (s *workflowRunSuite) TestExecuteWorkflowWithUpdate_Retry() { | |
s.NoError(err) | ||
} | ||
|
||
func (s *workflowRunSuite) TestExecuteWorkflowWithUpdate_OperationNotExecuted() { | ||
s.workflowServiceClient.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any(), gomock.Any()). | ||
Return(&workflowservice.StartWorkflowExecutionResponse{ | ||
RunId: runID, | ||
}, nil) | ||
|
||
updOp := NewUpdateWithStartWorkflowOperation( | ||
UpdateWorkflowOptions{ | ||
UpdateName: "update", | ||
WaitForStage: WorkflowUpdateStageCompleted, | ||
}) | ||
|
||
ctxWithTimeout, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) | ||
defer cancel() | ||
|
||
_, err := s.workflowClient.ExecuteWorkflow( | ||
ctxWithTimeout, | ||
StartWorkflowOptions{ | ||
ID: workflowID, | ||
TaskQueue: taskqueue, | ||
// WithStartOperation is not specified! | ||
}, workflowType, | ||
) | ||
require.NoError(s.T(), err) | ||
|
||
_, err = updOp.Get(ctxWithTimeout) | ||
require.EqualError(s.T(), err, "context deadline exceeded: operation was not executed") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming the cause of the deadline should be on the right. |
||
} | ||
|
||
func (s *workflowRunSuite) TestExecuteWorkflowWithUpdate_Abort() { | ||
tests := []struct { | ||
name string | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh? Seems correct now.