Simplify and Recommend Consistent Terraform State Writing Error Handling

[bflad]

SDK version

v2.4.0

Use-cases

Resources that need to write to the Terraform State use the (helper/schema.ResourceData).Set() receiver method. e.g.

// Terraform Plugin SDK v1
resource ExampleThingRead(d *schema.ResourceData, meta interface{}) error {
  // ...
  if err := d.Set("attribute_name", /* ... */); err != nil {
    return fmt.Errorf("error setting attribute_name: %w", err)
  }
  // ...
}

// Terraform Plugin SDK v2
resource ExampleThingRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostic {
  // ...
  if err := d.Set("attribute_name", /* ... */); err != nil {
    return diag.FromErr(fmt.Errorf("error setting attribute_name: %w", err))
  }
  // ...
}

Technically speaking, implementing the error checking for every attribute should be done to notify operators about the potential loss of drift detection and cover missing acceptance testing data scenarios. However for pragmatic purposes such as code readability and that acceptance testing coverage, many providers consider the error checking as optional for "simple" data types such as TypeString (string), instead suggesting it for "complex" types such as TypeList/TypeMap/TypeSet. The choice of whether or not to apply the error checking can be confusing for developers and the implementations can vary.

Especially since Set() returns an error type instead of one directly append-able to diag.Diagnostics, the ergonomics can be a little awkward and the implementations can very, even across resources within a single provider.

This issue to discuss if there is potential for simplifying this developer experience and then recommending a standardized implementation.

Attempted Solutions

Not performing the error checking or enforcing it via static analysis.

Proposal

Proposal 1

Implementing a diagnostic wrapper function with a consistent diagnostic:

func SetDiag(err error) diag.Diagnostic {
  if err == nil {
    return nil
  }

  return diag.Diagnostic{
    Severity: diag.Error,
    Summary: "Unable to update Terraform State value",
    Detail: fmt.Sprintf("For read-only attributes, the value will not be present. For writeable attributes, drift detection for this attribute will not work as expected. This is a bug that should be reported to the Terraform Provider. Error details: %s", err.Error()),
  }
}

resource ExampleThingRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
  var diags diag.Diagnostics
  // ...
  diags = append(diags, schema.SetDiag(d.Set("attribute_name", /* ... */)))
  // ...
  return diags
}

Proposal 2

Taking proposal 1 further and instead create a separate SetWithDiag() (up for bikeshedding the name) receiver method that can be directly used.

func (d *ResourceData) SetWithDiag(key string, value interface{}) diag.Diagnostic {
  err := d.Set(key, value)

  if err == nil {
    return nil
  }

  return diag.Diagnostic{
    Severity: diag.Error,
    Summary: "Unable to update Terraform State value",
    Detail: fmt.Sprintf("For read-only attributes, the value will not be present. For writeable attributes, drift detection for this attribute will not work as expected. This is a bug that should be reported to the Terraform Provider. Attribute %q received error: %s", key, err.Error()),
  }
}

resource ExampleThingRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
  var diags diag.Diagnostics
  // ...
  diags = append(diags, d.SetWithDiag("attribute_name", /* ... */))
  // ...
  return diags
}

References

• schema ResourceData Set does not work for nested types like TypeList #670
• Consistent Error Handling for Setting Terraform State terraform-provider-aws#17117
• https://learn.hashicorp.com/collections/terraform/providers

[bflad]

Closing due to lack of 👍 and since this feature is already handled in terraform-plugin-framework.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.