From fcb6d88c5d03d51d41fb606e543324152b40b523 Mon Sep 17 00:00:00 2001
From: Haitao Chen <haidaocht@gmail.com>
Date: Mon, 13 Apr 2020 23:58:11 -0700
Subject: [PATCH] fix reflect error check

---
 async_task.go      | 19 ++++++++++++++++++-
 async_task_test.go | 11 ++++++++++-
 error_test.go      | 28 ++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/async_task.go b/async_task.go
index 16fc9cc..3ef49d2 100644
--- a/async_task.go
+++ b/async_task.go
@@ -135,6 +135,23 @@ func Start(ctx context.Context, task AsyncFunc) *TaskStatus {
 	return record
 }
 
+// isErrorReallyError do extra error check
+//    - Nil Pointer to a Type (that implement error)
+//    - Zero Value of a Type (that implement error)
+func isErrorReallyError(err error) bool {
+	v := reflect.ValueOf(err)
+	if v.Type().Kind() == reflect.Ptr &&
+		v.IsNil() {
+		return false
+	}
+
+	if v.Type().Kind() == reflect.Struct &&
+		v.IsZero() {
+		return false
+	}
+	return true
+}
+
 func runAndTrackTask(record *TaskStatus, task func(ctx context.Context) (interface{}, error)) {
 	defer record.waitGroup.Done()
 	defer func() {
@@ -150,7 +167,7 @@ func runAndTrackTask(record *TaskStatus, task func(ctx context.Context) (interfa
 		// incase some team use pointer typed error (implement Error() string on a pointer type)
 		// which can break err check (but nil point assigned to error result to non-nil error)
 		// check out TestPointerErrorCase in error_test.go
-		reflect.ValueOf(err).IsNil() {
+		!isErrorReallyError(err) {
 		record.finish(StateCompleted, result, nil)
 		return
 	}
diff --git a/async_task_test.go b/async_task_test.go
index 79ab77a..6c14720 100644
--- a/async_task_test.go
+++ b/async_task_test.go
@@ -61,7 +61,7 @@ func TestEasyCase(t *testing.T) {
 	result = rawResult.(int)
 	assert.Equal(t, result, 9)
 
-	assert.True(t, elapsed.Microseconds() < 2, "Second wait should take more than 2 millisecond")
+	assert.True(t, elapsed.Microseconds() < 2, "Second wait should return immediately")
 }
 
 func TestCancelFunc(t *testing.T) {
@@ -150,6 +150,15 @@ func TestConsistentResultAfterTimeout(t *testing.T) {
 	rawResult, err = t1.Wait()
 	assert.Equal(t, asynctask.ErrTimeout, err, "should return reason of error")
 	assert.Nil(t, rawResult, "didn't expect resule on canceled task")
+
+	// invoke WaitWithTimeout again,
+	start := time.Now()
+	rawResult, err = t1.WaitWithTimeout(1 * time.Second)
+	elapsed := time.Since(start)
+	assert.Equal(t, asynctask.StateCanceled, t1.State(), "t1 should remain canceled even after routine finish")
+	assert.Equal(t, asynctask.ErrTimeout, err, "should return reason of error")
+	assert.Nil(t, rawResult, "didn't expect resule on canceled task")
+	assert.True(t, elapsed.Microseconds() < 2, "Second wait should return immediately")
 }
 
 func TestCompletedTask(t *testing.T) {
diff --git a/error_test.go b/error_test.go
index 8fce030..9a7f0b2 100644
--- a/error_test.go
+++ b/error_test.go
@@ -10,6 +10,12 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+type structError struct{}
+
+func (pe structError) Error() string {
+	return "Error from struct type"
+}
+
 type pointerError struct{}
 
 func (pe *pointerError) Error() string {
@@ -77,3 +83,25 @@ func TestPointerErrorCase(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, result, "Done")
 }
+
+func TestStructErrorCase(t *testing.T) {
+	t.Parallel()
+
+	// nil point of a type that implement error
+	var se structError
+	// pass this nil pointer to error interface
+	var err error = se
+	// now you get a non-nil error
+	assert.False(t, err == nil, "reason this test is needed")
+
+	ctx := newTestContext(t)
+	tsk := asynctask.Start(ctx, func(ctx context.Context) (interface{}, error) {
+		time.Sleep(100 * time.Millisecond)
+		var se structError
+		return "Done", se
+	})
+
+	result, err := tsk.Wait()
+	assert.NoError(t, err)
+	assert.Equal(t, result, "Done")
+}