hashicorp / go-multierror

A Go (golang) package for representing a list of errors as a single error.
Mozilla Public License 2.0
2.3k stars 123 forks source link

Update error output to be more readable #17

Closed FrenchBen closed 6 years ago

FrenchBen commented 6 years ago

Current error output looks like:

Error applying plan:

3 error(s) occurred:

* azurerm_network_interface.machine1: 1 error(s) occurred:

* azurerm_network_interface.machine1: <some error string goes here>
* azurerm_network_interface.machine1: <some error string goes here>
* azurerm_network_interface.machine2: 1 error(s) occurred:

* azurerm_network_interface.machine: <some error string goes here>

This changes it to be:

Error applying plan:

3 error(s) occurred:
* azurerm_network_interface.machine1: 2 error(s) occurred:
    * azurerm_network_interface.machine1: <some error string goes here>
    * azurerm_network_interface.machine1: <some error string goes here>

* azurerm_network_interface.machine2: 1 error(s) occurred:
    * azurerm_network_interface.machine2: <some error string goes here>

Which imho makes things more readable.

mitchellh commented 6 years ago

This looks pretty promising. @hashicorp/terraform-core what do you think?

jbardin commented 6 years ago

Hi @mitchellh,

I think this looks good, though I don't think it's not going to be directly useful in Terraform moving forward.

We're moving the handling all warnings and errors to a single Diagnostics type, which does its own formatting (and besides combining the handling warnings and errors, it also allows other features like correlating any configuration related errors back to the original source location). When remaining uses of multierror are appended to Diagnostics they are unpacked into separate errors to be formatted later for the user.

apparentlymart commented 6 years ago

Looks like a nice improvement to me!

Note though that Terraform will be using go-multierror a lot less from 0.12 onwards because we're switching to a HCL2-inspired "diagnostics" model for most things. This allows us to include source code snippets in the output:

Error: Extra characters after interpolation expression

This change will be nice for other go-multierror callers, though!

apparentlymart commented 6 years ago

Oh heh... I guess I should reload before I post a comment I've had sitting in my browser for 30 minutes. :wink:

FrenchBen commented 6 years ago

What's the overall consensus? Should I close this PR and wait for the HCL-2 release?

apparentlymart commented 6 years ago

I think this is good to merge, regardless of how Terraform is changing. I think we were just a little unclear with each other about who was going to own merging this. 🙄

I'm not at a computer right now but I will merge this tomorrow if Mitchell or James doesn't beat me to it, but they are in earlier timezones than me so they might get there first!