Repository: cycloidio/terracost (go)

Issue:
estimation: Return a list of errors instead of failing the entire process
## Abstract

Failing the entire cost estimation process because of a single failure in retrieving the correct product/price is not the best UX. The failure should be recorded and an error returned, however, all the successful estimates should be returned as well.

## Implementation details

For example, by adding a new field to the `cost.State` struct that holds the failed resources and the error of each, e.g. in a `map[string]error`. As well as a method on `cost.Plan` that can be used to retrieve all the failed resources.

## Suggestions/ideas

This is similar in concept to #12 and could probably be accomplished together.

Pull request description:
cost: store the errors instead of failing
## Abstract

Whenever a `cost.State` is created using `cost.NewState` function, the previous behaviour was to fail it the first time an error is encountered (e.g. when no product matching the query was found.)

This PR adds a new field on `Component` to store the error. It is used by `cost.NewState` instead of returning the error from the function. The errors can then be read from a `ResourceDiff` by using the `Errors` method.

Closes #17 

Patch:
diff --git a/cost/component.go b/cost/component.go
index bb33a14..3bb2760 100644
--- a/cost/component.go
+++ b/cost/component.go
@@ -11,6 +11,8 @@ type Component struct {
 	Unit     string
 	Rate     decimal.Decimal
 	Details  []string
+
+	Error error
 }
 
 // Cost returns the cost of this component (Rate multiplied by Quantity).
@@ -38,3 +40,8 @@ func (cd ComponentDiff) PlannedCost() decimal.Decimal {
 	}
 	return cd.Planned.Cost()
 }
+
+// Valid returns true if there are no errors in both the Planned and Prior components.
+func (cd ComponentDiff) Valid() bool {
+	return !((cd.Prior != nil && cd.Prior.Error != nil) || (cd.Planned != nil && cd.Planned.Error != nil))
+}
diff --git a/cost/resource.go b/cost/resource.go
index 9539e8f..000266f 100644
--- a/cost/resource.go
+++ b/cost/resource.go
@@ -43,3 +43,26 @@ func (rd ResourceDiff) PlannedCost() decimal.Decimal {
 	}
 	return total
 }
+
+// Errors returns a map of Component errors keyed by the Component label.
+func (rd ResourceDiff) Errors() map[string]error {
+	errs := make(map[string]error)
+	for label, cd := range rd.ComponentDiffs {
+		if cd.Prior != nil && cd.Prior.Error != nil {
+			errs[label] = cd.Prior.Error
+		} else if cd.Planned != nil && cd.Planned.Error != nil {
+			errs[label] = cd.Planned.Error
+		}
+	}
+	return errs
+}
+
+// Valid returns true if there are no errors in all of the ResourceDiff's components.
+func (rd ResourceDiff) Valid() bool {
+	for _, cd := range rd.ComponentDiffs {
+		if (cd.Prior != nil && cd.Prior.Error != nil) || (cd.Planned != nil && cd.Planned.Error != nil) {
+			return false
+		}
+	}
+	return true
+}
diff --git a/cost/state.go b/cost/state.go
index b4dfe6a..da34602 100644
--- a/cost/state.go
+++ b/cost/state.go
@@ -29,6 +29,12 @@ type State struct {
 // It is calculated as 365 days in a year x 24 hours in a day / 12 months in year.
 const HoursPerMonth = 730
 
+// Errors that might be returned from NewState if either a product or a price are not found.
+var (
+	ErrProductNotFound = fmt.Errorf("product not found")
+	ErrPriceNotFound   = fmt.Errorf("price not found")
+)
+
 // NewState returns a new State from a query.Resource slice by using the Backend to fetch the pricing data.
 func NewState(ctx context.Context, backend Backend, queries []query.Resource) (*State, error) {
 	state := &State{Resources: make(map[string]Resource)}
@@ -37,17 +43,21 @@ func NewState(ctx context.Context, backend Backend, queries []query.Resource) (*
 		for _, comp := range res.Components {
 			prods, err := backend.Product().Filter(ctx, comp.ProductFilter)
 			if err != nil {
-				return nil, err
+				state.addComponent(res.Address, comp.Name, Component{Error: err})
+				continue
 			}
 			if len(prods) < 1 {
-				return nil, fmt.Errorf("product not found")
+				state.addComponent(res.Address, comp.Name, Component{Error: ErrProductNotFound})
+				continue
 			}
 			prices, err := backend.Price().Filter(ctx, prods[0].ID, comp.PriceFilter)
 			if err != nil {
-				return nil, err
+				state.addComponent(res.Address, comp.Name, Component{Error: err})
+				continue
 			}
 			if len(prices) < 1 {
-				return nil, fmt.Errorf("price not found")
+				state.addComponent(res.Address, comp.Name, Component{Error: ErrPriceNotFound})
+				continue
 			}
 
 			quantity := comp.MonthlyQuantity


Test patch:
diff --git a/cost/component_test.go b/cost/component_test.go
index 0b83205..7254e41 100644
--- a/cost/component_test.go
+++ b/cost/component_test.go
@@ -1,6 +1,7 @@
 package cost_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/shopspring/decimal"
@@ -36,3 +37,26 @@ func TestComponentDiff_PlannedCost(t *testing.T) {
 		assert.True(t, actual.Equal(decimal.NewFromFloat(7.5)))
 	})
 }
+
+func TestComponentDiff_Valid(t *testing.T) {
+	err := fmt.Errorf("test error")
+	testcases := []struct {
+		prior, planned *cost.Component
+		valid          bool
+	}{
+		{&cost.Component{}, &cost.Component{}, true},
+		{&cost.Component{}, nil, true},
+		{nil, &cost.Component{}, true},
+		{&cost.Component{Error: err}, &cost.Component{}, false},
+		{&cost.Component{}, &cost.Component{Error: err}, false},
+		{&cost.Component{Error: err}, &cost.Component{Error: err}, false},
+		{&cost.Component{Error: err}, nil, false},
+		{nil, &cost.Component{Error: err}, false},
+		{nil, nil, true},
+	}
+
+	for i, tc := range testcases {
+		cd := cost.ComponentDiff{Prior: tc.prior, Planned: tc.planned}
+		assert.Equal(t, tc.valid, cd.Valid(), "case %d", i)
+	}
+}
diff --git a/cost/resource_test.go b/cost/resource_test.go
new file mode 100644
index 0000000..cd26d51
--- /dev/null
+++ b/cost/resource_test.go
@@ -0,0 +1,105 @@
+package cost_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/cycloidio/terracost/cost"
+)
+
+func TestResourceDiff_Errors(t *testing.T) {
+	rd := &cost.ResourceDiff{
+		Address: "complex_resource.example",
+		ComponentDiffs: map[string]*cost.ComponentDiff{
+			"BothNil": {
+				Prior:   nil,
+				Planned: nil,
+			},
+			"BothCorrect": {
+				Prior:   &cost.Component{},
+				Planned: &cost.Component{},
+			},
+			"BrokenPrior": {
+				Prior:   &cost.Component{Error: cost.ErrProductNotFound},
+				Planned: nil,
+			},
+			"BrokenPlanned": {
+				Prior:   nil,
+				Planned: &cost.Component{Error: cost.ErrPriceNotFound},
+			},
+			"BothBroken": {
+				Prior:   &cost.Component{Error: cost.ErrProductNotFound},
+				Planned: &cost.Component{Error: cost.ErrPriceNotFound},
+			},
+		},
+	}
+
+	expected := map[string]error{
+		"BrokenPrior":   cost.ErrProductNotFound,
+		"BrokenPlanned": cost.ErrPriceNotFound,
+		"BothBroken":    cost.ErrProductNotFound,
+	}
+	assert.Equal(t, expected, rd.Errors())
+}
+
+func TestResourceDiff_Valid(t *testing.T) {
+	testcases := []struct {
+		label string
+		cd    *cost.ComponentDiff
+		valid bool
+	}{
+		{
+			"BothNil",
+			&cost.ComponentDiff{
+				Prior:   nil,
+				Planned: nil,
+			},
+			true,
+		},
+		{
+			"BothCorrect",
+			&cost.ComponentDiff{
+				Prior:   &cost.Component{},
+				Planned: &cost.Component{},
+			},
+			true,
+		},
+		{
+			"BrokenPrior",
+			&cost.ComponentDiff{
+				Prior:   &cost.Component{Error: cost.ErrProductNotFound},
+				Planned: nil,
+			},
+			false,
+		},
+		{
+			"BrokenPlanned",
+			&cost.ComponentDiff{
+				Prior:   nil,
+				Planned: &cost.Component{Error: cost.ErrPriceNotFound},
+			},
+			false,
+		},
+		{
+			"BothBroken",
+			&cost.ComponentDiff{
+				Prior:   &cost.Component{Error: cost.ErrProductNotFound},
+				Planned: &cost.Component{Error: cost.ErrPriceNotFound},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.label, func(t *testing.T) {
+			rd := &cost.ResourceDiff{
+				Address: tc.label,
+				ComponentDiffs: map[string]*cost.ComponentDiff{
+					tc.label: tc.cd,
+				},
+			}
+			assert.Equal(t, tc.valid, rd.Valid())
+		})
+	}
+}
diff --git a/cost/state_test.go b/cost/state_test.go
index f6a111f..68245de 100644
--- a/cost/state_test.go
+++ b/cost/state_test.go
@@ -87,8 +87,9 @@ func TestNewState(t *testing.T) {
 
 		productRepo.EXPECT().Filter(ctx, queries[0].Components[0].ProductFilter).Return(nil, errors.New("repo fail"))
 
-		_, err := cost.NewState(ctx, backend, queries)
-		assert.Error(t, err)
+		state, err := cost.NewState(ctx, backend, queries)
+		require.NoError(t, err)
+		assert.Error(t, state.Resources["aws_instance.test1"].Components["Compute"].Error)
 	})
 
 	t.Run("PriceRepositoryFailure", func(t *testing.T) {
@@ -106,8 +107,9 @@ func TestNewState(t *testing.T) {
 		productRepo.EXPECT().Filter(ctx, queries[0].Components[0].ProductFilter).Return([]*product.Product{prod1}, nil)
 		priceRepo.EXPECT().Filter(ctx, prod1.ID, queries[0].Components[0].PriceFilter).Return(nil, errors.New("repo fail"))
 
-		_, err := cost.NewState(ctx, backend, queries)
-		assert.Error(t, err)
+		state, err := cost.NewState(ctx, backend, queries)
+		require.NoError(t, err)
+		assert.Error(t, state.Resources["aws_instance.test1"].Components["Compute"].Error)
 	})
 }
 
diff --git a/e2e/aws_estimation_test.go b/e2e/aws_estimation_test.go
index 0122987..cff237f 100644
--- a/e2e/aws_estimation_test.go
+++ b/e2e/aws_estimation_test.go
@@ -13,6 +13,7 @@ import (
 	costestimation "github.com/cycloidio/terracost"
 	"github.com/cycloidio/terracost/aws/region"
 	awstf "github.com/cycloidio/terracost/aws/terraform"
+	"github.com/cycloidio/terracost/cost"
 	"github.com/cycloidio/terracost/mysql"
 	"github.com/cycloidio/terracost/price"
 	"github.com/cycloidio/terracost/product"
@@ -159,8 +160,22 @@ func TestAWSEstimation(t *testing.T) {
 		require.NoError(t, err)
 		defer f.Close()
 
-		_, err = costestimation.EstimateTerraformPlan(ctx, backend, f, terraformProviderInitializer)
-		assert.Error(t, err)
+		plan, err := costestimation.EstimateTerraformPlan(ctx, backend, f, terraformProviderInitializer)
+		require.NoError(t, err)
+
+		diffs := plan.ResourceDifferences()
+		require.Len(t, diffs, 1)
+		rd := diffs[0]
+
+		rootVol := diffs[0].ComponentDiffs["Root volume: Storage"]
+		require.NotNil(t, rootVol)
+		assertDecimalEqual(t, decimal.NewFromFloat(3.6), rootVol.PriorCost())
+		assertDecimalEqual(t, decimal.NewFromFloat(3.6), rootVol.PlannedCost())
+
+		expected := map[string]error{
+			"Compute": cost.ErrProductNotFound,
+		}
+		assert.Equal(t, expected, rd.Errors())
 	})
 }