fix(cmd): fail the run when recommendation fetching fails

cmd/cancellation_prepurchase_test.go

@@ -88,8 +88,9 @@ func TestFetchRecommendationsForRegion_InterruptIsNotReportedAsProviderFailure(t
 	client := &MockRecommendationsClient{}
 	client.On("GetRecommendations", mock.Anything, mock.Anything).Return(nil, ctx.Err())
 
-	got := fetchRecommendationsForRegion(ctx, client, common.ServiceRDS, "us-east-1", Config{TermYears: 1})
+	got, err := fetchRecommendationsForRegion(ctx, client, common.ServiceRDS, "us-east-1", Config{TermYears: 1})
 
+	assert.ErrorIs(t, err, context.Canceled)
 	assert.Nil(t, got)
 	assert.NotContains(t, out.String(), "Failed to fetch recommendations")
 }
@@ -111,20 +112,31 @@ func TestFetchAllRecs_InterruptStopsFanOutAndDiscardsPartialData(t *testing.T) {
 	client.AssertNumberOfCalls(t, "GetRecommendations", 1) // later regions are never queried
 }
 
-func TestFetchAllRecs_ActiveContextProviderErrorStaysRecoverable(t *testing.T) {
-	out := captureAppLog(t)
+func TestFetchAllRecs_ProviderErrorFailsTheRun(t *testing.T) {
+	captureAppLog(t)
 	cfg := Config{Regions: []string{"us-east-1", "us-west-2"}, Coverage: 100, TermYears: 1, PaymentOption: "partial-upfront"}
 	client := &MockRecommendationsClient{}
 	client.On("GetRecommendations", mock.Anything, mock.MatchedBy(func(p *common.RecommendationParams) bool { return p.Region == "us-east-1" })).
-		Return(nil, errors.New("throttling")).Once()
-	rec := common.Recommendation{Service: common.ServiceRDS, Region: "us-west-2", ResourceType: "db.t3.small", Count: 3, EstimatedSavings: 10, SavingsPercentage: 10}
-	client.On("GetRecommendations", mock.Anything, mock.MatchedBy(func(p *common.RecommendationParams) bool { return p.Region == "us-west-2" })).
-		Return([]common.Recommendation{rec}, nil).Once()
+		Return(nil, errors.New("expired SSO token")).Once()
+
+	recs, _, err := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+
+	require.Error(t, err, "a fetch failure must not read as 'no recommendations'")
+	assert.Contains(t, err.Error(), "expired SSO token")
+	assert.Empty(t, recs)
+	client.AssertNumberOfCalls(t, "GetRecommendations", 1)
+}
+
+func TestFetchAllRecs_EmptyResultIsNotAnError(t *testing.T) {
+	captureAppLog(t)
+	cfg := Config{Regions: []string{"us-east-1"}, Coverage: 100, TermYears: 1, PaymentOption: "partial-upfront"}
+	client := &MockRecommendationsClient{}
+	client.On("GetRecommendations", mock.Anything, mock.Anything).Return([]common.Recommendation{}, nil)
 
-	recs, _, _ := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+	recs, _, err := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
 
-	assert.Len(t, recs, 1, "a provider error with an active context only loses that region")
-	assert.Contains(t, out.String(), "Failed to fetch recommendations")
+	require.NoError(t, err)
+	assert.Empty(t, recs)
 }
 
 func TestCheckDuplicates_InterruptedCheckIsTerminalInDryAndRealRuns(t *testing.T) {

cmd/multi_service.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"sync/atomic"
@@ -188,7 +189,7 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 	// Phase 1: collect all recommendations without purchasing.
 	AppLogger.Printf("\n📥 Fetching recommendations from all services...\n")
 	allRecs, drops, err := fetchAllRecs(ctx, awsCfg, recClient, accountCache, servicesToProcess, engineData, cfg, coverageMap)
-	exitOnExclusionError(err)
+	exitOnFetchError(err)
 	exitIfInterrupted(ctx, "fetching recommendations")
 
 	// Phase 2: score, enforce the run-wide instance cap, and display.
@@ -1051,6 +1052,13 @@ func hasDatabaseRecs(recs []common.Recommendation) bool {
 
 // exitOnExclusionError aborts the run before any purchase when the
 // extended-support exclusion cannot be applied (issue #2147).
+func exitOnFetchError(err error) {
+	if errors.Is(err, errRecommendationFetch) {
+		log.Fatalf("Cannot complete the recommendation fetch: %v", err)
+	}
+	exitOnExclusionError(err)
+}
+
 func exitOnExclusionError(err error) {
 	if err != nil {
 		log.Fatalf("Cannot apply extended-support exclusion: %v", err)

cmd/multi_service_helpers.go

@@ -508,7 +508,13 @@ func processRegionRecommendations(
 	AppLogger.Printf("\n  📍 [%d/%d] Region: %s\n", regionIndex, totalRegions, region)
 
 	// Fetch recommendations
-	recs := fetchRecommendationsForRegion(ctx, recClient, service, region, cfg)
+	recs, fetchErr := fetchRecommendationsForRegion(ctx, recClient, service, region, cfg)
+	if fetchErr != nil {
+		// Legacy test-only path: the CLI run goes through fetchAndFilterRegionRecs,
+		// which returns this error to the caller.
+		AppLogger.Printf("  ❌ Failed to fetch recommendations: %v\n", fetchErr)
+		return result
+	}
 	if len(recs) == 0 {
 		AppLogger.Printf("  ℹ️  No recommendations found\n")
 		return result
@@ -582,14 +588,21 @@ func processRegionRecommendations(
 	return result
 }
 
+// errRecommendationFetch marks a failure to obtain recommendations from the
+// provider, as opposed to a successful but empty answer.
+var errRecommendationFetch = errors.New("recommendation fetch failed")
+
 // fetchRecommendationsForRegion fetches recommendations from AWS for a specific region.
+// A provider failure is returned, not logged away: an empty slice with a nil
+// error means the provider answered and had nothing to recommend. An
+// incomplete-but-usable answer (partial sweep) is returned with a warning.
 func fetchRecommendationsForRegion(
 	ctx context.Context,
 	recClient provider.RecommendationsClient,
 	service common.ServiceType,
 	region string,
 	cfg Config,
-) []common.Recommendation {
+) ([]common.Recommendation, error) {
 	termStr := "1yr"
 	if cfg.TermYears == 3 {
 		termStr = "3yr"
@@ -622,7 +635,7 @@ func fetchRecommendationsForRegion(
 	var incomplete *recommendations.IncompleteRecommendationsError
 	if errors.As(err, &incomplete) {
 		AppLogger.Printf("  ⚠️  %v\n", incomplete)
-		return recs
+		return recs, nil
 	}
 	if partial := azureprovider.AsPartialSubscriptionFailure(err); partial != nil {
 		// Keep the subscriptions that did answer, but say plainly that the
@@ -634,18 +647,17 @@ func fetchRecommendationsForRegion(
 		for _, f := range partial.Failed {
 			AppLogger.Printf("      subscription %s: %v\n", f.SubscriptionID, f.Err)
 		}
-		return recs
+		return recs, nil
 	}
-	if ctx.Err() != nil {
+	if ctxErr := ctx.Err(); ctxErr != nil {
 		// Interrupted, not a provider failure: the caller stops on ctx.Err().
-		return nil
+		return nil, ctxErr
 	}
 	if err != nil {
-		AppLogger.Printf("  ❌ Failed to fetch recommendations: %v\n", err)
-		return nil
+		return nil, fmt.Errorf("%w: %s in %s: %w", errRecommendationFetch, getServiceDisplayName(service), region, err)
 	}
 
-	return recs
+	return recs, nil
 }
 
 // stampSavingsPlanRegions copies the region Cost Explorer reports for an EC2
@@ -854,7 +866,10 @@ func fetchAndFilterRegionRecs(
 ) ([]common.Recommendation, error) {
 	AppLogger.Printf("\n  📍 [%d/%d] Region: %s\n", regionIndex, totalRegions, region)
 
-	recs := fetchRecommendationsForRegion(ctx, recClient, service, region, cfg)
+	recs, err := fetchRecommendationsForRegion(ctx, recClient, service, region, cfg)
+	if err != nil {
+		return nil, err
+	}
 	if len(recs) == 0 {
 		AppLogger.Printf("  ℹ️  No recommendations found\n")
 		return nil, nil
@@ -930,14 +945,16 @@ func fetchAllRecs(
 			if ctx.Err() != nil {
 				return nil, drops, nil
 			}
-			log.Printf("❌ Failed to determine regions for %s: %v", getServiceDisplayName(service), err)
-			continue
+			return nil, nil, fmt.Errorf("%w: determining regions for %s: %w", errRecommendationFetch, getServiceDisplayName(service), err)
 		}
 		for i, region := range regions {
 			if ctx.Err() != nil {
 				return nil, drops, nil // partial data from an interrupted fetch is not a result
 			}
 			recs, err := fetchAndFilterRegionRecs(ctx, awsCfg, recClient, accountCache, service, region, i+1, len(regions), engineData, cfg, coverageMap, drops)
+			if ctx.Err() != nil {
+				return nil, drops, nil // interrupted mid-fetch: not a result, not a failure
+			}
 			if err != nil {
 				return nil, nil, err
 			}

cmd/recommendation_completeness_test.go

@@ -67,9 +67,20 @@ func runCompletenessScenario(t *testing.T, service, regionMode, details string)
 	}
 	var stdout, stderr bytes.Buffer
 	child.Stdout, child.Stderr = &stdout, &stderr
-	require.NoError(t, child.Run(), "stdout: %s\nstderr: %s", &stdout, &stderr)
-	fixture.assertRequests(t)
+	runErr := child.Run()
 	logs := stdout.String() + stderr.String()
+	if details == "api-error" || details == "failed-type" {
+		// A provider failure is a failed run, not "nothing to buy".
+		var exitErr *exec.ExitError
+		require.ErrorAs(t, runErr, &exitErr, logs)
+		require.Equal(t, 1, exitErr.ExitCode(), logs)
+		require.Contains(t, logs, "Cannot complete the recommendation fetch")
+		require.Contains(t, logs, "fixture denied recommendations")
+		require.NoFileExists(t, output)
+		return
+	}
+	require.NoError(t, runErr, "stdout: %s\nstderr: %s", &stdout, &stderr)
+	fixture.assertRequests(t)
 	for _, failure := range []string{"Could not check", "Failed to query", "request send failed", "certificate", "TLS handshake"} {
 		require.NotContains(t, logs, failure, "unexpected ancillary error")
 	}
@@ -125,14 +136,8 @@ func assertSPCompleteness(t *testing.T, output, service, details, logs string) {
 		warnings, failedDetails = len(types), 1
 	case "late-page":
 		warnings, failedScopes = len(types), 1
-	case "failed-type":
-		if service == "savingsplans" {
-			survivors = types[:3]
-		} else {
-			survivors = nil
-		}
 	}
-	if details == "empty" || details == "all-invalid" || details == "api-error" {
+	if details == "empty" || details == "all-invalid" {
 		survivors = nil
 	}
 	require.Equal(t, warnings, strings.Count(logs, "incomplete AWS recommendations:"), logs)
@@ -143,15 +148,8 @@ func assertSPCompleteness(t *testing.T, output, service, details, logs string) {
 			require.Contains(t, logs, "not-a-number")
 		}
 	}
-	failures := 0
-	switch details {
-	case "api-error":
-		failures = len(types)
-	case "failed-type":
-		failures = 1
-	}
-	require.Equal(t, failures, strings.Count(logs, "Failed to fetch recommendations:"), logs)
-	if failures > 0 || details == "late-page" {
+	require.NotContains(t, logs, "Failed to fetch recommendations:")
+	if details == "late-page" {
 		require.Contains(t, logs, "fixture denied recommendations")
 	}
 	require.NotContains(t, logs, "Region discovery incomplete")
@@ -203,12 +201,8 @@ func assertCompletenessDiagnostics(t *testing.T, regionMode, details, stdout, st
 	} else {
 		require.NotContains(t, stdout, "incomplete AWS recommendations")
 	}
-	if details == "api-error" {
-		require.Contains(t, stdout+stderr, "fixture denied recommendations")
-	} else {
-		require.NotContains(t, stdout+stderr, "Failed to fetch recommendations", "partial collection treated as fatal")
-		require.NotContains(t, stdout+stderr, "failed to discover regions", "partial collection treated as fatal")
-	}
+	require.NotContains(t, stdout+stderr, "Failed to fetch recommendations", "partial collection treated as fatal")
+	require.NotContains(t, stdout+stderr, "failed to discover regions", "partial collection treated as fatal")
 }
 
 func assertCompletenessCSV(t *testing.T, output, details, logs string) {
@@ -262,14 +256,16 @@ func TestRecommendationCompletenessHelpers(t *testing.T) {
 			client.On("GetRecommendationsForService", ctx, common.ServiceRDS).Return(tc.recs, tc.err)
 			client.On("GetRecommendations", ctx, mock.Anything).Return(tc.recs, tc.err)
 			regions, err := discoverRegionsForService(ctx, client, common.ServiceRDS)
-			got := fetchRecommendationsForRegion(ctx, client, common.ServiceRDS, "us-east-1", Config{TermYears: 1})
+			got, fetchErr := fetchRecommendationsForRegion(ctx, client, common.ServiceRDS, "us-east-1", Config{TermYears: 1})
 			if !tc.keep {
 				require.ErrorIs(t, err, tc.err)
+				require.ErrorIs(t, fetchErr, tc.err)
 				require.Nil(t, regions)
 				require.Nil(t, got)
 				require.NotContains(t, output.String(), "incomplete AWS recommendations")
 			} else {
 				require.NoError(t, err)
+				require.NoError(t, fetchErr)
 				require.Equal(t, tc.recs, got)
 				require.Len(t, regions, len(tc.recs))
 				require.Equal(t, 2, strings.Count(output.String(), "2 failed details, 1 failed scopes"))

cmd/savings_plan_purchase_test.go

@@ -419,7 +419,8 @@ func TestFetchRecommendationsForRegion_SavingsPlanDoesNotFilterOnEndpointRegion(
 			Run(func(args mock.Arguments) { got = args.Get(1).(*common.RecommendationParams) }).
 			Return([]common.Recommendation{}, nil)
 
-		fetchRecommendationsForRegion(context.Background(), client, tc.service, spAPIRegion, Config{TermYears: 1})
+		_, err := fetchRecommendationsForRegion(context.Background(), client, tc.service, spAPIRegion, Config{TermYears: 1})
+		require.NoError(t, err)
 
 		require.NotNil(t, got, tc.service)
 		assert.Equal(t, tc.wantRegion, got.Region, tc.service)

cmd/savings_plan_region_test.go

@@ -32,7 +32,8 @@ func TestFetchRecommendationsForRegion_SavingsPlanRowsCarryDetailsRegion(t *test
 	client.On("GetRecommendations", mock.Anything, mock.Anything).
 		Return([]common.Recommendation{ec2InstanceSPRow(), compute}, nil)
 
-	got := fetchRecommendationsForRegion(context.Background(), client, common.ServiceSavingsPlansEC2Instance, spAPIRegion, Config{TermYears: 1})
+	got, err := fetchRecommendationsForRegion(context.Background(), client, common.ServiceSavingsPlansEC2Instance, spAPIRegion, Config{TermYears: 1})
+	require.NoError(t, err)
 
 	require.Len(t, got, 2)
 	assert.Equal(t, "eu-west-2", got[0].Region)

cmd/sp_account_scope_test.go

@@ -33,8 +33,9 @@ func TestFetchRecommendationsForRegion_PassesSPAccountScope(t *testing.T) {
 		Run(func(args mock.Arguments) { got = args.Get(1).(*common.RecommendationParams) }).
 		Return([]common.Recommendation{}, nil)
 
-	fetchRecommendationsForRegion(context.Background(), client, common.ServiceSavingsPlansCompute, spAPIRegion,
+	_, err := fetchRecommendationsForRegion(context.Background(), client, common.ServiceSavingsPlansCompute, spAPIRegion,
 		Config{TermYears: 1, SPAccountScope: "payer"})
+	require.NoError(t, err)
 
 	require.NotNil(t, got)
 	assert.Equal(t, "payer", got.SPAccountScope)