internal/work: classify more build errors

Otherwise, they'll be considered MISC errors which might confuse users:
misc errors might indicate issues in the tools, but build issues are
related to projects. This will also increase the size of unrecoverable
error set, making the govulncheck pipeline finish faster.

Few more comments are added to clarify how we classify errors.

Change-Id: I426532161401d22367fd699a682e2993d186d2d5
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/523057
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Maceo Thompson <maceothompson@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/internal/worker/analysis.go b/internal/worker/analysis.go
index eb52215..8fd820b 100644
--- a/internal/worker/analysis.go
+++ b/internal/worker/analysis.go
@@ -201,11 +201,18 @@
 		return addSource(ctx, row.Diagnostics, 1)
 	})
 	if err != nil {
+		// The errors are classified as to explicitly make a distinction
+		// between misc errors for modules and non-modules. The intended
+		// audience for analysis pipeline will directly look at errors.
+		// Without this distinction, experiments where there are a lot of
+		// misc errors might sway users into thinking that something is
+		// wrong with their analysis, while in fact it can be the case
+		// that synthetic (non-modules) are just outdated.
 		switch {
 		case isNoModulesSpecified(err):
 			// We try to turn every non-module project into a module, so this
 			// branch should never be reached. We keep this for sanity and to
-			// catch any errors during the conversion.
+			// catch any regressions.
 			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesNoGoModError)
 		case isNoRequiredModule(err):
 			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesNoRequiredModuleError)
@@ -219,6 +226,8 @@
 			err = fmt.Errorf("%v: %w", err, derrors.LoadVendorError)
 		case isProxyCacheMiss(err):
 			err = fmt.Errorf("%v: %w", err, derrors.ProxyError)
+		case isBuildIssue(err):
+			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesError)
 		case !hasGoMod:
 			// Classify misc errors on synthetic modules separately.
 			err = fmt.Errorf("%v: %w", err, derrors.ScanSyntheticModuleError)
diff --git a/internal/worker/govulncheck_scan.go b/internal/worker/govulncheck_scan.go
index d5f5da6..206b2f6 100644
--- a/internal/worker/govulncheck_scan.go
+++ b/internal/worker/govulncheck_scan.go
@@ -320,10 +320,7 @@
 	row.ScanMemory = int64(stats.ScanMemory)
 	if err != nil {
 		switch {
-		case isMissingGoMod(err) || isNoModulesSpecified(err):
-			// Covers the missing go.mod file cases when running govulncheck in the sandbox
-			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesNoGoModError)
-		case isLoadError(err):
+		case isGovulncheckLoadError(err) || isBuildIssue(err):
 			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesError)
 		case isNoRequiredModule(err):
 			// Should be subsumed by LoadPackagesError, kept for sanity
@@ -341,6 +338,10 @@
 			// Should be subsumed by LoadPackagesError, kept for sanity.
 			// and to catch unexpected changes in govulncheck output.
 			err = fmt.Errorf("%v: %w", err, derrors.LoadVendorError)
+		case isMissingGoMod(err) || isNoModulesSpecified(err):
+			// Should be subsumed by LoadPackagesError, kept for sanity
+			// and to catch unexpected changes in govulncheck output.
+			err = fmt.Errorf("%v: %w", err, derrors.LoadPackagesNoGoModError)
 		case isTooManyFiles(err):
 			err = fmt.Errorf("%v: %w", err, derrors.ScanModuleTooManyOpenFiles)
 		case isProxyCacheMiss(err):
@@ -558,7 +559,7 @@
 	return findings, nil
 }
 
-func isLoadError(err error) bool {
+func isGovulncheckLoadError(err error) bool {
 	return strings.Contains(err.Error(), "govulncheck: loading packages:") ||
 		strings.Contains(err.Error(), "FindAndBuildBinaries")
 }
diff --git a/internal/worker/scan.go b/internal/worker/scan.go
index 1d41b05..65e2f12 100644
--- a/internal/worker/scan.go
+++ b/internal/worker/scan.go
@@ -353,3 +353,16 @@
 	errStr := err.Error()
 	return strings.Contains(errStr, "server response") && strings.Contains(errStr, "temporarily unavailable")
 }
+
+// isBuildIssue recognizes general load issues that might not
+// be directly captured by govulncheck or analysis binaries.
+// These errors happen sometimes when go mod download fails
+// but are effectively a build issue from user perspective.
+// They often occur for synthetic projects, but module projects
+// might have them as well.
+func isBuildIssue(err error) bool {
+	errStr := err.Error()
+	return strings.Contains(errStr, "does not contain package") ||
+		strings.Contains(errStr, "but was required") ||
+		strings.Contains(errStr, "relative import paths are not supported in module mode")
+}