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

hcl: Allow individual diagnostics to carry extra information #539

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented Jun 21, 2022

The primary goal of the diagnostics design in HCL is to return high-quality diagnostics messages primarily for human consumption, and so their regular structure is only machine-processable in a general sense where we treat all diagnostics as subject to the same processing.

A few times now we've ended up wanting to carry some additional optional contextual information along with the diagnostic, for example so that a more advanced diagnostics renderer might optionally annotate a diagnostic with extra notes to help the reader debug.

We got pretty far with our previous extension of hcl.Diagnostic to include the Expression and EvalContext fields, which allow an advanced diagnostic renderer to offer hints about what values contributed to the expression that failed, but some context is even more specific than that, or is defined by the application itself and therefore not appropriate to model directly here in HCL.

As a pragmatic compromise then, here we introduce one more field Extra to hcl.Diagnostic, which comes with a documented convention of placing into it situation-specific dynamically-typed values that implement particular interfaces, and therefore a diagnostics renderer or other consumer can potentially "sniff" this field for particular interfaces it knows about and treat them in a special way if present.

Since there is only one field here that might end up being asked to capture multiple extra values as the call stack unwinds, there is also a simple predefined protocol for "unwrapping" extra values in order to find nested implementations within.

For callers that are prepared to require Go 1.18, the generic helper function hcl.DiagnosticExtra provides a type-assertion-like mechanism for sniffing for a particular interface type while automatically respecting the nesting protocol. For the moment that function lives behind a build constraint so that callers which are not yet ready to use Go 1.18 can continue to use other parts of HCL, and can implement a non-generic equivalent of this function within their own codebase if absolutely necessary.


As an initial example to demonstrate the idea I've also implemented some extra information for error diagnostics returned from FunctionCallExpr, which gives the name of the function being called and, if the diagnostic is describing an error returned by the function itself, a direct reference to the raw error value returned from the function call.

I anticipate a diagnostic renderer sniffing for hclsyntax.FunctionCallDiagExtra to see if a particular diagnostic is related to a function call, and if so to include additional context about the signature of that function in the diagnostic messages (by correlating with the function in the EvalContext functions table). For example:

    While calling: join(separator, list)

This extra annotation could be helpful in a situation where the user got the arguments switched around, or forgot to write one of the arguments, or similar. I've not implemented this for the built-in diagnostics renderer in this module because it has no precedent for this sort of annotation, but applications like Terraform which have their own more elaborate diagnostic presentation could make use of this.


An example application-specific use-case would be for Terraform to annotate diagnostics that relate to situations where an unknown value is invalid, or where a "sensitive" value (a Terraform-specific value mark) is invalid, so that Terraform's own diagnostic renderer can avoid distracting users with "red herring" commentary about unknown or sensitive values unless they seem likely to be relevant to the error being printed.

Currently we often see users confused because an error message introduces the unfamiliar concept "foo is a string, known only after apply" and assume that it's relevant to the error, even though the error message actually has nothing to do with values being unknown. Only a few very specific error messages in Terraform actually hinge on whether a value is known or non-sensitive, and so ideally Terraform would be able to avoid mentioning these at all unless it's one of those specific error messages.

The primary goal of the diagnostics design in HCL is to return
high-quality diagnostics messages primarily for human consumption, and so
their regular structure is only machine-processable in a general sense
where we treat all diagnostics as subject to the same processing.

A few times now we've ended up wanting to carry some additional optional
contextual information along with the diagnostic, for example so that a
more advanced diagnostics renderer might optionally annotate a diagnostic
with extra notes to help the reader debug.

We got pretty far with our previous extension of hcl.Diagnostic to include
the Expression and EvalContext fields, which allow an advanced diagnostic
renderer to offer hints about what values contributed to the expression
that failed, but some context is even more specific than that, or is
defined by the application itself and therefore not appropriate to model
directly here in HCL.

As a pragmatic compromise then, here we introduce one more field Extra
to hcl.Diagnostic, which comes with a documented convention of placing
into it situation-specific values that implement particular interfaces,
and therefore a diagnostics renderer or other consumer can potentially
"sniff" this field for particular interfaces it knows about and treat them
in a special way if present.

Since there is only one field here that might end up being asked to
capture multiple extra values as the call stack unwinds, there is also a
simple predefined protocol for "unwrapping" extra values in order to find
nested implementations within.


For callers that are prepared to require Go 1.18, the helper function
hcl.DiagnosticExtra provides a type-assertion-like mechanism for sniffing
for a particular interface type while automatically respecting the nesting
protocol. For the moment that function lives behind a build constraint
so that callers which are not yet ready to use Go 1.18 can continue to
use other parts of HCL, and can implement a non-generic equivalent of
this function within their own codebase if absolutely necessary.

As an initial example to demonstrate the idea I've also implemented some
extra information for error diagnostics returned from FunctionCallExpr,
which gives the name of the function being called and, if the diagnostic
is describing an error returned by the function itself, a direct reference
to the raw error value returned from the function call. I anticipate a
diagnostic renderer sniffing for hclsyntax.FunctionCallDiagExtra to see
if a particular diagnostic is related to a function call, and if so to
include additional context about the signature of that function in the
diagnostic messages (by correlating with the function in the EvalContext
functions table). For example:
    While calling: join(separator, list)

An example application-specific "extra value" could be for Terraform to
annotate diagnostics that relate to situations where an unknown value is
invalid, or where a "sensitive" value (a Terraform-specific value mark) is
invalid, so that the diagnostic renderer can avoid distracting users with
"red herring" commentary about unknown or sensitive values unless they
seem likely to be relevant to the error being printed.
@apparentlymart apparentlymart added enhancement core v2 Relates to the v2 line of releases labels Jun 21, 2022
@apparentlymart apparentlymart requested a review from a team June 21, 2022 19:12
@apparentlymart apparentlymart self-assigned this Jun 21, 2022
@apparentlymart apparentlymart merged commit 88ecd13 into main Jun 22, 2022
@apparentlymart apparentlymart deleted the f-diags-extra branch June 22, 2022 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement v2 Relates to the v2 line of releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants