diff --git a/internal/error.go b/internal/error.go index 12dc9f654..087cf3f6e 100644 --- a/internal/error.go +++ b/internal/error.go @@ -190,6 +190,10 @@ const ( errReasonGeneric = "cadenceInternal:Generic" errReasonCanceled = "cadenceInternal:Canceled" errReasonTimeout = "cadenceInternal:Timeout" + + badNilErrMsgFmt = "cadence received an invalid nil `%T`." + + " this likely means you have a typed nil which is being" + + " converted into an `error` value: https://go.dev/doc/faq#nil_error" ) // ErrNoData is returned when trying to extract strong typed data while there is no data available. diff --git a/internal/internal_utils.go b/internal/internal_utils.go index b0e5f09f2..e31e23e36 100644 --- a/internal/internal_utils.go +++ b/internal/internal_utils.go @@ -63,9 +63,9 @@ const ( // defaultRPCTimeout is the default tchannel rpc call timeout defaultRPCTimeout = 10 * time.Second - //minRPCTimeout is minimum rpc call timeout allowed + // minRPCTimeout is minimum rpc call timeout allowed minRPCTimeout = 1 * time.Second - //maxRPCTimeout is maximum rpc call timeout allowed + // maxRPCTimeout is maximum rpc call timeout allowed maxRPCTimeout = 5 * time.Second // maxQueryRPCTimeout is the maximum rpc call timeout allowed for query maxQueryRPCTimeout = 20 * time.Second @@ -243,6 +243,9 @@ func getErrorDetails(err error, dataConverter DataConverter) (string, []byte) { case *CustomError: var data []byte var err0 error + if err == nil { + return errReasonGeneric, []byte(fmt.Sprintf(badNilErrMsgFmt, err)) + } switch details := err.details.(type) { case ErrorDetailsValues: data, err0 = encodeArgs(dataConverter, details) @@ -258,6 +261,10 @@ func getErrorDetails(err error, dataConverter DataConverter) (string, []byte) { case *CanceledError: var data []byte var err0 error + if err == nil { + // treat this as a failure, not a cancel, as it likely is not a real cancel + return errReasonGeneric, []byte(fmt.Sprintf(badNilErrMsgFmt, err)) + } switch details := err.details.(type) { case ErrorDetailsValues: data, err0 = encodeArgs(dataConverter, details) @@ -271,6 +278,10 @@ func getErrorDetails(err error, dataConverter DataConverter) (string, []byte) { } return errReasonCanceled, data case *PanicError: + if err == nil { + // treat this as a failure, not a panic, as it likely is not a real panic + return errReasonGeneric, []byte(fmt.Sprintf(badNilErrMsgFmt, err)) + } data, err0 := encodeArgs(dataConverter, []interface{}{err.Error(), err.StackTrace()}) if err0 != nil { panic(err0) @@ -279,6 +290,10 @@ func getErrorDetails(err error, dataConverter DataConverter) (string, []byte) { case *TimeoutError: var data []byte var err0 error + if err == nil { + // treat this as a failure, not a timeout, as it likely is not a real timeout + return errReasonGeneric, []byte(fmt.Sprintf(badNilErrMsgFmt, err)) + } switch details := err.details.(type) { case ErrorDetailsValues: data, err0 = encodeArgs(dataConverter, details) diff --git a/internal/internal_utils_test.go b/internal/internal_utils_test.go index d6a268619..c555541d8 100644 --- a/internal/internal_utils_test.go +++ b/internal/internal_utils_test.go @@ -25,6 +25,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" s "go.uber.org/cadence/.gen/go/shared" @@ -135,6 +136,23 @@ func TestGetErrorDetails_TimeoutError(t *testing.T) { require.Equal(t, val2, data) } +func Test_getErrorDetails_nil(t *testing.T) { + // typed nils caused panics, prevent them from coming back + check := func(t *testing.T, err error) { + reason, data := getErrorDetails(err, getDefaultDataConverter()) + dataStr := string(data) + assert.Equalf(t, errReasonGeneric, reason, "wrong reason for %T", err) + assert.Contains(t, dataStr, "typed nil", "wrong data for %T", err) + } + check(t, (*CanceledError)(nil)) + check(t, (*CustomError)(nil)) + check(t, (*PanicError)(nil)) + check(t, (*TimeoutError)(nil)) + // other types are probably also good to handle, but e.g. handling + // external error types will require panic recovery. + // this might be a good idea, but for now this just covers "we can do better with our types". +} + func TestConstructError_TimeoutError(t *testing.T) { t.Parallel() dc := getDefaultDataConverter()