Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve multiple error handling #1669

Open
wata727 opened this issue Feb 5, 2023 · 0 comments
Open

Improve multiple error handling #1669

wata727 opened this issue Feb 5, 2023 · 0 comments
Labels
enhancement needs-design Detailed design is required for implementation

Comments

@wata727
Copy link
Member

wata727 commented Feb 5, 2023

Introduction

Currently, TFLint will stop immediately and return an error if an error occurs on the way:

$ tflint
Failed to check ruleset; Failed to check `azurerm_lb_probe_invalid_protocol` rule: main.tf:6,53-72: Invalid template interpolation value; The expression result is null. Cannot include a null value in a string template.

However, if multiple problems exist, you need to run TFLint multiple times until all errors are resolved. This is inefficient.

Fortunately, HCL parse errors (hcl.Diagnostics) already support multiple error handling:

if errors.As(err, &diags) {
fmt.Fprintf(f.Stderr, "%s:\n\n", err)
writer := hcl.NewDiagnosticTextWriter(f.Stderr, parseSources(sources), 0, !f.NoColor)
_ = writer.WriteDiagnostics(diags)

This proposal mainly focuses on multiple error handling otherwise (especially errors returned by plugins).

See also #1665

Proposal

Change the plugin interface to allow multiple errors to be returned. google.golang.org/grpc/status.WithDetails allows us to attach multiple pieces of information to the status code, so we might be able to take advantage of this. FYI, the SDK already uses this mechanism to propagate application errors:
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.15.0/plugin/toproto/toproto.go#L226-L254
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.15.0/plugin/fromproto/fromproto.go#L283-L321

When the Check returns an error, the SDK follows a convention to split it into multiple errors and turn them into status details. As the convention, errors.Join, introduced in Go 1.20, is a good option.
https://github.com/terraform-linters/tflint-plugin-sdk/blob/v0.15.0/plugin/host2plugin/server.go#L119

Consideration should be given to how hcl.Diagnostics should be treated. hcl.Diagnostics can already handle multiple errors, but not all contexts over the wire protocol (e.g. EvalContext, Extra). Maybe we should invent our own error representation like tfdiags.

References

@wata727 wata727 added enhancement needs-design Detailed design is required for implementation labels Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs-design Detailed design is required for implementation
Development

No branches or pull requests

1 participant