Bug fix for batch size issue (AST-181549)

internal/services/sast-metadata.go

@@ -2,6 +2,7 @@ package services
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"sync"
 
@@ -11,7 +12,8 @@ import (
 )
 
 const (
-	BatchSize = 200
+	// BatchSize must not exceed the sast-metadata backend's per-request scan-ids cap (currently 50).
+	BatchSize = 50
 )
 
 func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scanIDs []string) (*wrappers.SastMetadataModel, error) {
@@ -21,7 +23,7 @@ func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scan
 
 	var wg sync.WaitGroup
 	results := make(chan wrappers.SastMetadataModel, totalBatches)
-	errors := make(chan error, totalBatches)
+	errCh := make(chan error, totalBatches)
 	ctx := context.Background()
 
 	for i := 0; i < totalBatches; i++ {
@@ -46,21 +48,23 @@ func GetSastMetadataByIDs(sastMetaDataWrapper wrappers.SastMetadataWrapper, scan
 
 			result, err := sastMetaDataWrapper.GetSastMetadataByIDs(batchParams)
 			if err != nil {
-				errors <- err
+				errCh <- err
 				return
 			}
 			results <- *result
 		}()
 	}
 
-	go func() {
-		wg.Wait()
-		close(results)
-		close(errors)
-	}()
+	wg.Wait()
+	close(results)
+	close(errCh)
 
-	if len(errors) > 0 {
-		return nil, <-errors
+	if len(errCh) > 0 {
+		var batchErrs []error
+		for batchErr := range errCh {
+			batchErrs = append(batchErrs, batchErr)
+		}
+		return nil, errors.Join(batchErrs...)
 	}
 
 	var models []wrappers.SastMetadataModel

internal/services/sast-metadata_test.go

@@ -48,6 +48,28 @@ func TestGetSastMetadataByIDs(t *testing.T) {
 	}
 }
 
+func TestGetSastMetadataByIDs_BatchFailureIsSurfacedNotPanicked(t *testing.T) {
+	// A failing batch must be reliably surfaced as an error, not raced past or dereferenced as nil.
+	scanIDs := append(createScanIDs(BatchSize), createErrorScanIDs(BatchSize)...)
+	scanIDs = append(scanIDs, createScanIDs(BatchSize)...)
+
+	wrapper := &mock.SastMetadataMockWrapper{}
+	actual, err := GetSastMetadataByIDs(wrapper, scanIDs)
+
+	assert.Nil(t, actual)
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "Too many ids in url")
+	}
+}
+
+func createErrorScanIDs(count int) []string {
+	scanIDs := make([]string, count)
+	for i := 0; i < count; i++ {
+		scanIDs[i] = fmt.Sprintf("TriggerError%d", i)
+	}
+	return scanIDs
+}
+
 func createScanIDs(count int) []string {
 	scanIDs := make([]string, count)
 	for i := 0; i < count; i++ {

internal/wrappers/mock/sast-metadata-mock.go

@@ -1,6 +1,7 @@
 package mock
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/checkmarx/ast-cli/internal/wrappers"
@@ -10,8 +11,11 @@ import (
 type SastMetadataMockWrapper struct{}
 
 const (
-	scanIDParam = "scan-ids"
-	concurrent  = "ConcurrentTest"
+	scanIDParam         = "scan-ids"
+	concurrent          = "ConcurrentTest"
+	errorTrigger        = "TriggerError"
+	tooManyIDsErrorCode = 3102
+	tooManyIDsMessage   = "Too many ids in url"
 	// FakeMetadataErrorID is used to trigger error responses in mock SAST metadata calls.
 	FakeMetadataErrorID = "fake-sast-metadata-error-id"
 	// FakeMetadataEmptyID is used to trigger empty responses in mock SAST metadata calls.
@@ -22,6 +26,9 @@ func (s SastMetadataMockWrapper) GetSastMetadataByIDs(params map[string]string)
 	*wrappers.SastMetadataModel,
 	error,
 ) {
+	if strings.Contains(params[scanIDParam], errorTrigger) {
+		return nil, fmt.Errorf("%d - %s", tooManyIDsErrorCode, tooManyIDsMessage)
+	}
 	if strings.Contains(params[scanIDParam], FakeMetadataErrorID) {
 		return nil, errors.New("fake sast metadata fetch error")
 	}

internal/wrappers/sast-metadata-http.go

@@ -27,13 +27,10 @@ func (s *SastIncrementalHTTPWrapper) GetSastMetadataByIDs(params map[string]stri
 	if err != nil {
 		return nil, err
 	}
-	decoder := json.NewDecoder(resp.Body)
-
 	defer func() {
-		if err == nil {
-			_ = resp.Body.Close()
-		}
+		_ = resp.Body.Close()
 	}()
+	decoder := json.NewDecoder(resp.Body)
 
 	switch resp.StatusCode {
 	case http.StatusBadRequest, http.StatusInternalServerError:
@@ -42,7 +39,7 @@ func (s *SastIncrementalHTTPWrapper) GetSastMetadataByIDs(params map[string]stri
 		if err != nil {
 			return nil, fmt.Errorf("%v %s", err, failedToParseGetAll)
 		}
-		return nil, err
+		return nil, fmt.Errorf("%d - %s", errorModel.Code, errorModel.Message)
 	case http.StatusOK:
 		model := SastMetadataModel{}
 		err = decoder.Decode(&model)