Deleting the deprecated proto codepaths

Author: chrsmith

common/nexus/dispatch_outcome.go

@@ -11,7 +11,8 @@ import (
 
 // DispatchOutcome names the arm of matching's DispatchNexusTaskResponse that came back from a
 // DispatchNexusTask call. The nested oneofs and the deprecated variants collapse into one flat set of
-// cases.
+// cases. (Including those from a worker sending the deprecated failure responses, those will get
+// silently converted into the newer forms.)
 //
 // The zero value is the empty string and is not a valid outcome, so a switch over a DispatchOutcome
 // needs a default clause. For metric tags use DispatchResult.OutcomeTag, not the string value.
@@ -32,26 +33,30 @@ const (
 	// DispatchOutcomeCancelAccepted means the worker accepted the cancellation request.
 	DispatchOutcomeCancelAccepted DispatchOutcome = "cancel-accepted"
 
-	// DispatchOutcomeOperationFailure means the worker ran the operation and it failed or was
-	// canceled. The task was handled; this is the handler's answer, not a delivery problem.
+	// DispatchOutcomeOperationFailure means the worker successfully ran the operation to completion. But
+	// operation resulted in a failure or was canceled. This is not a delivery problem.
+	//
+	// e.g. the Nexus handler fails with:
+	//     nexus.NewOperationFailedError("insufficient funds")
+	//     nexus.NewOperationCanceledError("already canceled upstream")
 	DispatchOutcomeOperationFailure DispatchOutcome = "operation-failure"
 
-	// DispatchOutcomeOperationFailureDeprecated is DispatchOutcomeOperationFailure as reported by a
-	// worker predating Temporal failure responses.
-	DispatchOutcomeOperationFailureDeprecated DispatchOutcome = "operation-failure-deprecated"
-
 	// DispatchOutcomeHandlerFailure means the worker refused the task with a Nexus handler error,
 	// whose retry behavior says whether another attempt is worthwhile.
+	//
+	// e.g. the Temporal worker fails before the handler executes, with:
+	//     nexus.NewHandlerErrorf(nexus.HandlerErrorTypeBadRequest, "cannot deserialize input")
+	//     nexus.NewHandlerErrorf(nexus.HandlerErrorTypeBadRequest, "callback URL required for async UpdateWorkflow operation invocations")
 	DispatchOutcomeHandlerFailure DispatchOutcome = "nexus-handler-failure"
 
 	// DispatchOutcomeWorkerFailure means the worker failed the task with a failure that is not a
 	// Nexus handler error, e.g. an application error sent via RespondNexusTaskFailed.
+	//
+	// e.g. the Temporal worker fails before the handler executes, but with a non-HandlerError like:
+	//     TimeoutFailureInfo
+	//     CanceledFailureInfo
 	DispatchOutcomeWorkerFailure DispatchOutcome = "worker-failure"
 
-	// DispatchOutcomeHandlerFailureDeprecated is DispatchOutcomeHandlerFailure as reported by a worker
-	// predating Temporal failure responses.
-	DispatchOutcomeHandlerFailureDeprecated DispatchOutcome = "nexus-handler-failure-deprecated"
-
 	// DispatchOutcomeRequestTimeout means matching gave up before the task was answered: no worker
 	// was polling the task queue, or a worker took the task and never responded.
 	DispatchOutcomeRequestTimeout DispatchOutcome = "request-timeout"
@@ -94,44 +99,17 @@ type DispatchResult struct {
 	// in place mutate the response too.
 	Failure *failurepb.Failure
 
-	// HandlerError is set only for DispatchOutcomeHandlerFailureDeprecated.
-	HandlerError *nexuspb.HandlerError
-
-	// OperationError is set only for DispatchOutcomeOperationFailureDeprecated.
-	OperationError *nexuspb.UnsuccessfulOperationError
-}
-
-// handlerErrorType returns the Nexus handler error type the worker reported, or "" when the outcome is
-// not a handler error.
-func (r DispatchResult) handlerErrorType() string {
-	switch r.Outcome {
-	case DispatchOutcomeHandlerFailure:
-		return r.Failure.GetNexusHandlerFailureInfo().GetType()
-	case DispatchOutcomeHandlerFailureDeprecated:
-		//nolint:staticcheck // Deprecated field on a deprecated variant.
-		return r.HandlerError.GetErrorType()
-	default:
-		return ""
-	}
+	// usedDeprecatedFormat records that the worker answered in a format predating Temporal failure
+	// responses. Outcome and Failure are normalized to failurepb.Failure. This is only needed
+	// for reporting metrics. (Which are different based on the kind of error format received.)
+	usedDeprecatedFormat bool
 }
 
-// ClassifyStartOperationDispatch classifies matching's response to a dispatched StartOperation task.
-func ClassifyStartOperationDispatch(resp *matchingservice.DispatchNexusTaskResponse) DispatchResult {
-	return classifyDispatchNexusTaskResponse(resp, classifyStartOperationResponse)
-}
-
-// ClassifyCancelOperationDispatch classifies matching's response to a dispatched CancelOperation task.
-func ClassifyCancelOperationDispatch(resp *matchingservice.DispatchNexusTaskResponse) DispatchResult {
-	return classifyDispatchNexusTaskResponse(
-		resp,
-		func(*nexuspb.StartOperationResponse) DispatchResult {
-			// A cancel response carries no fields, so any response means the worker accepted.
-			return DispatchResult{Outcome: DispatchOutcomeCancelAccepted}
-		})
-}
-
-// classifyDispatchNexusTaskResponse converts a DispatchNexusTaskResponse into a DispatchResult object.
-func classifyDispatchNexusTaskResponse(
+// baseClassifyDispatchNexusTaskResponse converts a DispatchNexusTaskResponse into a DispatchResult object.
+//
+// The supplied lambda is invoked to classify a successful Response outcome. (Since the DispatchNexusTaskResponse
+// proto is used for different kinds of request types, only the caller will know what success means.)
+func baseClassifyDispatchNexusTaskResponse(
 	resp *matchingservice.DispatchNexusTaskResponse,
 	onResponseFn func(*nexuspb.StartOperationResponse) DispatchResult,
 ) DispatchResult {
@@ -147,9 +125,10 @@ func classifyDispatchNexusTaskResponse(
 
 	case *matchingservice.DispatchNexusTaskResponse_HandlerError: //nolint:staticcheck // Deprecated, still sent by older workers.
 		return DispatchResult{
-			Outcome: DispatchOutcomeHandlerFailureDeprecated,
+			Outcome: DispatchOutcomeHandlerFailure,
 			//nolint:staticcheck // Deprecated field on a deprecated variant.
-			HandlerError: t.HandlerError,
+			Failure:              deprecatedHandlerErrorToFailure(t.HandlerError),
+			usedDeprecatedFormat: true,
 		}
 
 	case *matchingservice.DispatchNexusTaskResponse_RequestTimeout:
@@ -165,7 +144,7 @@ func classifyDispatchNexusTaskResponse(
 	}
 }
 
-// classifyStartOperationResponse classifies the answer a worker gave to a StartOperation request.
+// classifyStartOperationResponse classifies the response a worker gave to a StartOperation request.
 func classifyStartOperationResponse(resp *nexuspb.StartOperationResponse) DispatchResult {
 	switch t := resp.GetVariant().(type) {
 	case *nexuspb.StartOperationResponse_SyncSuccess:
@@ -196,16 +175,91 @@ func classifyStartOperationResponse(resp *nexuspb.StartOperationResponse) Dispat
 
 	case *nexuspb.StartOperationResponse_OperationError: //nolint:staticcheck // Deprecated, still sent by older workers.
 		return DispatchResult{
-			Outcome: DispatchOutcomeOperationFailureDeprecated,
+			Outcome: DispatchOutcomeOperationFailure,
 			//nolint:staticcheck // Deprecated field on a deprecated variant.
-			OperationError: t.OperationError,
+			Failure:              deprecatedOperationErrorToFailure(t.OperationError),
+			usedDeprecatedFormat: true,
 		}
 
 	default:
 		return DispatchResult{Outcome: DispatchOutcomeUnrecognized}
 	}
 }
 
+// ClassifyStartOperationDispatch classifies matching's response to a dispatched StartOperation task.
+func ClassifyStartOperationDispatch(resp *matchingservice.DispatchNexusTaskResponse) DispatchResult {
+	return baseClassifyDispatchNexusTaskResponse(resp, classifyStartOperationResponse)
+}
+
+// ClassifyCancelOperationDispatch classifies matching's response to a dispatched CancelOperation task.
+func ClassifyCancelOperationDispatch(resp *matchingservice.DispatchNexusTaskResponse) DispatchResult {
+	return baseClassifyDispatchNexusTaskResponse(
+		resp,
+		func(*nexuspb.StartOperationResponse) DispatchResult {
+			// A cancel response carries no fields, so any response means the worker accepted.
+			return DispatchResult{Outcome: DispatchOutcomeCancelAccepted}
+		})
+}
+
+// deprecatedHandlerErrorToFailure converts the deprecated nexuspb.HandlerError into the new format,
+// a Temporal failurepb.Failure.
+func deprecatedHandlerErrorToFailure(handlerErr *nexuspb.HandlerError) *failurepb.Failure {
+	failure := &failurepb.Failure{
+		FailureInfo: &failurepb.Failure_NexusHandlerFailureInfo{
+			NexusHandlerFailureInfo: &failurepb.NexusHandlerFailureInfo{
+				Type:          handlerErr.GetErrorType(),
+				RetryBehavior: handlerErr.GetRetryBehavior(),
+			},
+		},
+	}
+	if handlerErr.GetFailure() != nil {
+		failure.Cause = convertNexusFailureToTemporalFailure(handlerErr.GetFailure())
+	}
+	return failure
+}
+
+// deprecatedOperationErrorToFailure converts the deprecated nexuspb.UnsccessfulOperationError into the
+// new format, a Temporal failurepb.Failure proto.
+func deprecatedOperationErrorToFailure(opErr *nexuspb.UnsuccessfulOperationError) *failurepb.Failure {
+	failure := convertNexusFailureToTemporalFailure(opErr.GetFailure())
+
+	// If the OperationError was because it was cancelled, encode that into the Failure proto.
+	opState := nexus.OperationState(opErr.GetOperationState())
+	isCanceledErr := opState == nexus.OperationStateCanceled
+	if !isCanceledErr && failure.GetCanceledFailureInfo() == nil {
+		return failure
+	}
+
+	return &failurepb.Failure{
+		Message: failure.GetMessage(),
+		Cause:   failure,
+		FailureInfo: &failurepb.Failure_CanceledFailureInfo{
+			CanceledFailureInfo: &failurepb.CanceledFailureInfo{},
+		},
+	}
+}
+
+// convertNexusFailureToTemporalFailure converts the deprecated nexuspb.Failure into the new format,
+// a Temporal failurepb.Failure.
+func convertNexusFailureToTemporalFailure(nexusFailure *nexuspb.Failure) *failurepb.Failure {
+	//nolint:staticcheck // Deprecated function still in use for backward compatibility.
+	converted, err := NexusFailureToTemporalFailure(ProtoFailureToNexusFailure(nexusFailure))
+	// A failure that cannot be re-encoded falls back to its message. We know the operation failed,
+	// we just don't recognize the format of the data in its cause/details.
+	if err != nil {
+		return &failurepb.Failure{
+			Message: nexusFailure.GetMessage(),
+			FailureInfo: &failurepb.Failure_ApplicationFailureInfo{
+				ApplicationFailureInfo: &failurepb.ApplicationFailureInfo{
+					Type:         "NexusFailure",
+					NonRetryable: false,
+				},
+			},
+		}
+	}
+	return converted
+}
+
 // OutcomeTag returns the metrics outcome tag for a dispatch.
 //
 // The handler-error suffix is bounded by boundHandlerErrorType(). A worker failure that is not a handler
@@ -225,14 +279,15 @@ func (r DispatchResult) metricOutcome() string {
 	case DispatchOutcomeCancelAccepted:
 		return "success"
 	case DispatchOutcomeOperationFailure:
+		if r.usedDeprecatedFormat {
+			return "operation_error"
+		}
 		return "failure"
-	case DispatchOutcomeOperationFailureDeprecated:
-		return "operation_error"
 	case DispatchOutcomeHandlerFailure,
-		DispatchOutcomeWorkerFailure,
-		DispatchOutcomeHandlerFailureDeprecated:
+		DispatchOutcomeWorkerFailure:
 		// A worker failure has no handler error type to report and will map to UNKNOWN.
-		return "handler_error:" + boundHandlerErrorType(r.handlerErrorType())
+		hErrType := r.Failure.GetNexusHandlerFailureInfo().GetType()
+		return "handler_error:" + boundHandlerErrorType(hErrType)
 	case DispatchOutcomeRequestTimeout:
 		return "handler_timeout"
 	default: