diff --git a/go/cmd/gitter/git.go b/go/cmd/gitter/git.go index 60a8f352c5d..bd11a6556b4 100644 --- a/go/cmd/gitter/git.go +++ b/go/cmd/gitter/git.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "fmt" "log/slog" "os" @@ -11,6 +10,7 @@ import ( "syscall" "time" + "github.com/google/osv.dev/go/internal/osvutil" "github.com/google/osv.dev/go/logger" ) @@ -48,7 +48,7 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... if err != nil { if ctx.Err() != nil { // Log separately if cancelled - logger.WarnContext(ctx, "Command cancelled", slog.String("cmd", name), slog.Any("err", ctx.Err())) + logger.DebugContext(ctx, "Command cancelled", slog.String("cmd", name), slog.Any("err", ctx.Err())) return fmt.Errorf("command %s cancelled: %w", name, ctx.Err()) } @@ -81,7 +81,7 @@ func attemptGitRecovery(ctx context.Context, repoPath string, err error) bool { if isRefConflictError(err) { logger.WarnContext(ctx, "Ref conflict detected, running git remote prune origin") if err := runCmd(ctx, repoPath, nil, "git", "remote", "prune", "origin"); err != nil { - logger.ErrorContext(ctx, "Failed to prune origin", slog.Any("err", err)) + logger.WarnContext(ctx, "Failed to prune origin", slog.Any("err", err)) return false } @@ -107,7 +107,7 @@ func fetchRepo(ctx context.Context, repoPath string) error { // Make sure origin/HEAD points to the latest default branch from remotes err = runCmd(ctx, repoPath, nil, "git", "remote", "set-head", "origin", "--auto") if err != nil { - logger.WarnContext(ctx, "git remote set-head failed: ", slog.Any("err", err)) + logger.WarnContext(ctx, "git remote set-head failed", slog.Any("err", err)) } return nil @@ -153,7 +153,7 @@ func refreshRepo(ctx context.Context, repoURL string, forceUpdate bool) error { // Attempt recovery and retry fetch if successful if attemptGitRecovery(ctx, repoPath, err) { - logger.InfoContext(ctx, "Retrying fetch after recovery") + logger.DebugContext(ctx, "Retrying fetch after recovery") err = fetchRepo(ctx, repoPath) } @@ -167,7 +167,7 @@ func refreshRepo(ctx context.Context, repoURL string, forceUpdate bool) error { reason = "upstream rate limit or load" case isRemoteHostError(err): reason = "remote host or network error" - case ctx.Err() != nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded): + case osvutil.IsContextError(err): reason = "context cancelled" } @@ -190,7 +190,7 @@ func refreshRepo(ctx context.Context, repoURL string, forceUpdate bool) error { return fmt.Errorf("failed to remove repo directory for reclone: %w", err) } - logger.InfoContext(ctx, "Cloning git repository after fallback") + logger.DebugContext(ctx, "Cloning git repository after fallback") if err := cloneRepo(ctx, repoURL, repoPath); err != nil { return fmt.Errorf("git clone failed after fallback: %w", err) } @@ -211,7 +211,7 @@ func refreshRepo(ctx context.Context, repoURL string, forceUpdate bool) error { return fmt.Errorf("failed to read file: %w", err) } - logger.InfoContext(ctx, "Repository refresh completed", slog.Duration("duration", time.Since(start))) + logger.DebugContext(ctx, "Repository refresh completed", slog.Duration("duration", time.Since(start))) return nil } @@ -222,10 +222,6 @@ func SyncRepoOnDisk(ctx context.Context, repoURL string, opts FetchOptions) (*Re _, err, _ := gFetch.Do(repoURL, func() (any, error) { return runWithConcurrencyControl(ctx, opts.SkipReqConcurrencySemaphore, func() (any, error) { err := refreshRepo(ctx, repoURL, opts.ForceUpdate) - if err != nil { - logger.ErrorContext(ctx, "Error syncing repository on disk", slog.Any("error", err)) - } - return nil, err }) }) @@ -262,7 +258,7 @@ func LoadRepo(ctx context.Context, repoURL string, opts FetchOptions) (*Reposito repo, err := LoadRepository(ctx, repoPath) if err != nil { - logger.ErrorContext(ctx, "Failed to load repository", slog.Any("error", err)) + logger.WarnContext(ctx, "Failed to load repository", slog.Any("error", err)) } return repo, err @@ -310,7 +306,7 @@ func ArchiveRepo(ctx context.Context, repoURL string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("tar zstd failed: %w", err) } - logger.InfoContext(ctx, "Archiving git blob completed", slog.Duration("duration", time.Since(startArchive))) + logger.DebugContext(ctx, "Archiving git blob completed", slog.Duration("duration", time.Since(startArchive))) } // If the context is cancelled, still do the fetching stuff, just don't bother returning the result diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index a37dbc2d96a..db7432aa9ee 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -783,8 +783,8 @@ func tagsHandler(w http.ResponseWriter, req *http.Request) { logger.DebugContext(ctx, "Local repo not found, using ls-remote") tagsMapAny, errLsRemote, _ := gLsRemote.Do(repoURL, func() (any, error) { tags, err := repo.GetRemoteTags(ctx) - if err != nil && !isAuthError(err) && !isForbiddenError(err) && !isNotFoundError(err) { - logger.ErrorContext(ctx, "Error running git ls-remote", slog.Any("error", err)) + if err != nil && !isAuthError(err) && !isForbiddenError(err) && !isNotFoundError(err) && !isRateLimitError(err) && !isRemoteHostError(err) && !errors.Is(ctx.Err(), context.Canceled) && !errors.Is(ctx.Err(), context.DeadlineExceeded) { + logger.WarnContext(ctx, "Error running git ls-remote", slog.Any("error", err)) } return tags, err @@ -801,7 +801,7 @@ func tagsHandler(w http.ResponseWriter, req *http.Request) { } if len(tagsMap) == 0 { - logger.InfoContext(ctx, "No tags in repository") + logger.DebugContext(ctx, "No tags in repository") invalidRepoCache.SetWithTTL(repoURL, http.StatusNoContent, 1, invalidRepoTTL) statusCode = http.StatusNoContent w.WriteHeader(statusCode) diff --git a/go/cmd/gitter/persistence.go b/go/cmd/gitter/persistence.go index 4b6edfe5f68..6b16bc703a5 100644 --- a/go/cmd/gitter/persistence.go +++ b/go/cmd/gitter/persistence.go @@ -30,7 +30,7 @@ func updateLastFetch(url string) { // deleteLastFetch removed an entry from the last fetch map for cases where the git dir does not exist // This should not happen, but if it does, we should clean up the last fetch map to allow refetching func deleteLastFetch(url string) { - logger.Error("Cache says file should exist, but file does not exist.", slog.String("url", url)) + logger.Warn("Cache says file should exist, but file does not exist.", slog.String("url", url)) lastFetchMu.Lock() defer lastFetchMu.Unlock() @@ -70,7 +70,7 @@ func loadLastFetchMap() { data, err := os.ReadFile(persistencePath) if err != nil { if !os.IsNotExist(err) { - logger.Error("Error reading lastFetch map", slog.String("path", persistencePath), slog.Any("error", err)) + logger.Warn("Error reading lastFetch map", slog.String("path", persistencePath), slog.Any("error", err)) } return @@ -80,7 +80,7 @@ func loadLastFetchMap() { defer lastFetchMu.Unlock() if err := json.Unmarshal(data, &lastFetch); err != nil { - logger.Error("Error unmarshaling lastFetch map", slog.String("path", persistencePath), slog.Any("error", err)) + logger.Warn("Error unmarshaling lastFetch map", slog.String("path", persistencePath), slog.Any("error", err)) } logger.Debug("Loaded lastFetch map", slog.Int("entry_count", len(lastFetch))) diff --git a/go/cmd/gitter/repository.go b/go/cmd/gitter/repository.go index c2e2f513335..ea303a76112 100644 --- a/go/cmd/gitter/repository.go +++ b/go/cmd/gitter/repository.go @@ -119,8 +119,8 @@ func LoadRepository(ctx context.Context, repoPath string) (*Repository, error) { patchIDErr = repo.calculatePatchIDs(ctx, newCommits) } - // If error is anything other than context cancel, exit early without saving - if patchIDErr != nil && !errors.Is(ctx.Err(), context.Canceled) { + // If error is anything other than context cancel/timeout, exit early without saving + if patchIDErr != nil && !errors.Is(ctx.Err(), context.Canceled) && !errors.Is(ctx.Err(), context.DeadlineExceeded) { return nil, fmt.Errorf("failed to calculate patch id for commits: %w", patchIDErr) } @@ -194,7 +194,7 @@ func (r *Repository) buildCommitGraph(ctx context.Context, cache *pb.RepositoryC cmd.Stderr = &stderr if err := cmd.Run(); err != nil { if ctx.Err() != nil { - logger.WarnContext(ctx, "Command cancelled", slog.String("cmd", "git log"), slog.Any("err", ctx.Err())) + logger.DebugContext(ctx, "Command cancelled", slog.String("cmd", "git log"), slog.Any("err", ctx.Err())) return nil, fmt.Errorf("command git log cancelled: %w", ctx.Err()) } @@ -261,8 +261,8 @@ func (r *Repository) buildCommitGraph(ctx context.Context, cache *pb.RepositoryC } childHash = SHA1(hash) default: - // No line should be completely empty (doesn't even have a commit hash) so error - logger.ErrorContext(ctx, "Invalid commit info", slog.String("line", line)) + // No line should be completely empty (doesn't even have a commit hash) + logger.WarnContext(ctx, "Invalid commit info", slog.String("line", line)) continue } @@ -504,7 +504,7 @@ func (r *Repository) parseHashes(ctx context.Context, hashesStr []string) []int if idx, ok := r.hashToIndex[h]; ok { indices = append(indices, idx) } else { - logger.ErrorContext(ctx, "commit hash not found in repository", slog.String("hash", hash)) + logger.WarnContext(ctx, "commit hash not found in repository", slog.String("hash", hash)) } }