Add error wrapper types to highlight bad plan/state data

This commit uses Go's error wrapping features to transparently add some optional
info to certain planfile/state read errors. Specifically, we wrap errors when we
think we've identified the file type but are somehow unable to use it.

Callers that aren't interested in what we think about our input can just ignore
the wrapping; callers that ARE interested can use `errors.As()`.

internal/cloud/cloudplan/saved_plan.go

@@ -47,6 +47,11 @@ func LoadSavedPlanBookmark(filepath string) (SavedPlanBookmark, error) {
 		return bookmark, err
 	}
 
+	// Note that these error cases are somewhat ambiguous, but they *likely*
+	// mean we're not looking at a saved plan bookmark at all. Since we're not
+	// certain about the format at this point, it doesn't quite make sense to
+	// emit a "known file type but bad" error struct the way we do over in the
+	// planfile and statefile packages.
 	if bookmark.RemotePlanFormat != 1 {
 		return bookmark, ErrInvalidRemotePlanFormat
 	} else if bookmark.Hostname == "" {

internal/plans/planfile/reader.go

@@ -21,6 +21,25 @@ const tfstateFilename = "tfstate"
 const tfstatePreviousFilename = "tfstate-prev"
 const dependencyLocksFilename = ".terraform.lock.hcl" // matches the conventional name in an input configuration
 
+// ErrUnusableLocalPlan is an error wrapper to indicate that we *think* the
+// input represents plan file data, but can't use it for some reason (as
+// explained in the error text). Callers can check against this type with
+// errors.As() if they need to distinguish between corrupt plan files and more
+// fundamental problems like an empty file.
+type ErrUnusableLocalPlan struct {
+	inner error
+}
+
+func errUnusable(err error) *ErrUnusableLocalPlan {
+	return &ErrUnusableLocalPlan{inner: err}
+}
+func (e *ErrUnusableLocalPlan) Error() string {
+	return e.inner.Error()
+}
+func (e *ErrUnusableLocalPlan) Unwrap() error {
+	return e.inner
+}
+
 // Reader is the main type used to read plan files. Create a Reader by calling
 // Open.
 //
@@ -42,7 +61,7 @@ func Open(filename string) (*Reader, error) {
 		// like our old plan format from versions prior to 0.12.
 		if b, sErr := ioutil.ReadFile(filename); sErr == nil {
 			if bytes.HasPrefix(b, []byte("tfplan")) {
-				return nil, fmt.Errorf("the given plan file was created by an earlier version of Terraform; plan files cannot be shared between different Terraform versions")
+				return nil, errUnusable(fmt.Errorf("the given plan file was created by an earlier version of Terraform; plan files cannot be shared between different Terraform versions"))
 			}
 		}
 		return nil, err
@@ -86,12 +105,12 @@ func (r *Reader) ReadPlan() (*plans.Plan, error) {
 	if planFile == nil {
 		// This should never happen because we checked for this file during
 		// Open, but we'll check anyway to be safe.
-		return nil, fmt.Errorf("the plan file is invalid")
+		return nil, errUnusable(fmt.Errorf("the plan file is invalid"))
 	}
 
 	pr, err := planFile.Open()
 	if err != nil {
-		return nil, fmt.Errorf("failed to retrieve plan from plan file: %s", err)
+		return nil, errUnusable(fmt.Errorf("failed to retrieve plan from plan file: %s", err))
 	}
 	defer pr.Close()
 
@@ -108,16 +127,16 @@ func (r *Reader) ReadPlan() (*plans.Plan, error) {
 	// access the prior state (this and the ReadStateFile method).
 	ret, err := readTfplan(pr)
 	if err != nil {
-		return nil, err
+		return nil, errUnusable(err)
 	}
 
 	prevRunStateFile, err := r.ReadPrevStateFile()
 	if err != nil {
-		return nil, fmt.Errorf("failed to read previous run state from plan file: %s", err)
+		return nil, errUnusable(fmt.Errorf("failed to read previous run state from plan file: %s", err))
 	}
 	priorStateFile, err := r.ReadStateFile()
 	if err != nil {
-		return nil, fmt.Errorf("failed to read prior state from plan file: %s", err)
+		return nil, errUnusable(fmt.Errorf("failed to read prior state from plan file: %s", err))
 	}
 
 	ret.PrevRunState = prevRunStateFile.State
@@ -136,12 +155,12 @@ func (r *Reader) ReadStateFile() (*statefile.File, error) {
 		if file.Name == tfstateFilename {
 			r, err := file.Open()
 			if err != nil {
-				return nil, fmt.Errorf("failed to extract state from plan file: %s", err)
+				return nil, errUnusable(fmt.Errorf("failed to extract state from plan file: %s", err))
 			}
 			return statefile.Read(r)
 		}
 	}
-	return nil, statefile.ErrNoState
+	return nil, errUnusable(statefile.ErrNoState)
 }
 
 // ReadPrevStateFile reads the previous state file embedded in the plan file, which
@@ -154,12 +173,12 @@ func (r *Reader) ReadPrevStateFile() (*statefile.File, error) {
 		if file.Name == tfstatePreviousFilename {
 			r, err := file.Open()
 			if err != nil {
-				return nil, fmt.Errorf("failed to extract previous state from plan file: %s", err)
+				return nil, errUnusable(fmt.Errorf("failed to extract previous state from plan file: %s", err))
 			}
 			return statefile.Read(r)
 		}
 	}
-	return nil, statefile.ErrNoState
+	return nil, errUnusable(statefile.ErrNoState)
 }
 
 // ReadConfigSnapshot reads the configuration snapshot embedded in the plan

internal/plans/planfile/wrapped.go

@@ -1,6 +1,7 @@
 package planfile
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/hashicorp/terraform/internal/cloud/cloudplan"
@@ -81,10 +82,13 @@ func OpenWrapped(filename string) (*WrappedPlanFile, error) {
 	if cloudErr == nil {
 		return &WrappedPlanFile{cloud: &cloud}, nil
 	}
-	// If neither worked, return both errors. In general we don't care to give
-	// any advice about how to fix an internal problem in a plan file, since
-	// both formats are opaque, but we do want to give the user the best chance
-	// at resolving whatever their problem was.
+	// If neither worked, prioritize definitive "confirmed the format but can't
+	// use it" errors, then fall back to dumping everything we know.
+	var ulp *ErrUnusableLocalPlan
+	if errors.As(localErr, &ulp) {
+		return nil, ulp
+	}
+
 	combinedErr := fmt.Errorf("couldn't load the provided path as either a local plan file (%s) or a saved cloud plan (%s)", localErr, cloudErr)
 	return nil, combinedErr
 }

internal/states/statefile/read.go

@@ -20,6 +20,25 @@ import (
 // ErrNoState is returned by ReadState when the state file is empty.
 var ErrNoState = errors.New("no state")
 
+// ErrUnusableState is an error wrapper to indicate that we *think* the input
+// represents state data, but can't use it for some reason (as explained in the
+// error text). Callers can check against this type with errors.As() if they
+// need to distinguish between corrupt state and more fundamental problems like
+// an empty file.
+type ErrUnusableState struct {
+	inner error
+}
+
+func errUnusable(err error) *ErrUnusableState {
+	return &ErrUnusableState{inner: err}
+}
+func (e *ErrUnusableState) Error() string {
+	return e.inner.Error()
+}
+func (e *ErrUnusableState) Unwrap() error {
+	return e.inner
+}
+
 // Read reads a state from the given reader.
 //
 // Legacy state format versions 1 through 3 are supported, but the result will
@@ -55,9 +74,9 @@ func Read(r io.Reader) (*File, error) {
 		return nil, ErrNoState
 	}
 
-	state, diags := readState(src)
-	if diags.HasErrors() {
-		return nil, diags.Err()
+	state, err := readState(src)
+	if err != nil {
+		return nil, err
 	}
 
 	if state == nil {
@@ -68,7 +87,7 @@ func Read(r io.Reader) (*File, error) {
 	return state, diags.Err()
 }
 
-func readState(src []byte) (*File, tfdiags.Diagnostics) {
+func readState(src []byte) (*File, error) {
 	var diags tfdiags.Diagnostics
 
 	if looksLikeVersion0(src) {
@@ -77,15 +96,20 @@ func readState(src []byte) (*File, tfdiags.Diagnostics) {
 			unsupportedFormat,
 			"The state is stored in a legacy binary format that is not supported since Terraform v0.7. To continue, first upgrade the state using Terraform 0.6.16 or earlier.",
 		))
-		return nil, diags
+		return nil, errUnusable(diags.Err())
 	}
 
 	version, versionDiags := sniffJSONStateVersion(src)
 	diags = diags.Append(versionDiags)
 	if versionDiags.HasErrors() {
-		return nil, diags
+		// This is the last point where there's a really good chance it's not a
+		// state file at all. Past here, we'll assume errors mean it's state but
+		// we can't use it.
+		return nil, diags.Err()
 	}
 
+	var result *File
+	var err error
 	switch version {
 	case 0:
 		diags = diags.Append(tfdiags.Sourceless(
@@ -93,15 +117,14 @@ func readState(src []byte) (*File, tfdiags.Diagnostics) {
 			unsupportedFormat,
 			"The state file uses JSON syntax but has a version number of zero. There was never a JSON-based state format zero, so this state file is invalid and cannot be processed.",
 		))
-		return nil, diags
 	case 1:
-		return readStateV1(src)
+		result, diags = readStateV1(src)
 	case 2:
-		return readStateV2(src)
+		result, diags = readStateV2(src)
 	case 3:
-		return readStateV3(src)
+		result, diags = readStateV3(src)
 	case 4:
-		return readStateV4(src)
+		result, diags = readStateV4(src)
 	default:
 		thisVersion := tfversion.SemVer.String()
 		creatingVersion := sniffJSONStateTerraformVersion(src)
@@ -119,8 +142,13 @@ func readState(src []byte) (*File, tfdiags.Diagnostics) {
 				fmt.Sprintf("The state file uses format version %d, which is not supported by Terraform %s. This state file may have been created by a newer version of Terraform.", version, thisVersion),
 			))
 		}
-		return nil, diags
 	}
+
+	if diags.HasErrors() {
+		err = errUnusable(diags.Err())
+	}
+
+	return result, err
 }
 
 func sniffJSONStateVersion(src []byte) (uint64, tfdiags.Diagnostics) {