log error but return nil to ensure deps.dev continues if package is n…

…ot found (#2416) * log error but return nil to ensure deps.dev continues if package is not found Signed-off-by: pxp928 <[email protected]> * add unit test for invalid package and esure it does not error but just log the error Signed-off-by: pxp928 <[email protected]> * move error logging from private function and add comments on why we are logging the error Signed-off-by: pxp928 <[email protected]> --------- Signed-off-by: pxp928 <[email protected]>
guacsec · Jan 6, 2025 · 1e2e2e7 · 1e2e2e7
1 parent 6dad00a
commit 1e2e2e7
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 22 deletions.
diff --git a/internal/client/depsdevclient/deps_dev_client.go b/internal/client/depsdevclient/deps_dev_client.go
@@ -315,13 +315,19 @@ func (d *DepsClient) RetrieveDependencies(ctx context.Context, purls []string) e
 // GetDependencies gets the information about dependencies for a PURL. This should be called after a
 // call to RetrieveDependencies.
 func (d *DepsClient) GetDependencies(ctx context.Context, purls []string) ([]*PackageComponent, error) {
+	logger := logging.FromContext(ctx)
 	var pcs []*PackageComponent
 	for _, purl := range purls {
 		pc, err := d.getDependenciesForPurl(ctx, purl)
+		// log the error message if getDependenciesForPurl are not found but continue forward
+		// this is to because there will be packages that deps.dev does not know about thus,
+		// we will skip those and move forward.
 		if err != nil {
-			return nil, fmt.Errorf("failed to getDependencies for purl %v, with err: %w", purl, err)
+			logger.Debugf("failed to getDependenciesForPurl with error: %v", err)
+		}
+		if pc != nil {
+			pcs = append(pcs, pc)
 		}
-		pcs = append(pcs, pc)
 	}
 	return pcs, nil
 }
@@ -334,51 +340,46 @@ func (d *DepsClient) getDependenciesForPurl(ctx context.Context, purl string) (*
 
 	packageInput, err := helpers.PurlToPkg(purl)
 	if err != nil {
-		logger.Infof("failed to parse purl to pkg: %s", purl)
-		return nil, fmt.Errorf("failed to parse purl to pkg: %w", err)
+		return nil, fmt.Errorf("failed to parse purl to pkg: %s", purl)
 	}
 
 	// skip all type guac as they are generated by guac and will not be found in deps.dev
 	if packageInput.Type == "guac" {
-		logger.Debugf("guac purl, skipping deps.dev query: %s", purl)
-		return nil, nil
+		return nil, fmt.Errorf("guac purl, skipping deps.dev query: %s", purl)
 	}
 
 	// if version is not specified, cannot obtain accurate information from deps.dev. Log as info and skip the purl.
 	if *packageInput.Version == "" {
-		logger.Infof("purl does not contain version, skipping deps.dev query: %s", purl)
-		return nil, nil
+		return nil, fmt.Errorf("purl does not contain version, skipping deps.dev query: %s", purl)
 	}
 
 	component.CurrentPackage = packageInput
 
-	err = d.collectAdditionalMetadata(ctx, packageInput.Type, packageInput.Namespace, packageInput.Name, packageInput.Version, component)
-	if err != nil {
-		logger.Debugf("failed to get additional metadata for package: %s, err: %v", purl, err)
+	if err := d.collectAdditionalMetadata(ctx, packageInput.Type, packageInput.Namespace, packageInput.Name, packageInput.Version, component); err != nil {
+		return nil, fmt.Errorf("failed to get additional metadata for package: %s, err: %w", purl, err)
 	}
 
 	// Make an RPC Request. The returned result is a stream of
 	// DependenciesResponse structs.
 	versionKey, err := getVersionKey(packageInput.Type, packageInput.Namespace, packageInput.Name, packageInput.Version)
 	if err != nil {
-		logger.Infof("failed to getVersionKey with the following error: %v", err)
-		return nil, err
+		return nil, fmt.Errorf("failed to getVersionKey with the following error: %w", err)
 	}
 
 	dependenciesReq := &pb.GetDependenciesRequest{
 		VersionKey: versionKey,
 	}
 	var deps *pb.Dependencies
+	var clientDepsErr error
 	if _, ok := d.dependencies[versionKey.String()]; ok {
 		deps = d.dependencies[versionKey.String()]
 	} else {
 		logger.Debugf("The version key was not found in the map: %v", versionKey)
-		deps, err = d.client.GetDependencies(ctx, dependenciesReq)
-		if err != nil {
-			logger.Debugf("failed to get dependencies: %v", err)
-			return nil, err
+		deps, clientDepsErr = d.client.GetDependencies(ctx, dependenciesReq)
+		if clientDepsErr != nil {
+			return nil, fmt.Errorf("failed to get dependencies: %w", clientDepsErr)
 		}
-		logger.Infof("Retrieved dependencies for %s", purl)
+		logger.Debugf("Retrieved dependencies for %s", purl)
 		d.dependencies[versionKey.String()] = deps
 	}
 
@@ -405,7 +406,7 @@ func (d *DepsClient) getDependenciesForPurl(ctx context.Context, purl string) (*
 		depPurl := "pkg:" + pkgtype + "/" + node.VersionKey.Name + "@" + node.VersionKey.Version
 		depPackageInput, err := helpers.PurlToPkg(depPurl)
 		if err != nil {
-			logger.Infof("unable to parse purl: %v, error: %v", depPurl, err)
+			logger.Debugf("unable to parse purl: %v, error: %v", depPurl, err)
 			continue
 		}
 		// check if dependent package purl has already been queried. If found, append to the list of dependent packages for top level package
@@ -418,8 +419,8 @@ func (d *DepsClient) getDependenciesForPurl(ctx context.Context, purl string) (*
 			continue
 		}
 		depComponent.CurrentPackage = depPackageInput
-		err = d.collectAdditionalMetadata(ctx, depPackageInput.Type, depPackageInput.Namespace, depPackageInput.Name, depPackageInput.Version, depComponent)
-		if err != nil {
+		if err := d.collectAdditionalMetadata(ctx, depPackageInput.Type, depPackageInput.Namespace, depPackageInput.Name, depPackageInput.Version, depComponent); err != nil {
+			// if additional metadata is not found (not found in deps.dev) log the error and move forward
 			logger.Debugf("failed to get additional metadata for package: %s, err: %v", depPurl, err)
 		}
 		dependencyNodes = append(dependencyNodes, depComponent)

diff --git a/internal/client/depsdevclient/deps_dev_client_test.go b/internal/client/depsdevclient/deps_dev_client_test.go
@@ -68,7 +68,7 @@ func Test_depsCollector_GetX(t *testing.T) {
 		{
 			name:     "invalid packages",
 			packages: []string{"not-a-package"},
-			wantErr:  true,
+			wantErr:  false,
 		},
 		{
 			name:     "org.webjars.npm:a maven package",
@@ -380,6 +380,10 @@ func Test_depsCollector_GetDependenciesEq(t *testing.T) {
 			name:     "multiple different packages",
 			packages: []string{"pkg:cargo/[email protected]", "pkg:npm/[email protected]"},
 		},
+		{
+			name:     "deb package - invalid",
+			packages: []string{"pkg:deb/org.webjars.npm/[email protected]"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {