Merge pull request #199 from hashicorp/f-helper-resource-explicit-Ret…

…ryError

helper/resource: Require explicit usage of NonRetryableError and RetryableError

helper/resource/wait.go

@@ -1,6 +1,7 @@
 package resource
 
 import (
+	"errors"
 	"sync"
 	"time"
 )
@@ -66,19 +67,31 @@ type RetryError struct {
 }
 
 // RetryableError is a helper to create a RetryError that's retryable from a
-// given error.
+// given error. To prevent logic errors, will return an error when passed a
+// nil error.
 func RetryableError(err error) *RetryError {
 	if err == nil {
-		return nil
+		return &RetryError{
+			Err: errors.New("empty retryable error received. " +
+				"This is a bug with the Terraform provider and should be " +
+				"reported as a GitHub issue in the provider repository."),
+			Retryable: false,
+		}
 	}
 	return &RetryError{Err: err, Retryable: true}
 }
 
 // NonRetryableError is a helper to create a RetryError that's _not_ retryable
-// from a given error.
+// from a given error. To prevent logic errors, will return an error when
+// passed a nil error.
 func NonRetryableError(err error) *RetryError {
 	if err == nil {
-		return nil
+		return &RetryError{
+			Err: errors.New("empty non-retryable error received. " +
+				"This is a bug with the Terraform provider and should be " +
+				"reported as a GitHub issue in the provider repository."),
+			Retryable: false,
+		}
 	}
 	return &RetryError{Err: err, Retryable: false}
 }

helper/resource/wait_test.go

@@ -93,3 +93,29 @@ func TestRetry_error(t *testing.T) {
 		t.Fatal("timeout")
 	}
 }
+
+func TestRetry_nilNonRetryableError(t *testing.T) {
+	t.Parallel()
+
+	f := func() *RetryError {
+		return NonRetryableError(nil)
+	}
+
+	err := Retry(1*time.Second, f)
+	if err == nil {
+		t.Fatal("should error")
+	}
+}
+
+func TestRetry_nilRetryableError(t *testing.T) {
+	t.Parallel()
+
+	f := func() *RetryError {
+		return RetryableError(nil)
+	}
+
+	err := Retry(1*time.Second, f)
+	if err == nil {
+		t.Fatal("should error")
+	}
+}