Test-suite bugfix: local activity errors were not encoded correctly (#1247)

The added test hopefully reveals the issue - prior to this fix,
that `err := f.Get(...)` -> `errors.Is(err, ...)` check passed, because
the error value was not ever encoded into a `*GenericError`.

Thankfully this was only a test environment bug, and a user discovered
it due to writing tests.

---

The non-test behavior can be seen split between these two locations:
- encoding error reason / data: https://github.com/uber-go/cadence-client/blob/ad91fc718330cb40ff26627fa1d1a6c78c2ceaca/internal/internal_event_handlers.go#L1159-L1163
- constructing the generic error in handleLocalActivityMarker: https://github.com/uber-go/cadence-client/blob/ad91fc718330cb40ff26627fa1d1a6c78c2ceaca/internal/internal_event_handlers.go#L1132-L1135

... which also show incorrect / pointless string/byte conversions.
I've left comments there just to prevent confusion, but these fields are
probably worth changing at some point.

Author: Groxx

internal/internal_event_handlers.go

@@ -1159,10 +1159,10 @@ func (weh *workflowExecutionEventHandlerImpl) ProcessLocalActivityResult(lar *lo
 	if lar.err != nil {
 		errReason, errDetails := getErrorDetails(lar.err, weh.GetDataConverter())
 		lamd.ErrReason = errReason
-		lamd.ErrJSON = string(errDetails)
+		lamd.ErrJSON = string(errDetails) // TODO: this is not json, nor is it necessarily a valid string
 		lamd.Backoff = lar.backoff
 	} else {
-		lamd.ResultJSON = string(lar.result)
+		lamd.ResultJSON = string(lar.result) // TODO: same, not json nor a string
 	}
 
 	// encode marker data

internal/internal_task_pollers.go

@@ -140,7 +140,7 @@ type (
 
 	localActivityResult struct {
 		result  []byte
-		err     error
+		err     error // original error type, possibly an un-encoded user error
 		task    *localActivityTask
 		backoff time.Duration
 	}

internal/internal_worker_base.go

@@ -60,7 +60,7 @@ type (
 	laResultHandler func(lar *localActivityResultWrapper)
 
 	localActivityResultWrapper struct {
-		err     error
+		err     error // internal error type, possibly containing encoded user-error data
 		result  []byte
 		attempt int32
 		backoff time.Duration

internal/internal_workflow_testsuite.go

@@ -1301,7 +1301,16 @@ func (env *testWorkflowEnvironmentImpl) handleLocalActivityResult(result *localA
 	}
 
 	delete(env.localActivities, activityID)
-	lar := &localActivityResultWrapper{err: result.err, result: result.result, backoff: noRetryBackoff}
+	var encodedErr error
+	if result.err != nil {
+		errReason, errDetails := getErrorDetails(result.err, env.GetDataConverter())
+		encodedErr = constructError(errReason, errDetails, env.GetDataConverter())
+	}
+	lar := &localActivityResultWrapper{
+		err:     encodedErr,
+		result:  result.result,
+		backoff: noRetryBackoff,
+	}
 	if result.task.retryPolicy != nil && result.err != nil {
 		lar.backoff = getRetryBackoff(result, env.Now())
 		lar.attempt = task.attempt

internal/internal_workflow_testsuite_test.go

@@ -31,7 +31,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zaptest"
@@ -3169,3 +3171,35 @@ func (s *WorkflowTestSuiteUnitTest) Test_Regression_ExecuteChildWorkflowWithCanc
 		check(0, true, "no err")
 	})
 }
+
+func TestRegression_LocalActivityErrorEncoding(t *testing.T) {
+	// previously not encoded correctly
+	s := WorkflowTestSuite{}
+	s.SetLogger(zaptest.NewLogger(t))
+	env := s.NewTestWorkflowEnvironment()
+	sentinel := errors.New("sentinel error value")
+	env.RegisterWorkflowWithOptions(func(ctx Context) error {
+		ctx = WithLocalActivityOptions(ctx, LocalActivityOptions{ScheduleToCloseTimeout: time.Second})
+		err := ExecuteLocalActivity(ctx, func(ctx context.Context) error {
+			return sentinel
+		}).Get(ctx, nil)
+		if errors.Is(err, sentinel) {
+			// incorrect path, taken through v0.19.1
+			return fmt.Errorf("local activity errors need to be encoded, and must not be `.Is` a specific value: %w", err)
+		}
+		// correct path
+		return sentinel
+	}, RegisterWorkflowOptions{Name: "errorsis"})
+	env.ExecuteWorkflow("errorsis")
+	err := env.GetWorkflowError()
+
+	// make sure that the right path was chosen, and that the GetWorkflowError returns the same encoded value.
+	// GetWorkflowError could... *possibly* return a wrapped original error value, but this seems unnecessarily
+	// difficult to maintain long-term, doesn't reflect actual non-test behavior, and might lead to misunderstandings.
+	require.Error(t, err) // stop early to avoid confusing NPEs
+	var generr *GenericError
+	assert.ErrorAs(t, err, &generr, "should be an encoded generic error")
+	assert.NotErrorIs(t, err, sentinel, "should not contain a specific value, as this cannot be replayed")
+	assert.Contains(t, err.Error(), "sentinel error value", "should contain the user error text")
+	assert.NotContains(t, err.Error(), "need to be encoded", "should not contain the wrong-err-type branch message")
+}