From 2f0d955196e39bee764a63fba6be769c06c092f9 Mon Sep 17 00:00:00 2001 From: Alistair Scott Date: Wed, 7 Oct 2020 14:08:12 +0100 Subject: [PATCH] refactor(errors): update error messaging Co-authored-by: Ali Khajeh-Hosseini Update internal/providers/terraform/provider.go Co-authored-by: Ali Khajeh-Hosseini Apply suggestions from code review Co-authored-by: Ali Khajeh-Hosseini refactor(errors): code review changes --- cmd/infracost/default.go | 15 ++++++++++----- cmd/infracost/main.go | 15 +++++++++------ internal/providers/terraform/provider.go | 18 +++++++++++------- pkg/output/output.go | 2 +- pkg/prices/query.go | 16 +++++++++++++--- 5 files changed, 44 insertions(+), 22 deletions(-) diff --git a/cmd/infracost/default.go b/cmd/infracost/default.go index d6352094dcaa..aab2f447c3c7 100644 --- a/cmd/infracost/default.go +++ b/cmd/infracost/default.go @@ -21,17 +21,17 @@ func defaultCmd() *cli.Command { Flags: []cli.Flag{ &cli.StringFlag{ Name: "tfjson", - Usage: "Path to Terraform Plan JSON file", + Usage: "Path to Terraform plan JSON file", TakesFile: true, }, &cli.StringFlag{ Name: "tfplan", - Usage: "Path to Terraform Plan file. Requires 'tfdir' to be set", + Usage: "Path to Terraform plan file relative to 'tfdir'. Requires 'tfdir' to be set", TakesFile: true, }, &cli.StringFlag{ Name: "tfdir", - Usage: "Path to the Terraform project directory", + Usage: "Path to the Terraform code directory", TakesFile: true, Value: getcwd(), DefaultText: "current working directory", @@ -71,13 +71,18 @@ func defaultCmd() *cli.Command { if err := prices.PopulatePrices(resources); err != nil { spinner.Fail() + red := color.New(color.FgHiRed) + bold := color.New(color.Bold, color.FgHiWhite) if e := unwrapped(err); errors.Is(e, prices.InvalidAPIKeyError) { - red := color.New(color.FgHiRed) - bold := color.New(color.Bold, color.FgHiWhite) fmt.Fprintln(os.Stderr, red.Sprint(e.Error())) fmt.Fprintln(os.Stderr, red.Sprint("Please check your"), bold.Sprint("INFRACOST_API_KEY"), red.Sprint("environment variable. If you continue having issues please email hello@infracost.io")) os.Exit(1) } + if e, ok := err.(*prices.PricingAPIError); ok { + fmt.Fprintln(os.Stderr, red.Sprint(e.Error())) + fmt.Fprintln(os.Stderr, red.Sprint("We have been notified of this issue.")) + os.Exit(1) + } return err } diff --git a/cmd/infracost/main.go b/cmd/infracost/main.go index 7d8fbdaf00f0..ffb291edcd18 100644 --- a/cmd/infracost/main.go +++ b/cmd/infracost/main.go @@ -82,14 +82,14 @@ func main() { UsageText: `infracost [global options] command [command options] [arguments...] Example: - # Run infracost with a terraform directory - infracost --tfdir /path/to/code + # Run infracost with a Terraform directory and var file + infracost --tfdir /path/to/code --tfflags "-var-file=myvars.tf" - # Run infracost with a JSON terraform plan file - infracost --tfjson /path/to/plan/file + # Run infracost with a JSON Terraform plan file + infracost --tfjson /path/to/plan.json - # Run infracost with a terraform directory and a plan file in it - infracost --tfdir /path/to/code --tfplan plan_file`, + # Run infracost with a Terraform directory and a plan file in it + infracost --tfdir /path/to/code --tfplan plan.save`, EnableBashCompletion: true, Version: version.Version, Flags: append([]cli.Flag{ @@ -120,6 +120,9 @@ Example: defer func() { if err := recover(); err != nil { + if spinner != nil { + spinner.Fail() + } red := color.New(color.FgHiRed) bold := color.New(color.Bold, color.FgHiWhite) fmt.Fprintln(os.Stderr, red.Sprint("An unexpected error occurred\n")) diff --git a/internal/providers/terraform/provider.go b/internal/providers/terraform/provider.go index b99b29024ea6..afa5cba87d27 100644 --- a/internal/providers/terraform/provider.go +++ b/internal/providers/terraform/provider.go @@ -59,7 +59,7 @@ func (p *terraformProvider) LoadResources() ([]*schema.Resource, error) { resources, err := parsePlanJSON(plan) if err != nil { - return resources, errors.Wrap(err, "Error parsing plan JSON") + return resources, errors.Wrap(err, "Error parsing Terraform plan JSON") } return resources, nil @@ -72,13 +72,13 @@ func (p *terraformProvider) loadPlanJSON() ([]byte, error) { f, err := os.Open(p.jsonFile) if err != nil { - return []byte{}, errors.Wrapf(err, "Error reading plan file") + return []byte{}, errors.Wrapf(err, "Error reading Terraform plan file") } defer f.Close() out, err := ioutil.ReadAll(f) if err != nil { - return []byte{}, errors.Wrapf(err, "Error reading plan file") + return []byte{}, errors.Wrapf(err, "Error reading Terraform plan file") } return out, nil @@ -150,7 +150,7 @@ func (p *terraformProvider) terraformPreChecks() error { if p.jsonFile == "" { _, err := exec.LookPath(terraformBinary()) if err != nil { - return errors.Errorf("Could not find Terraform binary \"%s\" in path.\nYou can set a custom Terraform binary using the environment variable TERRAFORM_BINARY.", terraformBinary()) + return errors.Errorf("Terraform binary \"%s\" could not be found.\nSet a custom Terraform binary using the environment variable TERRAFORM_BINARY.", terraformBinary()) } if v, ok := checkTerraformVersion(); !ok { @@ -158,7 +158,7 @@ func (p *terraformProvider) terraformPreChecks() error { } if !p.inTerraformDir() { - return errors.Errorf("Directory \"%s\" does not have any .tf files.\nYou can pass a path to a Terraform directory using the --tfdir option.", p.dir) + return errors.Errorf("Directory \"%s\" does not have any .tf files.\nSet the Terraform directory path using the --tfdir option.", p.dir) } } return nil @@ -204,13 +204,17 @@ func terraformError(err error) { stderr := stripBlankLines(string(e.Stderr)) fmt.Fprintln(os.Stderr, indent(color.HiRedString(stderr), " ")) if strings.HasPrefix(stderr, "Error: No value for required variable") { - fmt.Fprintln(os.Stderr, color.HiRedString("You can pass any Terraform args using the --tfflags option.")) + fmt.Fprintln(os.Stderr, color.HiRedString("Pass Terraform args using the --tfflags option.")) fmt.Fprintln(os.Stderr, color.HiRedString("For example: infracost --tfdir=path/to/terraform --tfflags=\"-var-file=myvars.tf\"\n")) } if strings.HasPrefix(stderr, "Error: Failed to read variables file") { - fmt.Fprintln(os.Stderr, color.HiRedString("You should specify the -var-file flag as a path relative to your Terraform directory.")) + fmt.Fprintln(os.Stderr, color.HiRedString("Specify the -var-file flag as a path relative to your Terraform directory.")) fmt.Fprintln(os.Stderr, color.HiRedString("For example: infracost --tfdir=path/to/terraform --tfflags=\"-var-file=myvars.tf\"\n")) } + if strings.HasPrefix(stderr, "Terraform couldn't read the given file as a state or plan file.") { + fmt.Fprintln(os.Stderr, color.HiRedString("Specify the --tfplan flag as a path relative to your Terraform directory.")) + fmt.Fprintln(os.Stderr, color.HiRedString("For example: infracost --tfdir=path/to/terraform --tfplan=plan.save\n")) + } } } diff --git a/pkg/output/output.go b/pkg/output/output.go index 93293784ffd3..c543ea589571 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -17,7 +17,7 @@ func skippedResourcesMessage(resources []*schema.Resource, showDetails bool) str } else { message += ", re-run with --show-skipped to see the list.\n" } - message += "We're continually adding new resources, please create an issue if you'd like us to prioritize your list." + message += "We're continually adding new resources, please email hello@infracost.io if you'd like us to prioritize your list." if showDetails { for rType, count := range unsupportedTypeCount { message += fmt.Sprintf("\n%d x %s", count, rType) diff --git a/pkg/prices/query.go b/pkg/prices/query.go index 75d6993e8200..a20c0d3415e8 100644 --- a/pkg/prices/query.go +++ b/pkg/prices/query.go @@ -3,6 +3,7 @@ package prices import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" @@ -16,6 +17,15 @@ import ( var InvalidAPIKeyError = errors.New("Invalid API key") +type PricingAPIError struct { + err error + msg string +} + +func (e *PricingAPIError) Error() string { + return fmt.Sprintf("%s: %v", e.msg, e.err.Error()) +} + type pricingAPIErrorResponse struct { Error string `json:"error"` } @@ -117,18 +127,18 @@ func (q *GraphQLQueryRunner) getQueryResults(queries []GraphQLQuery) ([]gjson.Re body, err := ioutil.ReadAll(resp.Body) if err != nil { - return results, errors.Wrap(err, "Invalid response from pricing API") + return results, &PricingAPIError{err, "Invalid response from pricing API"} } if resp.StatusCode != 200 { var r pricingAPIErrorResponse err = json.Unmarshal(body, &r) if err != nil { - return results, errors.Wrap(err, "Invalid response from pricing API") + return results, &PricingAPIError{err, "Invalid response from pricing API"} } if r.Error == "Invalid API key" { return results, InvalidAPIKeyError } - return results, errors.Wrap(err, "Received error from pricing API") + return results, &PricingAPIError{err, "Received error from pricing API"} } results = append(results, gjson.ParseBytes(body).Array()...)