Add resource address to error message

[mrzor]

When having multiple shell_script resources sharing the same name (i.e. in a module instanciated several times), it's hard to know which one is yielding an error because Terraform displays the error at the end without context.

Current

Error: Error occurred during execution.
 Command: './redacted read' 
 Error: 'exit status 127' 
 StdOut: 
  
 StdErr: 
 /bin/sh: 1: ./redacted: not found

One can guess it is the last shell_script resources to be read in that case.

I imagine that with parallelism turned on, such guessing becomes even harder.

Suggested

Example:

Error: Error occurred during execution.
Resource: module.mymodule.shell_script.mything
 Command: './redacted read' 
 Error: 'exit status 127' 
 StdOut: 
  
 StdErr: 
 /bin/sh: 1: ./redacted: not found

[scottwinkler]

I think this is a great idea, and I spent some time looking into this but I really don't think its possible. Terraform providers operate in a sandbox and don't have very much information available to them except the minimum they need to be able to do their job. As such, you know the InstanceState of resources, which may include information such as you would find in the "instance" attribute of the state file, e.g.

{
          "schema_version": 0,
          "attributes": {
            "environment": null,
            "id": "bsehc7dgrkrh05uual20",
            "interpreter": null,
            "lifecycle_commands": [
              {
                "read": "echo \"{\\\"user\\\": \\\"$(whoami)\\\"}\"\n"
              }
            ],
            "output": {
              "user": "swinkler"
            },
            "sensitive_environment": null,
            "working_directory": "."
          }
        }

As you can see, the id in the instances is not the same as the resource address. Worse, it is only set after Create() has successfully completed, meaning that you could only print it in the error message for Read(), Update() and Delete(). I guess this could still be beneficial, but it could also be confusing and frustrating not to have the error message for Create(), which IMO is the most important.

There does not appear to be any way to the resource address, as far as I can tell. Resources only have *schema.ResourceData available to them, which does contain a reference to State(), but again this is InstanceState and not the complete state file. ResourceAddress could be gleaned from InstanceInfo, but no reference to this struct is present via *schema.ResourceData. Even if you were to do something janky like read the configuration code or state file through alternative means, you still wouldn't know exactly what the resource address is for the resource in question.

Might I suggest using a for_each on the shell resource/module instead? this might make it more clear where the error message is coming from. Resources in for_each are addressed as: module.shell_script.example["a"] where "a" is an element in the set used to instantiate the resources.

[mrzor]

I now understand the limitations the provider has to work under. I think an argument could be made to terraform devs to expand the context a bit for error messages - or they could provide the context when displaying the error - because Terraform displays the error, the provider merely formats it. That would be a proper fix.

Using for_each looks like a good workaround.

I thing the workaround has some downsides (I'll admit they're pretty weak):

• It's subversive! for_each is meant to iterate on several things, as the documentation clearly states.
• It makes the address longer and redundant - it would be module.mymodinstance.shell_script.example["mymodinstance"]
• References to my shell script resource now either have to use the longer address, or a local - which makes the config harder to grasp.
• My experience with the count = 0 kludge - which is similar in principle - is that one should be wary of unchecked dynamic addressing, as it can fail in perplexing ways in some contexts like terraform import (see "random_id.cache is empty tuple" terraform-google-modules/terraform-google-gcloud#66).

... Which is why I've opened a PR with a pretty limited "error_tag" hack that addresses the issue with hopefully no side-effects or wide reaching concerns.

[scottwinkler]

i now print env in the output, which should be enough to be able to uniquely identify resources. i will favor this over an error tag for now. let me know what you think, i can always add error tag later