Unwrap exec.ExitError on all our special errors

And respond to errors.Is for Context errors

Author: paultyng

tfexec/cmd_default.go

@@ -38,7 +38,7 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
 		err = ctx.Err()
 	}
 	if err != nil {
-		return tf.parseError(err, errBuf.String())
+		return tf.wrapExitError(ctx, err, errBuf.String())
 	}
 
 	return nil

tfexec/cmd_linux.go

@@ -47,7 +47,7 @@ func (tf *Terraform) runTerraformCmd(ctx context.Context, cmd *exec.Cmd) error {
 		err = ctx.Err()
 	}
 	if err != nil {
-		return tf.parseError(err, errBuf.String())
+		return tf.wrapExitError(ctx, err, errBuf.String())
 	}
 
 	return nil

tfexec/errors.go

@@ -12,6 +12,10 @@ func (e *ErrNoSuitableBinary) Error() string {
 	return fmt.Sprintf("no suitable terraform binary could be found: %s", e.err.Error())
 }
 
+func (e *ErrNoSuitableBinary) Unwrap() error {
+	return e.err
+}
+
 // ErrVersionMismatch is returned when the detected Terraform version is not compatible with the
 // command or flags being used in this invocation.
 type ErrVersionMismatch struct {
@@ -27,9 +31,9 @@ func (e *ErrVersionMismatch) Error() string {
 // ErrManualEnvVar is returned when an env var that should be set programatically via an option or method
 // is set via the manual environment passing functions.
 type ErrManualEnvVar struct {
-	name string
+	Name string
 }
 
 func (err *ErrManualEnvVar) Error() string {
-	return fmt.Sprintf("manual setting of env var %q detected", err.name)
+	return fmt.Sprintf("manual setting of env var %q detected", err.Name)
 }

tfexec/exit_errors.go

@@ -1,7 +1,7 @@
 package tfexec
 
 import (
-	"errors"
+	"context"
 	"fmt"
 	"os/exec"
 	"regexp"
@@ -29,12 +29,21 @@ var (
 	tfVersionMismatchConstraintRegexp = regexp.MustCompile(`required_version = "(.+)"|Required version: (.+)\b`)
 )
 
-func (tf *Terraform) parseError(err error, stderr string) error {
-	ee, ok := err.(*exec.ExitError)
+func (tf *Terraform) wrapExitError(ctx context.Context, err error, stderr string) error {
+	exitErr, ok := err.(*exec.ExitError)
 	if !ok {
+		// not an exit error, short circuit, nothing to wrap
 		return err
 	}
 
+	ctxErr := ctx.Err()
+
+	// nothing to parse, return early
+	errString := strings.TrimSpace(stderr)
+	if errString == "" {
+		return &unwrapper{exitErr, ctxErr}
+	}
+
 	switch {
 	case tfVersionMismatchErrRegexp.MatchString(stderr):
 		constraint := ""
@@ -58,6 +67,8 @@ func (tf *Terraform) parseError(err error, stderr string) error {
 		}
 
 		return &ErrTFVersionMismatch{
+			unwrapper: unwrapper{exitErr, ctxErr},
+
 			Constraint: constraint,
 			TFVersion:  ver,
 		}
@@ -71,33 +82,77 @@ func (tf *Terraform) parseError(err error, stderr string) error {
 			}
 		}
 
-		return &ErrMissingVar{name}
+		return &ErrMissingVar{
+			unwrapper: unwrapper{exitErr, ctxErr},
+
+			VariableName: name,
+		}
 	case usageRegexp.MatchString(stderr):
-		return &ErrCLIUsage{stderr: stderr}
+		return &ErrCLIUsage{
+			unwrapper: unwrapper{exitErr, ctxErr},
+
+			stderr: stderr,
+		}
 	case noInitErrRegexp.MatchString(stderr):
-		return &ErrNoInit{stderr: stderr}
+		return &ErrNoInit{
+			unwrapper: unwrapper{exitErr, ctxErr},
+
+			stderr: stderr,
+		}
 	case noConfigErrRegexp.MatchString(stderr):
-		return &ErrNoConfig{stderr: stderr}
+		return &ErrNoConfig{
+			unwrapper: unwrapper{exitErr, ctxErr},
+
+			stderr: stderr,
+		}
 	case workspaceDoesNotExistRegexp.MatchString(stderr):
 		submatches := workspaceDoesNotExistRegexp.FindStringSubmatch(stderr)
 		if len(submatches) == 2 {
-			return &ErrNoWorkspace{submatches[1]}
+			return &ErrNoWorkspace{
+				unwrapper: unwrapper{exitErr, ctxErr},
+
+				Name: submatches[1],
+			}
 		}
 	case workspaceAlreadyExistsRegexp.MatchString(stderr):
 		submatches := workspaceAlreadyExistsRegexp.FindStringSubmatch(stderr)
 		if len(submatches) == 2 {
-			return &ErrWorkspaceExists{submatches[1]}
+			return &ErrWorkspaceExists{
+				unwrapper: unwrapper{exitErr, ctxErr},
+
+				Name: submatches[1],
+			}
 		}
 	}
-	errString := strings.TrimSpace(stderr)
-	if errString == "" {
-		// if stderr is empty, return the ExitError directly, as it will have a better message
-		return ee
+
+	return fmt.Errorf("%w\n%s", &unwrapper{exitErr, ctxErr}, stderr)
+}
+
+type unwrapper struct {
+	err    error
+	ctxErr error
+}
+
+func (u *unwrapper) Unwrap() error {
+	return u.err
+}
+
+func (u *unwrapper) Is(target error) bool {
+	switch target {
+	case context.DeadlineExceeded, context.Canceled:
+		return u.ctxErr == context.DeadlineExceeded ||
+			u.ctxErr == context.Canceled
 	}
-	return errors.New(stderr)
+	return false
+}
+
+func (u *unwrapper) Error() string {
+	return u.err.Error()
 }
 
 type ErrMissingVar struct {
+	unwrapper
+
 	VariableName string
 }
 
@@ -106,6 +161,8 @@ func (err *ErrMissingVar) Error() string {
 }
 
 type ErrNoWorkspace struct {
+	unwrapper
+
 	Name string
 }
 
@@ -115,6 +172,8 @@ func (err *ErrNoWorkspace) Error() string {
 
 // ErrWorkspaceExists is returned when creating a workspace that already exists
 type ErrWorkspaceExists struct {
+	unwrapper
+
 	Name string
 }
 
@@ -123,6 +182,8 @@ func (err *ErrWorkspaceExists) Error() string {
 }
 
 type ErrNoInit struct {
+	unwrapper
+
 	stderr string
 }
 
@@ -131,6 +192,8 @@ func (e *ErrNoInit) Error() string {
 }
 
 type ErrNoConfig struct {
+	unwrapper
+
 	stderr string
 }
 
@@ -147,6 +210,8 @@ func (e *ErrNoConfig) Error() string {
 // Currently cases 1 and 2 are handled.
 // TODO KEM: Handle exit 127 case. How does this work on non-Unix platforms?
 type ErrCLIUsage struct {
+	unwrapper
+
 	stderr string
 }
 
@@ -157,6 +222,8 @@ func (e *ErrCLIUsage) Error() string {
 // ErrTFVersionMismatch is returned when the running Terraform version is not compatible with the
 // value specified for required_version in the terraform block.
 type ErrTFVersionMismatch struct {
+	unwrapper
+
 	TFVersion string
 
 	// Constraint is not returned in the error messaging on 0.12

tfexec/internal/e2etest/errors_test.go

@@ -7,13 +7,19 @@ import (
 	"context"
 	"errors"
 	"os"
+	"os/exec"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/go-version"
 
 	"github.com/hashicorp/terraform-exec/tfexec"
 )
 
+var (
+	protocol5MinVersion = version.Must(version.NewVersion("0.12.0"))
+)
+
 func TestUnparsedError(t *testing.T) {
 	// This simulates an unparsed error from the Cmd.Run method (in this case file not found). This
 	// is to ensure we don't miss raising unexpected errors in addition to parsed / well known ones.
@@ -74,6 +80,11 @@ func TestMissingVar(t *testing.T) {
 			t.Fatalf("expected missing %s, got %q", longVarName, e.VariableName)
 		}
 
+		var ee *exec.ExitError
+		if !errors.As(err, &ee) {
+			t.Fatalf("expected exec.ExitError, got %T, %s", err, err)
+		}
+
 		// Test for no error when all variables have a value
 		_, err = tf.Plan(context.Background(), tfexec.Var(shortVarName+"=foo"), tfexec.Var(longVarName+"=foo"))
 		if err != nil {
@@ -108,5 +119,95 @@ func TestTFVersionMismatch(t *testing.T) {
 		if e.TFVersion != tfv.String() {
 			t.Fatalf("expected %q, got %q", tfv.String(), e.TFVersion)
 		}
+
+		var ee *exec.ExitError
+		if !errors.As(err, &ee) {
+			t.Fatalf("expected exec.ExitError, got %T, %s", err, err)
+		}
+	})
+}
+
+func TestContext_alreadyPastDeadline(t *testing.T) {
+	runTest(t, "", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) {
+		ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Second))
+		defer cancel()
+
+		_, _, err := tf.Version(ctx, true)
+		if err == nil {
+			t.Fatal("expected error from version command")
+		}
+
+		if !errors.Is(err, context.DeadlineExceeded) {
+			t.Fatalf("expected context.DeadlineExceeded, got %T %s", err, err)
+		}
+	})
+}
+
+func TestContext_sleepNoCancellation(t *testing.T) {
+	// this test is just to ensure that time_sleep works properly without cancellation
+	runTest(t, "sleep", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) {
+		// only testing versions that can cancel mid apply
+		if !tfv.GreaterThanOrEqual(protocol5MinVersion) {
+			t.Skip("the ability to interrupt an apply was added in protocol 5.0 in Terraform 0.12, so test is not valid")
+		}
+
+		err := tf.Init(context.Background())
+		if err != nil {
+			t.Fatalf("err during init: %s", err)
+		}
+
+		ctx := context.Background()
+		start := time.Now()
+		err = tf.Apply(ctx, tfexec.Var(`create_duration=5s`))
+		if err != nil {
+			t.Fatalf("error during apply: %s", err)
+		}
+		elapsed := time.Now().Sub(start)
+		if elapsed < 5*time.Second {
+			t.Fatalf("expected runtime of at least 5s, got %s", elapsed)
+		}
+	})
+}
+
+func TestContext_sleepTimeoutExpired(t *testing.T) {
+	runTest(t, "sleep", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) {
+		// only testing versions that can cancel mid apply
+		if !tfv.GreaterThanOrEqual(protocol5MinVersion) {
+			t.Skip("the ability to interrupt an apply was added in protocol 5.0 in Terraform 0.12, so test is not valid")
+		}
+
+		err := tf.Init(context.Background())
+		if err != nil {
+			t.Fatalf("err during init: %s", err)
+		}
+
+		ctx := context.Background()
+		ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
+		defer cancel()
+
+		err = tf.Apply(ctx)
+		if err == nil {
+			t.Fatal("expected error, but didn't find one")
+		}
+
+		if !errors.Is(err, context.DeadlineExceeded) {
+			t.Fatalf("expected context.DeadlineExceeded, got %T %s", err, err)
+		}
+	})
+}
+
+func TestContext_alreadyCancelled(t *testing.T) {
+	runTest(t, "", func(t *testing.T, tfv *version.Version, tf *tfexec.Terraform) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+
+		_, _, err := tf.Version(ctx, true)
+		if err == nil {
+			t.Fatal("expected error from version command")
+		}
+
+		if !errors.Is(err, context.Canceled) {
+			t.Fatalf("expected context.Canceled, got %T %s", err, err)
+		}
 	})
 }

tfexec/internal/e2etest/testdata/sleep/main.tf

@@ -0,0 +1,14 @@
+variable "create_duration" {
+  type    = string
+  default = "60s"
+}
+
+variable "destroy_duration" {
+  type    = string
+  default = null
+}
+
+resource "time_sleep" "sleep" {
+  create_duration  = var.create_duration
+  destroy_duration = var.destroy_duration
+}

tfexec/terraform.go

@@ -23,6 +23,12 @@ type printfer interface {
 // but you can override paths used in some commands depending on the available
 // options.
 //
+// All functions that execute CLI commands take a context.Context. It should be noted that
+// exec.Cmd.Run will not return context.DeadlineExceeded or context.Canceled by default, we
+// have augmented our wrapped errors to respond true to errors.Is for context.DeadlineExceeded
+// and context.Canceled if those are present on the context when the error is parsed. See
+// https://github.com/golang/go/issues/21880 for more about the Go limitations.
+//
 // By default, the instance inherits the environment from the calling code (using os.Environ)
 // but it ignores certain environment variables that are managed within the code and prohibits
 // setting them through SetEnv:
@@ -67,8 +73,9 @@ func NewTerraform(workingDir string, execPath string) (*Terraform, error) {
 
 	if execPath == "" {
 		err := fmt.Errorf("NewTerraform: please supply the path to a Terraform executable using execPath, e.g. using the tfinstall package.")
-		return nil, &ErrNoSuitableBinary{err: err}
-
+		return nil, &ErrNoSuitableBinary{
+			err: err,
+		}
 	}
 	tf := Terraform{
 		execPath:   execPath,