Skip to content

Commit 6dc57f4

Browse files
authored
fix cryptic error message for FailedPrecondition failures (temporalio#7609)
## What changed? <!-- Describe what has changed in this PR --> - `FailedPrecondition` Errors from the deployment workflows are returned as `FailedPrecondition` from the deployment client. ## Why? <!-- Tell your future self why have you made these changes --> - Right now, if a user were to call SetCurrent/SetRamping and our internal deployment workflows were to throw a FailedPrecondition error, the deployment client would convert them to `Internal` errors which in-turn would be masked by gRPC interceptors. - The error message would be super vague and something like (inside actual): ![image](https://github.com/user-attachments/assets/1c808340-5388-4190-86c3-e6fc26bc930e) ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> - Altered existing tests to check for this. ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
1 parent 249d750 commit 6dc57f4

File tree

4 files changed

+18
-5
lines changed

4 files changed

+18
-5
lines changed

service/worker/workerdeployment/client.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,8 @@ func (d *ClientImpl) SetCurrentVersion(
543543
return &res, nil
544544
} else if failure := outcome.GetFailure(); failure.GetApplicationFailureInfo().GetType() == errVersionNotFound {
545545
return nil, serviceerror.NewNotFound(errVersionNotFound)
546+
} else if failure.GetApplicationFailureInfo().GetType() == errFailedPrecondition {
547+
return nil, serviceerror.NewFailedPrecondition(failure.Message)
546548
} else if failure != nil {
547549
// TODO: is there an easy way to recover the original type here?
548550
return nil, serviceerror.NewInternal(failure.Message)
@@ -635,6 +637,8 @@ func (d *ClientImpl) SetRampingVersion(
635637
return nil, serviceerror.NewNotFound(errVersionNotFound)
636638
} else if failure.GetApplicationFailureInfo().GetType() == errVersionAlreadyCurrentType {
637639
return nil, serviceerror.NewFailedPrecondition(fmt.Sprintf("Ramping version %v is already current", version))
640+
} else if failure.GetApplicationFailureInfo().GetType() == errFailedPrecondition {
641+
return nil, serviceerror.NewFailedPrecondition(failure.Message)
638642
} else if failure != nil {
639643
return nil, serviceerror.NewInternal(failure.Message)
640644
}

service/worker/workerdeployment/util.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ const (
9393
errVersionHasPollers = "Version cannot be deleted since it has active pollers."
9494
errVersionIsCurrentOrRamping = "Version cannot be deleted since it is current or ramping."
9595
errConflictTokenMismatchType = "errConflictTokenMismatch"
96+
errFailedPrecondition = "FailedPrecondition"
97+
98+
ErrRampingVersionDoesNotHaveAllTaskQueues = "proposed ramping version is missing active task queues from the current version; these would become unversioned if it is set as the ramping version"
99+
ErrCurrentVersionDoesNotHaveAllTaskQueues = "proposed current version is missing active task queues from the current version; these would become unversioned if it is set as the current version"
96100

97101
syncBatchSize = 25
98102
)

service/worker/workerdeployment/workflow.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ func (d *WorkflowRunner) handleSetRampingVersion(ctx workflow.Context, args *dep
405405
return nil, err
406406
}
407407
if isMissingTaskQueues {
408-
return nil, serviceerror.NewFailedPrecondition("New ramping version does not have all the task queues from the previous current version and some missing task queues are active and would become unversioned after this operation")
408+
return nil, serviceerror.NewFailedPrecondition(ErrRampingVersionDoesNotHaveAllTaskQueues)
409409
}
410410
}
411411
rampingSinceTime = routingUpdateTime
@@ -590,7 +590,7 @@ func (d *WorkflowRunner) handleSetCurrent(ctx workflow.Context, args *deployment
590590
return nil, err
591591
}
592592
if isMissingTaskQueues {
593-
return nil, serviceerror.NewFailedPrecondition("New current version does not have all the task queues from the previous current version and some missing task queues are active and would become unversioned after this operation")
593+
return nil, serviceerror.NewFailedPrecondition(ErrCurrentVersionDoesNotHaveAllTaskQueues)
594594
}
595595
}
596596

tests/worker_deployment_version_test.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,9 @@ func (s *DeploymentVersionSuite) TestDeleteVersion_ConcurrentDeleteVersion() {
972972

973973
// VersionMissingTaskQueues
974974
func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_InvalidSetCurrentVersion() {
975+
// Override the dynamic config to verify we don't get any unexpected masked errors.
976+
s.OverrideDynamicConfig(dynamicconfig.FrontendMaskInternalErrorDetails, true)
977+
975978
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
976979
defer cancel()
977980
tv := testvars.New(s)
@@ -1013,8 +1016,7 @@ func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_InvalidSetCurrentV
10131016

10141017
// SetCurrent should fail since task_queue_1 does not have a current version than the deployment's existing current version
10151018
// and it either has a backlog of tasks being present or an add rate > 0.
1016-
s.Error(err)
1017-
1019+
s.EqualErrorf(err, workerdeployment.ErrCurrentVersionDoesNotHaveAllTaskQueues, err.Error())
10181020
}
10191021

10201022
func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_ValidSetCurrentVersion() {
@@ -1054,6 +1056,9 @@ func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_ValidSetCurrentVer
10541056
}
10551057

10561058
func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_InvalidSetRampingVersion() {
1059+
// Override the dynamic config to verify we don't get any unexpected masked errors.
1060+
s.OverrideDynamicConfig(dynamicconfig.FrontendMaskInternalErrorDetails, true)
1061+
10571062
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
10581063
defer cancel()
10591064
tv := testvars.New(s)
@@ -1095,7 +1100,7 @@ func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_InvalidSetRampingV
10951100

10961101
// SetRampingVersion should fail since task_queue_1 does not have a current version than the deployment's existing current version
10971102
// and it either has a backlog of tasks being present or an add rate > 0.
1098-
s.Error(err)
1103+
s.EqualErrorf(err, workerdeployment.ErrRampingVersionDoesNotHaveAllTaskQueues, err.Error())
10991104
}
11001105

11011106
func (s *DeploymentVersionSuite) TestVersionMissingTaskQueues_ValidSetRampingVersion() {

0 commit comments

Comments
 (0)