context Refresh and Apply sometimes return nil

The documentation for Refresh indicates that it will always return a
valid state, but that wasn't true in the case of a graph builder error.
While this same concept wasn't documented for Apply, it was still
assumed in the terraform apply code.

Since the helper testing framework relies on the absence of a state to
determine if it can call Destroy, the Context can't can't start
returning a state in all cases. Document this, and use the State method
to fetch the correct state value after Apply.

Add a nil check to the WriteState function, so that writing a nil state
is a noop.

Make sure to init before sorting the state, to make sure we're not
attempting to sort nil values. This isn't technically needed with the
current code, but it's just safer in general.

backend/local/backend_apply.go

@@ -102,7 +102,9 @@ func (b *Local) opApply(
 	doneCh := make(chan struct{})
 	go func() {
 		defer close(doneCh)
-		applyState, applyErr = tfCtx.Apply()
+		_, applyErr = tfCtx.Apply()
+		// we always want the state, even if apply failed
+		applyState = tfCtx.State()
 
 		/*
 			// Record any shadow errors for later

terraform/context.go

@@ -453,8 +453,17 @@ func (c *Context) Input(mode InputMode) error {
 // Apply applies the changes represented by this context and returns
 // the resulting state.
 //
-// In addition to returning the resulting state, this context is updated
-// with the latest state.
+// Even in the case an error is returned, the state may be returned and will
+// potentially be partially updated.  In addition to returning the resulting
+// state, this context is updated with the latest state.
+//
+// If the state is required after an error, the caller should call
+// Context.State, rather than rely on the return value.
+//
+// TODO: Apply and Refresh should either always return a state, or rely on the
+//       State() method. Currently the helper/resource testing framework relies
+//       on the absence of a returned state to determine if Destroy can be
+//       called, so that will need to be refactored before this can be changed.
 func (c *Context) Apply() (*State, error) {
 	defer c.acquireRun("apply")()
 
@@ -580,7 +589,7 @@ func (c *Context) Plan() (*Plan, error) {
 // to their latest state. This will update the state that this context
 // works with, along with returning it.
 //
-// Even in the case an error is returned, the state will be returned and
+// Even in the case an error is returned, the state may be returned and
 // will potentially be partially updated.
 func (c *Context) Refresh() (*State, error) {
 	defer c.acquireRun("refresh")()

terraform/state.go

@@ -1960,12 +1960,12 @@ func ReadStateV2(jsonBytes []byte) (*State, error) {
 		}
 	}
 
-	// Sort it
-	state.sort()
-
 	// catch any unitialized fields in the state
 	state.init()
 
+	// Sort it
+	state.sort()
+
 	return state, nil
 }
 
@@ -1995,12 +1995,12 @@ func ReadStateV3(jsonBytes []byte) (*State, error) {
 		}
 	}
 
-	// Sort it
-	state.sort()
-
 	// catch any unitialized fields in the state
 	state.init()
 
+	// Sort it
+	state.sort()
+
 	// Now we write the state back out to detect any changes in normaliztion.
 	// If our state is now written out differently, bump the serial number to
 	// prevent conflicts.
@@ -2020,12 +2020,17 @@ func ReadStateV3(jsonBytes []byte) (*State, error) {
 
 // WriteState writes a state somewhere in a binary format.
 func WriteState(d *State, dst io.Writer) error {
-	// Make sure it is sorted
-	d.sort()
+	// writing a nil state is a noop.
+	if d == nil {
+		return nil
+	}
 
 	// make sure we have no uninitialized fields
 	d.init()
 
+	// Make sure it is sorted
+	d.sort()
+
 	// Ensure the version is set
 	d.Version = StateVersion