diff --git a/service/frontend/operator_handler.go b/service/frontend/operator_handler.go index dcdb3f76f6c..b9ea1879ba8 100644 --- a/service/frontend/operator_handler.go +++ b/service/frontend/operator_handler.go @@ -26,10 +26,10 @@ package frontend import ( "context" - "errors" "fmt" "maps" "sync/atomic" + "time" commonpb "go.temporal.io/api/common/v1" enumspb "go.temporal.io/api/enums/v1" @@ -38,7 +38,6 @@ import ( "go.temporal.io/api/serviceerror" "go.temporal.io/api/workflowservice/v1" sdkclient "go.temporal.io/sdk/client" - "go.temporal.io/sdk/temporal" "go.temporal.io/server/api/adminservice/v1" persistencespb "go.temporal.io/server/api/persistence/v1" svc "go.temporal.io/server/client" @@ -68,7 +67,6 @@ import ( "google.golang.org/grpc/health" healthpb "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/status" - "google.golang.org/protobuf/types/known/durationpb" ) var _ OperatorHandler = (*OperatorHandlerImpl)(nil) @@ -609,14 +607,16 @@ func (h *OperatorHandlerImpl) DeleteNamespace( ) (_ *operatorservice.DeleteNamespaceResponse, retError error) { defer log.CapturePanic(h.logger, &retError) - // validate request if request == nil { return nil, errRequestNotSet } // If NamespaceDeleteDelay is not provided, the default delay configured in the cluster should be used. + var namespaceDeleteDelay time.Duration if request.NamespaceDeleteDelay == nil { - request.NamespaceDeleteDelay = durationpb.New(h.config.DeleteNamespaceNamespaceDeleteDelay()) + namespaceDeleteDelay = h.config.DeleteNamespaceNamespaceDeleteDelay() + } else { + namespaceDeleteDelay = request.NamespaceDeleteDelay.AsDuration() } // Execute workflow. @@ -629,7 +629,7 @@ func (h *OperatorHandlerImpl) DeleteNamespace( PagesPerExecution: h.config.DeleteNamespacePagesPerExecution(), ConcurrentDeleteExecutionsActivities: h.config.DeleteNamespaceConcurrentDeleteExecutionsActivities(), }, - NamespaceDeleteDelay: request.NamespaceDeleteDelay.AsDuration(), + NamespaceDeleteDelay: namespaceDeleteDelay, } sdkClient := h.sdkClientFactory.GetSystemClient() @@ -646,16 +646,11 @@ func (h *OperatorHandlerImpl) DeleteNamespace( return nil, serviceerror.NewUnavailable(fmt.Sprintf(errUnableToStartWorkflowMessage, deletenamespace.WorkflowName, err)) } - // Wait for workflow to complete. + // Wait for the workflow to complete. var wfResult deletenamespace.DeleteNamespaceWorkflowResult err = run.Get(ctx, &wfResult) if err != nil { - // Special handling for validation errors. Convert them to InvalidArgument. - var appErr *temporal.ApplicationError - if errors.As(err, &appErr) && appErr.Type() == delnserrors.ValidationErrorErrType { - return nil, serviceerror.NewInvalidArgument(appErr.Message()) - } - return nil, serviceerror.NewSystemWorkflow(&commonpb.WorkflowExecution{WorkflowId: deletenamespace.WorkflowName, RunId: run.GetRunID()}, err) + return nil, delnserrors.ToServiceError(err, run.GetID(), run.GetRunID()) } return &operatorservice.DeleteNamespaceResponse{ diff --git a/service/frontend/operator_handler_test.go b/service/frontend/operator_handler_test.go index 732bb2c0af3..66c9469c6d7 100644 --- a/service/frontend/operator_handler_test.go +++ b/service/frontend/operator_handler_test.go @@ -40,7 +40,6 @@ import ( "go.temporal.io/api/serviceerror" "go.temporal.io/api/workflowservice/v1" sdkclient "go.temporal.io/sdk/client" - "go.temporal.io/sdk/temporal" "go.temporal.io/server/api/adminservice/v1" persistencespb "go.temporal.io/server/api/persistence/v1" "go.temporal.io/server/common/cluster" @@ -1158,6 +1157,7 @@ func (s *operatorHandlerSuite) Test_DeleteNamespace() { mockRun.EXPECT().Get(gomock.Any(), gomock.Any()).Return(errors.New("workflow failed")) const RunId = "9a9f668a-58b1-427e-bed6-bf1401049f7d" mockRun.EXPECT().GetRunID().Return(RunId) + mockRun.EXPECT().GetID().Return("test-workflow-id") mockSdkClient.EXPECT().ExecuteWorkflow(gomock.Any(), gomock.Any(), "temporal-sys-delete-namespace-workflow", gomock.Any()).Return(mockRun, nil) resp, err = handler.DeleteNamespace(ctx, &operatorservice.DeleteNamespaceRequest{ Namespace: "test-namespace", @@ -1166,20 +1166,38 @@ func (s *operatorHandlerSuite) Test_DeleteNamespace() { var sysWfErr *serviceerror.SystemWorkflow s.ErrorAs(err, &sysWfErr) s.Equal(RunId, sysWfErr.WorkflowExecution.RunId) - s.Equal(fmt.Sprintf("System Workflow with WorkflowId temporal-sys-delete-namespace-workflow and RunId %s returned an error: workflow failed", RunId), err.Error()) + s.Equal(fmt.Sprintf("System Workflow with WorkflowId test-workflow-id and RunId %s returned an error: workflow failed", RunId), err.Error()) s.Nil(resp) // Workflow failed because of validation error (an attempt to delete system namespace). mockRun2 := mocksdk.NewMockWorkflowRun(s.controller) - mockRun2.EXPECT().Get(gomock.Any(), gomock.Any()).Return(temporal.NewNonRetryableApplicationError("unable to delete system namespace", delnserrors.ValidationErrorErrType, nil, nil)) + mockRun2.EXPECT().Get(gomock.Any(), gomock.Any()).Return(delnserrors.NewFailedPrecondition("unable to delete system namespace", nil)) + mockRun2.EXPECT().GetRunID().Return(RunId) + mockRun2.EXPECT().GetID().Return("test-workflow-id") mockSdkClient.EXPECT().ExecuteWorkflow(gomock.Any(), gomock.Any(), "temporal-sys-delete-namespace-workflow", gomock.Any()).Return(mockRun2, nil) resp, err = handler.DeleteNamespace(ctx, &operatorservice.DeleteNamespaceRequest{ Namespace: "temporal-system", }) s.Error(err) + var failedPreconditionErr *serviceerror.FailedPrecondition + s.ErrorAs(err, &failedPreconditionErr) + s.Equal("unable to delete system namespace", failedPreconditionErr.Error()) + s.Nil(resp) + + // Workflow failed because of validation error (an attempt to delete system namespace). + mockRun3 := mocksdk.NewMockWorkflowRun(s.controller) + mockRun3.EXPECT().Get(gomock.Any(), gomock.Any()).Return(delnserrors.NewInvalidArgument("only one of namespace or namespace ID must be set", nil)) + mockRun3.EXPECT().GetRunID().Return(RunId) + mockRun3.EXPECT().GetID().Return("test-workflow-id") + mockSdkClient.EXPECT().ExecuteWorkflow(gomock.Any(), gomock.Any(), "temporal-sys-delete-namespace-workflow", gomock.Any()).Return(mockRun3, nil) + resp, err = handler.DeleteNamespace(ctx, &operatorservice.DeleteNamespaceRequest{ + Namespace: "temporal-system", + NamespaceId: "c13c01a7-3887-4eda-ba4b-9a07a6359e7e", + }) + s.Error(err) var invalidArgErr *serviceerror.InvalidArgument s.ErrorAs(err, &invalidArgErr) - s.Equal("unable to delete system namespace", invalidArgErr.Error()) + s.Equal("only one of namespace or namespace ID must be set", invalidArgErr.Error()) s.Nil(resp) // Success case. diff --git a/service/worker/deletenamespace/activities.go b/service/worker/deletenamespace/activities.go index 77c6c5e3bcd..28c5d696c86 100644 --- a/service/worker/deletenamespace/activities.go +++ b/service/worker/deletenamespace/activities.go @@ -26,13 +26,13 @@ package deletenamespace import ( "context" + stderrors "errors" "fmt" "slices" "strings" enumspb "go.temporal.io/api/enums/v1" "go.temporal.io/api/serviceerror" - "go.temporal.io/sdk/temporal" "go.temporal.io/server/common/dynamicconfig" "go.temporal.io/server/common/headers" "go.temporal.io/server/common/log" @@ -88,11 +88,19 @@ func (a *localActivities) GetNamespaceInfoActivity(ctx context.Context, nsID nam getNamespaceResponse, err := a.metadataManager.GetNamespace(ctx, getNamespaceRequest) if err != nil { + var nsNotFoundErr *serviceerror.NamespaceNotFound + if stderrors.As(err, &nsNotFoundErr) { + ns := nsName.String() + if ns == "" { + ns = nsID.String() + } + return getNamespaceInfoResult{}, errors.NewInvalidArgument(fmt.Sprintf("namespace %s is not found", ns), err) + } return getNamespaceInfoResult{}, err } if getNamespaceResponse.Namespace == nil || getNamespaceResponse.Namespace.Info == nil || getNamespaceResponse.Namespace.Info.Id == "" { - return getNamespaceInfoResult{}, temporal.NewNonRetryableApplicationError("namespace info is corrupted", "", nil) + return getNamespaceInfoResult{}, stderrors.New("namespace info is corrupted") } return getNamespaceInfoResult{ @@ -104,36 +112,37 @@ func (a *localActivities) GetNamespaceInfoActivity(ctx context.Context, nsID nam func (a *localActivities) ValidateProtectedNamespacesActivity(_ context.Context, nsName namespace.Name) error { if slices.Contains(a.protectedNamespaces(), nsName.String()) { - return temporal.NewNonRetryableApplicationError(fmt.Sprintf("namespace %s is protected from deletion", nsName), errors.ValidationErrorErrType, nil, nil) + return errors.NewFailedPrecondition(fmt.Sprintf("namespace %s is protected from deletion", nsName), nil) } return nil } func (a *localActivities) ValidateNexusEndpointsActivity(ctx context.Context, nsID namespace.ID, nsName namespace.Name) error { - if !a.allowDeleteNamespaceIfNexusEndpointTarget() { - // Prevent deletion of a namespace that is targeted by a Nexus endpoint. - var nextPageToken []byte - for { - resp, err := a.nexusEndpointManager.ListNexusEndpoints(ctx, &persistence.ListNexusEndpointsRequest{ - LastKnownTableVersion: 0, - NextPageToken: nextPageToken, - PageSize: a.nexusEndpointListDefaultPageSize(), - }) - if err != nil { - a.logger.Error("Unable to list Nexus endpoints from persistence.", tag.WorkflowNamespace(nsName.String()), tag.Error(err)) - return fmt.Errorf("unable to list Nexus endpoints for namespace %s: %w", nsName, err) - } + if a.allowDeleteNamespaceIfNexusEndpointTarget() { + return nil + } + // Prevent deletion of a namespace that is targeted by a Nexus endpoint. + var nextPageToken []byte + for { + resp, err := a.nexusEndpointManager.ListNexusEndpoints(ctx, &persistence.ListNexusEndpointsRequest{ + LastKnownTableVersion: 0, + NextPageToken: nextPageToken, + PageSize: a.nexusEndpointListDefaultPageSize(), + }) + if err != nil { + a.logger.Error("Unable to list Nexus endpoints from persistence.", tag.WorkflowNamespace(nsName.String()), tag.WorkflowNamespaceID(nsID.String()), tag.Error(err)) + return fmt.Errorf("unable to list Nexus endpoints for namespace %s: %w", nsName, err) + } - for _, entry := range resp.Entries { - if endpointNsID := entry.GetEndpoint().GetSpec().GetTarget().GetWorker().GetNamespaceId(); endpointNsID == nsID.String() { - return temporal.NewNonRetryableApplicationError(fmt.Sprintf("cannot delete a namespace that is a target of a Nexus endpoint %s", entry.GetEndpoint().GetSpec().GetName()), errors.ValidationErrorErrType, nil, nil) - } - } - nextPageToken = resp.NextPageToken - if len(nextPageToken) == 0 { - break + for _, entry := range resp.Entries { + if endpointNsID := entry.GetEndpoint().GetSpec().GetTarget().GetWorker().GetNamespaceId(); endpointNsID == nsID.String() { + return errors.NewFailedPrecondition(fmt.Sprintf("cannot delete a namespace that is a target of a Nexus endpoint %s", entry.GetEndpoint().GetSpec().GetName()), nil) } } + nextPageToken = resp.NextPageToken + if len(nextPageToken) == 0 { + break + } } return nil } @@ -201,11 +210,12 @@ func (a *localActivities) GenerateDeletedNamespaceNameActivity(ctx context.Conte return namespace.Name(newName), nil default: logger.Error("Unable to get namespace details.", tag.Error(err)) - return namespace.EmptyName, err + return namespace.EmptyName, fmt.Errorf("unable to get namespace details: %w", err) } } // Should never get here because namespace ID is guaranteed to be unique. - panic(fmt.Sprintf("Unable to generate new name for deleted namespace %s. ID %q is not unique.", nsName, nsID)) + return namespace.EmptyName, fmt.Errorf("unable to generate new name for deleted namespace %s. ID %q is not unique", nsName, nsID) + } func (a *localActivities) RenameNamespaceActivity(ctx context.Context, previousName namespace.Name, newName namespace.Name) error { diff --git a/service/worker/deletenamespace/deleteexecutions/activities.go b/service/worker/deletenamespace/deleteexecutions/activities.go index e796121ba4b..5cca5e9464a 100644 --- a/service/worker/deletenamespace/deleteexecutions/activities.go +++ b/service/worker/deletenamespace/deleteexecutions/activities.go @@ -117,7 +117,7 @@ func (a *LocalActivities) GetNextPageTokenActivity(ctx context.Context, params G resp, err := a.visibilityManager.ListWorkflowExecutions(ctx, req) if err != nil { - a.logger.Error("Unable to list all workflows to get next page token.", tag.WorkflowNamespace(params.Namespace.String()), tag.Error(err)) + a.logger.Error("Unable to list all workflows to get next page token.", tag.WorkflowNamespace(params.Namespace.String()), tag.WorkflowNamespaceID(params.NamespaceID.String()), tag.Error(err)) return nil, err } diff --git a/service/worker/deletenamespace/deleteexecutions/workflow.go b/service/worker/deletenamespace/deleteexecutions/workflow.go index 50ab0bf53d7..f4decae2b51 100644 --- a/service/worker/deletenamespace/deleteexecutions/workflow.go +++ b/service/worker/deletenamespace/deleteexecutions/workflow.go @@ -25,7 +25,6 @@ package deleteexecutions import ( - "fmt" "time" "go.temporal.io/sdk/log" @@ -81,15 +80,12 @@ var ( func validateParams(params *DeleteExecutionsParams) error { if params.NamespaceID.IsEmpty() { - return temporal.NewNonRetryableApplicationError("namespace ID is required", "", nil) + return errors.NewInvalidArgument("namespace ID is required", nil) } - if params.Namespace.IsEmpty() { - return temporal.NewNonRetryableApplicationError("namespace is required", "", nil) + return errors.NewInvalidArgument("namespace is required", nil) } - params.Config.ApplyDefaults() - return nil } @@ -145,7 +141,7 @@ func DeleteExecutionsWorkflow(ctx workflow.Context, params DeleteExecutionsParam NextPageToken: nextPageToken, }).Get(ctx, &nextPageToken) if err != nil { - return result, fmt.Errorf("%w: GetNextPageTokenActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } runningDeleteExecutionsActivityCount++ @@ -165,7 +161,7 @@ func DeleteExecutionsWorkflow(ctx workflow.Context, params DeleteExecutionsParam // Wait for one of running activities to complete. runningDeleteExecutionsSelector.Select(ctx) if lastDeleteExecutionsActivityErr != nil { - return result, fmt.Errorf("%w: DeleteExecutionsActivity: %v", errors.ErrUnableToExecuteActivity, lastDeleteExecutionsActivityErr) + return result, lastDeleteExecutionsActivityErr } } @@ -178,7 +174,7 @@ func DeleteExecutionsWorkflow(ctx workflow.Context, params DeleteExecutionsParam for runningDeleteExecutionsActivityCount > 0 { runningDeleteExecutionsSelector.Select(ctx) if lastDeleteExecutionsActivityErr != nil { - return result, fmt.Errorf("%w: DeleteExecutionsActivity: %v", errors.ErrUnableToExecuteActivity, lastDeleteExecutionsActivityErr) + return result, lastDeleteExecutionsActivityErr } } diff --git a/service/worker/deletenamespace/deleteexecutions/workflow_test.go b/service/worker/deletenamespace/deleteexecutions/workflow_test.go index d928bead7d4..e25b1ebc3c3 100644 --- a/service/worker/deletenamespace/deleteexecutions/workflow_test.go +++ b/service/worker/deletenamespace/deleteexecutions/workflow_test.go @@ -255,8 +255,7 @@ func Test_DeleteExecutionsWorkflow_ManyExecutions_ActivityError(t *testing.T) { require.Error(t, err) var appErr *temporal.ApplicationError require.True(t, stderrors.As(err, &appErr)) - require.Contains(t, appErr.Error(), "unable to execute activity: DeleteExecutionsActivity") - require.Contains(t, appErr.Error(), "specific_error_from_activity") + require.Equal(t, appErr.Error(), "specific_error_from_activity (type: Unavailable, retryable: true)") } func Test_DeleteExecutionsWorkflow_NoActivityMocks_ManyExecutions(t *testing.T) { diff --git a/service/worker/deletenamespace/errors/errors.go b/service/worker/deletenamespace/errors/errors.go index 0939b054519..5b329ccc62d 100644 --- a/service/worker/deletenamespace/errors/errors.go +++ b/service/worker/deletenamespace/errors/errors.go @@ -28,30 +28,51 @@ import ( "errors" "fmt" + commonpb "go.temporal.io/api/common/v1" + "go.temporal.io/api/serviceerror" "go.temporal.io/sdk/temporal" ) const ( - ValidationErrorErrType = "ValidationError" + InvalidArgumentErrType = "InvalidArgument" + FailedPreconditionErrType = "FailedPrecondition" ExecutionsStillExistErrType = "ExecutionsStillExist" NoProgressErrType = "NoProgress" NotDeletedExecutionsStillExistErrType = "NotDeletedExecutionsStillExist" ) -var ( - ErrUnableToExecuteActivity = errors.New("unable to execute activity") - ErrUnableToExecuteChildWorkflow = errors.New("unable to execute child workflow") - ErrUnableToSetUpdateHandler = errors.New("unable to set Update handler") -) +func NewInvalidArgument(message string, cause error) error { + return temporal.NewNonRetryableApplicationError(message, InvalidArgumentErrType, cause, nil) +} + +func NewFailedPrecondition(message string, cause error) error { + return temporal.NewNonRetryableApplicationError(message, FailedPreconditionErrType, cause, nil) +} -func NewExecutionsStillExistError(count int) error { +func NewExecutionsStillExist(count int) error { return temporal.NewApplicationError(fmt.Sprintf("%d executions are still exist", count), ExecutionsStillExistErrType, count) } -func NewNoProgressError(count int) error { +func NewNoProgress(count int) error { return temporal.NewNonRetryableApplicationError(fmt.Sprintf("no progress was made: %d executions are still exist", count), NoProgressErrType, nil, count) } -func NewNotDeletedExecutionsStillExistError(count int) error { +func NewNotDeletedExecutionsStillExist(count int) error { return temporal.NewNonRetryableApplicationError(fmt.Sprintf("%d not deleted executions are still exist", count), NotDeletedExecutionsStillExistErrType, nil, count) } + +func ToServiceError(err error, workflowID, runID string) error { + var appErr *temporal.ApplicationError + if errors.As(err, &appErr) { + switch appErr.Type() { + case InvalidArgumentErrType: + return serviceerror.NewInvalidArgument(appErr.Message()) + case FailedPreconditionErrType: + return serviceerror.NewFailedPrecondition(appErr.Message()) + } + } + return serviceerror.NewSystemWorkflow( + &commonpb.WorkflowExecution{WorkflowId: workflowID, RunId: runID}, + err, + ) +} diff --git a/service/worker/deletenamespace/reclaimresources/activities.go b/service/worker/deletenamespace/reclaimresources/activities.go index 15beca8c55a..8c8ebde1a46 100644 --- a/service/worker/deletenamespace/reclaimresources/activities.go +++ b/service/worker/deletenamespace/reclaimresources/activities.go @@ -138,18 +138,18 @@ func (a *Activities) EnsureNoExecutionsAdvVisibilityActivity(ctx context.Context // No progress was made. Something bad happened on the task processor side or new executions were created during deletion. // Return non-retryable error and workflow will try to delete executions again. logger.Warn("No progress was made.", tag.Attempt(activityInfo.Attempt), tag.Counter(count)) - return errors.NewNoProgressError(count) + return errors.NewNoProgress(count) } } logger.Warn("Some workflow executions still exist.", tag.Counter(count)) activity.RecordHeartbeat(ctx, count) - return errors.NewExecutionsStillExistError(count) + return errors.NewExecutionsStillExist(count) } if notDeletedCount > 0 { logger.Warn("Some workflow executions were not deleted and still exist.", tag.Counter(notDeletedCount)) - return errors.NewNotDeletedExecutionsStillExistError(notDeletedCount) + return errors.NewNotDeletedExecutionsStillExist(notDeletedCount) } logger.Info("All workflow executions are deleted successfully.") diff --git a/service/worker/deletenamespace/reclaimresources/workflow.go b/service/worker/deletenamespace/reclaimresources/workflow.go index 1839979daed..01c97ae8158 100644 --- a/service/worker/deletenamespace/reclaimresources/workflow.go +++ b/service/worker/deletenamespace/reclaimresources/workflow.go @@ -93,15 +93,12 @@ var ( func validateParams(params *ReclaimResourcesParams) error { if params.NamespaceID.IsEmpty() { - return temporal.NewNonRetryableApplicationError("namespace ID is required", "", nil) + return errors.NewInvalidArgument("namespace ID is required", nil) } - if params.Namespace.IsEmpty() { - return temporal.NewNonRetryableApplicationError("namespace is required", "", nil) + return errors.NewInvalidArgument("namespace is required", nil) } - params.Config.ApplyDefaults() - return nil } @@ -161,23 +158,23 @@ func ReclaimResourcesWorkflow(ctx workflow.Context, params ReclaimResourcesParam }, workflow.UpdateHandlerOptions{ Validator: func(_ workflow.Context, newNamespaceDeleteDelayStr string) error { if newNamespaceDeleteDelayStr == "" { - return temporal.NewNonRetryableApplicationError("delay duration is required", errors.ValidationErrorErrType, nil) + return errors.NewInvalidArgument("delay duration is required", nil) } newDuration, err := time.ParseDuration(newNamespaceDeleteDelayStr) if err != nil { - return temporal.NewNonRetryableApplicationError("unable to parse delay duration", errors.ValidationErrorErrType, err) + return errors.NewInvalidArgument("unable to parse delay duration", err) } if newDuration < 0 { - return temporal.NewNonRetryableApplicationError("delay duration must be positive", errors.ValidationErrorErrType, nil) + return errors.NewInvalidArgument("delay duration must be positive", nil) } if newDuration > 30*24*time.Hour { - return temporal.NewNonRetryableApplicationError("delay duration must be less than 30 days", errors.ValidationErrorErrType, nil) + return errors.NewInvalidArgument("delay duration must be less than 30 days", nil) } return nil }, }) if err != nil { - return result, fmt.Errorf("%w: %v", errors.ErrUnableToSetUpdateHandler, err) + return result, err } var la *LocalActivities @@ -211,7 +208,7 @@ func ReclaimResourcesWorkflow(ctx workflow.Context, params ReclaimResourcesParam ctx5 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err = workflow.ExecuteLocalActivity(ctx5, la.DeleteNamespaceActivity, params.NamespaceID, params.Namespace).Get(ctx, nil) if err != nil { - return result, fmt.Errorf("%w: DeleteNamespaceActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } result.NamespaceDeleted = true @@ -236,7 +233,7 @@ func deleteWorkflowExecutions(ctx workflow.Context, logger log.Logger, params Re var isAdvancedVisibility bool err := workflow.ExecuteLocalActivity(ctx1, la.IsAdvancedVisibilityActivity, params.Namespace).Get(ctx, &isAdvancedVisibility) if err != nil { - return result, fmt.Errorf("%w: IsAdvancedVisibilityActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } } @@ -244,7 +241,7 @@ func deleteWorkflowExecutions(ctx workflow.Context, logger log.Logger, params Re var executionsCount int64 err := workflow.ExecuteLocalActivity(ctx4, la.CountExecutionsAdvVisibilityActivity, params.NamespaceID, params.Namespace).Get(ctx, &executionsCount) if err != nil { - return result, fmt.Errorf("%w: CountExecutionsAdvVisibilityActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } if executionsCount == 0 { return result, nil @@ -255,8 +252,8 @@ func deleteWorkflowExecutions(ctx workflow.Context, logger log.Logger, params Re var der deleteexecutions.DeleteExecutionsResult err = workflow.ExecuteChildWorkflow(ctx2, deleteexecutions.DeleteExecutionsWorkflow, params.DeleteExecutionsParams).Get(ctx, &der) if err != nil { - logger.Error("Unable to execute child workflow.", tag.Error(err)) - return result, fmt.Errorf("%w: %s: %v", errors.ErrUnableToExecuteChildWorkflow, deleteexecutions.WorkflowName, err) + logger.Error("Child workflow error.", tag.Error(err)) + return result, err } result.DeleteSuccessCount = der.SuccessCount result.DeleteErrorCount = der.ErrorCount @@ -279,7 +276,7 @@ func deleteWorkflowExecutions(ctx workflow.Context, logger log.Logger, params Re return result, temporal.NewApplicationError(appErr.Message(), appErr.Type(), notDeletedCount) } } - return result, fmt.Errorf("%w: EnsureNoExecutionsActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } return result, nil diff --git a/service/worker/deletenamespace/reclaimresources/workflow_test.go b/service/worker/deletenamespace/reclaimresources/workflow_test.go index 98820b4f8f3..63d7c598df6 100644 --- a/service/worker/deletenamespace/reclaimresources/workflow_test.go +++ b/service/worker/deletenamespace/reclaimresources/workflow_test.go @@ -138,8 +138,9 @@ func Test_ReclaimResourcesWorkflow_EnsureNoExecutionsActivity_Error(t *testing.T require.True(t, env.IsWorkflowCompleted()) err := env.GetWorkflowError() require.Error(t, err) - require.Contains(t, err.Error(), "unable to execute activity: EnsureNoExecutionsActivity") - require.Contains(t, err.Error(), "specific_error_from_activity") + require.Equal(t, + err.Error(), + "workflow execution error (type: ReclaimResourcesWorkflow, workflowID: default-test-workflow-id, runID: default-test-run-id): activity error (type: EnsureNoExecutionsAdvVisibilityActivity, scheduledEventID: 0, startedEventID: 0, identity: ): specific_error_from_activity") } func Test_ReclaimResourcesWorkflow_EnsureNoExecutionsActivity_ExecutionsStillExist(t *testing.T) { @@ -169,7 +170,7 @@ func Test_ReclaimResourcesWorkflow_EnsureNoExecutionsActivity_ExecutionsStillExi env.OnActivity(la.CountExecutionsAdvVisibilityActivity, mock.Anything, namespace.ID("namespace-id"), namespace.Name("namespace")).Return(int64(10), nil).Once() env.OnActivity(a.EnsureNoExecutionsAdvVisibilityActivity, mock.Anything, namespace.ID("namespace-id"), namespace.Name("namespace"), 0). - Return(errors.NewExecutionsStillExistError(1)). + Return(errors.NewExecutionsStillExist(1)). Times(10) // GoSDK defaultMaximumAttemptsForUnitTest value. env.ExecuteWorkflow(ReclaimResourcesWorkflow, ReclaimResourcesParams{ diff --git a/service/worker/deletenamespace/workflow.go b/service/worker/deletenamespace/workflow.go index 67971ea8db4..001c6078231 100644 --- a/service/worker/deletenamespace/workflow.go +++ b/service/worker/deletenamespace/workflow.go @@ -25,7 +25,6 @@ package deletenamespace import ( - stderrors "errors" "fmt" "strings" "time" @@ -96,22 +95,19 @@ var ( func validateParams(params *DeleteNamespaceWorkflowParams) error { if params.Namespace.IsEmpty() && params.NamespaceID.IsEmpty() { - return temporal.NewNonRetryableApplicationError("namespace or namespace ID is required", "", nil) + return errors.NewInvalidArgument("namespace or namespace ID is required", nil) } - if !params.Namespace.IsEmpty() && !params.NamespaceID.IsEmpty() { - return temporal.NewNonRetryableApplicationError("only one of namespace or namespace ID must be set", "", nil) + return errors.NewInvalidArgument("only one of namespace or namespace ID must be set", nil) } - params.DeleteExecutionsConfig.ApplyDefaults() - return nil } func validateNamespace(ctx workflow.Context, nsID namespace.ID, nsName namespace.Name, nsClusters []string) error { if nsName == primitives.SystemLocalNamespace || nsID == primitives.SystemNamespaceID { - return temporal.NewNonRetryableApplicationError("unable to delete system namespace", errors.ValidationErrorErrType, nil, nil) + return errors.NewFailedPrecondition("unable to delete system namespace", nil) } // Disable namespace deletion if namespace is replicate because: @@ -121,7 +117,7 @@ func validateNamespace(ctx workflow.Context, nsID namespace.ID, nsName namespace // in this case it will be deleted from this cluster only because delete operation is not replicated), // but this is confusing for the users, as they might expect that namespace is deleted from all clusters. if len(nsClusters) > 1 { - return temporal.NewNonRetryableApplicationError(fmt.Sprintf("namespace %s is replicated in several clusters [%s]: remove all other cluster and retry", nsName, strings.Join(nsClusters, ",")), errors.ValidationErrorErrType, nil) + return errors.NewFailedPrecondition(fmt.Sprintf("namespace %s is replicated in several clusters [%s]: remove all other cluster and retry", nsName, strings.Join(nsClusters, ",")), nil) } // NOTE: there is very little chance that another cluster is added after the check above, @@ -132,21 +128,13 @@ func validateNamespace(ctx workflow.Context, nsID namespace.ID, nsName namespace ctx1 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err := workflow.ExecuteLocalActivity(ctx1, la.ValidateProtectedNamespacesActivity, nsName).Get(ctx, nil) if err != nil { - var appErr *temporal.ApplicationError - if stderrors.As(err, &appErr) { - return appErr - } - return fmt.Errorf("%w: ValidateProtectedNamespacesActivity: %v", errors.ErrUnableToExecuteActivity, err) + return err } ctx2 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err = workflow.ExecuteLocalActivity(ctx2, la.ValidateNexusEndpointsActivity, nsID, nsName).Get(ctx, nil) if err != nil { - var appErr *temporal.ApplicationError - if stderrors.As(err, &appErr) { - return appErr - } - return fmt.Errorf("%w: ValidateNexusEndpointsActivity: %v", errors.ErrUnableToExecuteActivity, err) + return err } return nil @@ -176,11 +164,7 @@ func DeleteNamespaceWorkflow(ctx workflow.Context, params DeleteNamespaceWorkflo var namespaceInfo getNamespaceInfoResult err := workflow.ExecuteLocalActivity(ctx1, la.GetNamespaceInfoActivity, params.NamespaceID, params.Namespace).Get(ctx, &namespaceInfo) if err != nil { - ns := params.Namespace.String() - if ns == "" { - ns = params.NamespaceID.String() - } - return result, temporal.NewNonRetryableApplicationError(fmt.Sprintf("namespace %s is not found", ns), errors.ValidationErrorErrType, err) + return result, err } params.Namespace = namespaceInfo.Namespace params.NamespaceID = namespaceInfo.NamespaceID @@ -194,7 +178,7 @@ func DeleteNamespaceWorkflow(ctx workflow.Context, params DeleteNamespaceWorkflo ctx2 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err = workflow.ExecuteLocalActivity(ctx2, la.MarkNamespaceDeletedActivity, params.Namespace).Get(ctx, nil) if err != nil { - return result, fmt.Errorf("%w: MarkNamespaceDeletedActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } result.DeletedNamespaceID = params.NamespaceID @@ -203,13 +187,13 @@ func DeleteNamespaceWorkflow(ctx workflow.Context, params DeleteNamespaceWorkflo ctx3 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err = workflow.ExecuteLocalActivity(ctx3, la.GenerateDeletedNamespaceNameActivity, params.NamespaceID, params.Namespace).Get(ctx, &result.DeletedNamespace) if err != nil { - return result, fmt.Errorf("%w: GenerateDeletedNamespaceNameActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } ctx31 := workflow.WithLocalActivityOptions(ctx, localActivityOptions) err = workflow.ExecuteLocalActivity(ctx31, la.RenameNamespaceActivity, params.Namespace, result.DeletedNamespace).Get(ctx, nil) if err != nil { - return result, fmt.Errorf("%w: RenameNamespaceActivity: %v", errors.ErrUnableToExecuteActivity, err) + return result, err } // Step 4. Reclaim workflow resources asynchronously. @@ -225,9 +209,9 @@ func DeleteNamespaceWorkflow(ctx workflow.Context, params DeleteNamespaceWorkflo NamespaceDeleteDelay: params.NamespaceDeleteDelay, }) var reclaimResourcesExecution workflow.Execution - if err := reclaimResourcesFuture.GetChildWorkflowExecution().Get(ctx, &reclaimResourcesExecution); err != nil { - logger.Error("Unable to execute child workflow.", tag.Error(err)) - return result, fmt.Errorf("%w: %s: %v", errors.ErrUnableToExecuteChildWorkflow, reclaimresources.WorkflowName, err) + if err = reclaimResourcesFuture.GetChildWorkflowExecution().Get(ctx, &reclaimResourcesExecution); err != nil { + logger.Error("Child workflow error.", tag.Error(err)) + return result, err } logger.Info("Child workflow executed successfully.", tag.NewStringTag("wf-child-type", reclaimresources.WorkflowName)) diff --git a/service/worker/deletenamespace/workflow_test.go b/service/worker/deletenamespace/workflow_test.go index 72b3d812400..09298f2099e 100644 --- a/service/worker/deletenamespace/workflow_test.go +++ b/service/worker/deletenamespace/workflow_test.go @@ -231,7 +231,7 @@ func Test_DeleteNamespaceUsedByNexus(t *testing.T) { }, nil).Once() env.OnActivity(la.ValidateProtectedNamespacesActivity, mock.Anything, mock.Anything).Return(nil).Once() env.OnActivity(la.ValidateNexusEndpointsActivity, mock.Anything, mock.Anything, mock.Anything). - Return(temporal.NewNonRetryableApplicationError("cannot delete a namespace that is a target of a Nexus endpoint", errors.ValidationErrorErrType, nil, nil)). + Return(errors.NewFailedPrecondition("cannot delete a namespace that is a target of a Nexus endpoint", nil)). Once() env.ExecuteWorkflow(DeleteNamespaceWorkflow, DeleteNamespaceWorkflowParams{ diff --git a/tests/namespace_delete_test.go b/tests/namespace_delete_test.go index 07d15dd616f..5ef07556779 100644 --- a/tests/namespace_delete_test.go +++ b/tests/namespace_delete_test.go @@ -570,7 +570,7 @@ func (s *namespaceTestSuite) Test_NamespaceDelete_Protected() { s.Error(err) s.Nil(delResp) - var invalidArgErr *serviceerror.InvalidArgument - s.ErrorAs(err, &invalidArgErr) - s.Equal(fmt.Sprintf("namespace %s is protected from deletion", tv.NamespaceName().String()), invalidArgErr.Message) + var failedPreconditionErr *serviceerror.FailedPrecondition + s.ErrorAs(err, &failedPreconditionErr) + s.Equal(fmt.Sprintf("namespace %s is protected from deletion", tv.NamespaceName().String()), failedPreconditionErr.Message) }