From c6f8c671a3d29949a47aca0e7600a74eee4c4472 Mon Sep 17 00:00:00 2001 From: Joey L Date: Fri, 13 Feb 2026 00:37:10 +0000 Subject: [PATCH 1/5] More logging in runCmd --- go/cmd/gitter/gitter.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 79eba120170..c605a6a974b 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -43,6 +43,7 @@ const shutdownTimeout = 10 * time.Second // runCmd executes a command with context cancellation handled by sending SIGINT. // It logs cancellation errors separately as requested. func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) error { + logger.Debug("Running command", slog.String("cmd", name), slog.String("dir", dir), slog.Any("args", args)) cmd := exec.CommandContext(ctx, name, args...) if dir != "" { cmd.Dir = dir @@ -52,6 +53,7 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... } // Use SIGINT instead of SIGKILL for graceful shutdown of subprocesses cmd.Cancel = func() error { + logger.Debug("SIGINT sent to command", slog.String("cmd", name), slog.String("dir", dir), slog.Any("args", args)) return cmd.Process.Signal(syscall.SIGINT) } // Ensure it eventually dies if it ignores SIGINT @@ -67,6 +69,7 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... return fmt.Errorf("command %s failed: %w, output: %s", name, err, out) } + logger.Debug("Command completed successfully", slog.String("cmd", name), slog.String("out", string(out))) return nil } From af15b936a45506d44887569bbb0aba113b6a55b9 Mon Sep 17 00:00:00 2001 From: Joey L Date: Fri, 13 Feb 2026 01:00:53 +0000 Subject: [PATCH 2/5] Add rm and retry logic for when index.lock exist --- go/cmd/gitter/gitter.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index c605a6a974b..2e3536ba53b 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -17,6 +17,7 @@ import ( "os/exec" "os/signal" "path" + "path/filepath" "regexp" "strings" "syscall" @@ -106,6 +107,11 @@ func isAuthError(err error) bool { (strings.Contains(strings.ToLower(errString), "repository") && strings.Contains(strings.ToLower(errString), "not found")) } +func isIndexLockError(err error) bool { + errString := err.Error() + return strings.Contains(errString, "index.lock") && strings.Contains(errString, "File exists") +} + func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error) { repoDirName := getRepoDirName(url) repoPath := path.Join(gitStorePath, repoDirName) @@ -133,6 +139,18 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error return nil, fmt.Errorf("git fetch failed: %w", err) } err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD") + if isIndexLockError(err) { + // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. + // We can remove the file and retry the command + logger.Warn("index.lock exists, attempting to remove and retry", slog.String("repoPath", repoPath)) + indexLockPath := filepath.Join(repoPath, ".git", "index.lock") + if err := os.Remove(indexLockPath); err != nil { + return nil, fmt.Errorf("failed to remove index.lock in %s: %w", repoPath, err) + } + // One more attempt at git reset + err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD") + } + if err != nil { return nil, fmt.Errorf("git reset failed: %w", err) } From 47c74c2f400aaa86df4ab1288ae3ebf2246b45fc Mon Sep 17 00:00:00 2001 From: Joey L Date: Fri, 13 Feb 2026 04:31:12 +0000 Subject: [PATCH 3/5] more consistent logging with url in ctx --- go/cmd/gitter/gitter.go | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 2e3536ba53b..4c8e345dab4 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -44,7 +44,7 @@ const shutdownTimeout = 10 * time.Second // runCmd executes a command with context cancellation handled by sending SIGINT. // It logs cancellation errors separately as requested. func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) error { - logger.Debug("Running command", slog.String("cmd", name), slog.String("dir", dir), slog.Any("args", args)) + logger.Debug("Running command", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("args", args)) cmd := exec.CommandContext(ctx, name, args...) if dir != "" { cmd.Dir = dir @@ -54,7 +54,7 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... } // Use SIGINT instead of SIGKILL for graceful shutdown of subprocesses cmd.Cancel = func() error { - logger.Debug("SIGINT sent to command", slog.String("cmd", name), slog.String("dir", dir), slog.Any("args", args)) + logger.Debug("SIGINT sent to command", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("args", args)) return cmd.Process.Signal(syscall.SIGINT) } // Ensure it eventually dies if it ignores SIGINT @@ -64,13 +64,13 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... if err != nil { if ctx.Err() != nil { // Log separately if cancelled - logger.Warn("Command cancelled", slog.String("cmd", name), slog.Any("err", ctx.Err())) + logger.Warn("Command cancelled", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("err", ctx.Err())) return fmt.Errorf("command %s cancelled: %w", name, ctx.Err()) } return fmt.Errorf("command %s failed: %w, output: %s", name, err, out) } - logger.Debug("Command completed successfully", slog.String("cmd", name), slog.String("out", string(out))) + logger.Debug("Command completed successfully", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.String("out", string(out))) return nil } @@ -123,7 +123,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error // Check if we need to fetch if forceUpdate || !ok || time.Since(accessTime) > fetchTimeout { - logger.Info("Fetching git blob", slog.String("url", url), slog.Duration("sinceAccessTime", time.Since(accessTime))) + logger.Info("Fetching git blob", slog.String("url", ctx.Value("url").(string)), slog.Duration("sinceAccessTime", time.Since(accessTime))) if _, err := os.Stat(path.Join(repoPath, ".git")); os.IsNotExist(err) { // Clone err := runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", url, repoPath) @@ -142,7 +142,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error if isIndexLockError(err) { // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. // We can remove the file and retry the command - logger.Warn("index.lock exists, attempting to remove and retry", slog.String("repoPath", repoPath)) + logger.Warn("index.lock exists, attempting to remove and retry", slog.String("url", ctx.Value("url").(string))) indexLockPath := filepath.Join(repoPath, ".git", "index.lock") if err := os.Remove(indexLockPath); err != nil { return nil, fmt.Errorf("failed to remove index.lock in %s: %w", repoPath, err) @@ -156,7 +156,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error } } - logger.Info("Archiving git blob", slog.String("url", url)) + logger.Info("Archiving git blob", slog.String("url", ctx.Value("url").(string))) // Archive // tar --zstd -cf -C "/" . // using -C to archive the relative path so it unzips nicely @@ -263,6 +263,9 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { } forceUpdate := r.URL.Query().Get("force-update") == "true" + ctx := r.Context() + ctx = context.WithValue(ctx, "url", url) + logger.Info("Received request", slog.String("url", url), slog.Bool("forceUpdate", forceUpdate), slog.String("remoteAddr", r.RemoteAddr)) // If request came from a local ip, don't do the check if !isLocalRequest(r) { @@ -281,11 +284,11 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { // This is a tradeoff for simplicity to avoid having to setup locks per repo. //nolint:contextcheck // I can't change singleflight's interface fileData, err, _ := g.Do(url, func() (any, error) { - return fetchBlob(r.Context(), url, forceUpdate) + return fetchBlob(ctx, url, forceUpdate) }) if err != nil { - logger.Error("Error fetching/archiving blob", slog.String("url", url), slog.Any("error", err)) + logger.Error("Error fetching/archiving blob", slog.String("url", ctx.Value("url").(string)), slog.Any("error", err)) if isAuthError(err) { http.Error(w, fmt.Sprintf("Error fetching blob: %v", err), http.StatusForbidden) return @@ -305,5 +308,5 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { return } - logger.Info("Request completed successfully", slog.String("url", url)) + logger.Info("Request completed successfully", slog.String("url", ctx.Value("url").(string))) } From 9620c59bc6ec7419ddf6b70367d1c7e4462c346a Mon Sep 17 00:00:00 2001 From: Joey L Date: Fri, 13 Feb 2026 04:43:59 +0000 Subject: [PATCH 4/5] add context key type --- go/cmd/gitter/gitter.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 4c8e345dab4..142cdbf2f8e 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -27,6 +27,12 @@ import ( "golang.org/x/sync/singleflight" ) +type contextKey string + +const ( + urlKey contextKey = "url" +) + const getGitEndpoint = "/getgit" const defaultGitterWorkDir = "/work/gitter" const persistanceFileName = "last-fetch.json" @@ -44,7 +50,7 @@ const shutdownTimeout = 10 * time.Second // runCmd executes a command with context cancellation handled by sending SIGINT. // It logs cancellation errors separately as requested. func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) error { - logger.Debug("Running command", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("args", args)) + logger.Debug("Running command", slog.String("cmd", name), slog.String("url", ctx.Value(urlKey).(string)), slog.Any("args", args)) cmd := exec.CommandContext(ctx, name, args...) if dir != "" { cmd.Dir = dir @@ -54,7 +60,7 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... } // Use SIGINT instead of SIGKILL for graceful shutdown of subprocesses cmd.Cancel = func() error { - logger.Debug("SIGINT sent to command", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("args", args)) + logger.Debug("SIGINT sent to command", slog.String("cmd", name), slog.String("url", ctx.Value(urlKey).(string)), slog.Any("args", args)) return cmd.Process.Signal(syscall.SIGINT) } // Ensure it eventually dies if it ignores SIGINT @@ -64,13 +70,13 @@ func runCmd(ctx context.Context, dir string, env []string, name string, args ... if err != nil { if ctx.Err() != nil { // Log separately if cancelled - logger.Warn("Command cancelled", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.Any("err", ctx.Err())) + logger.Warn("Command cancelled", slog.String("cmd", name), slog.String("url", ctx.Value(urlKey).(string)), slog.Any("err", ctx.Err())) return fmt.Errorf("command %s cancelled: %w", name, ctx.Err()) } return fmt.Errorf("command %s failed: %w, output: %s", name, err, out) } - logger.Debug("Command completed successfully", slog.String("cmd", name), slog.String("url", ctx.Value("url").(string)), slog.String("out", string(out))) + logger.Debug("Command completed successfully", slog.String("cmd", name), slog.String("url", ctx.Value(urlKey).(string)), slog.String("out", string(out))) return nil } @@ -123,7 +129,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error // Check if we need to fetch if forceUpdate || !ok || time.Since(accessTime) > fetchTimeout { - logger.Info("Fetching git blob", slog.String("url", ctx.Value("url").(string)), slog.Duration("sinceAccessTime", time.Since(accessTime))) + logger.Info("Fetching git blob", slog.String("url", ctx.Value(urlKey).(string)), slog.Duration("sinceAccessTime", time.Since(accessTime))) if _, err := os.Stat(path.Join(repoPath, ".git")); os.IsNotExist(err) { // Clone err := runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", url, repoPath) @@ -142,7 +148,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error if isIndexLockError(err) { // index.lock exists, likely a previous git reset got terminated and wasn't cleaned up properly. // We can remove the file and retry the command - logger.Warn("index.lock exists, attempting to remove and retry", slog.String("url", ctx.Value("url").(string))) + logger.Warn("index.lock exists, attempting to remove and retry", slog.String("url", ctx.Value(urlKey).(string))) indexLockPath := filepath.Join(repoPath, ".git", "index.lock") if err := os.Remove(indexLockPath); err != nil { return nil, fmt.Errorf("failed to remove index.lock in %s: %w", repoPath, err) @@ -156,7 +162,7 @@ func fetchBlob(ctx context.Context, url string, forceUpdate bool) ([]byte, error } } - logger.Info("Archiving git blob", slog.String("url", ctx.Value("url").(string))) + logger.Info("Archiving git blob", slog.String("url", ctx.Value(urlKey).(string))) // Archive // tar --zstd -cf -C "/" . // using -C to archive the relative path so it unzips nicely @@ -264,7 +270,7 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { forceUpdate := r.URL.Query().Get("force-update") == "true" ctx := r.Context() - ctx = context.WithValue(ctx, "url", url) + ctx = context.WithValue(ctx, urlKey, url) logger.Info("Received request", slog.String("url", url), slog.Bool("forceUpdate", forceUpdate), slog.String("remoteAddr", r.RemoteAddr)) // If request came from a local ip, don't do the check @@ -288,7 +294,7 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { }) if err != nil { - logger.Error("Error fetching/archiving blob", slog.String("url", ctx.Value("url").(string)), slog.Any("error", err)) + logger.Error("Error fetching/archiving blob", slog.String("url", ctx.Value(urlKey).(string)), slog.Any("error", err)) if isAuthError(err) { http.Error(w, fmt.Sprintf("Error fetching blob: %v", err), http.StatusForbidden) return @@ -308,5 +314,5 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { return } - logger.Info("Request completed successfully", slog.String("url", ctx.Value("url").(string))) + logger.Info("Request completed successfully", slog.String("url", ctx.Value(urlKey).(string))) } From ae61a794f41f427093993bd2e5e0b1c51b4686c7 Mon Sep 17 00:00:00 2001 From: Joey L Date: Fri, 13 Feb 2026 04:47:34 +0000 Subject: [PATCH 5/5] fix lint --- go/cmd/gitter/gitter.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/go/cmd/gitter/gitter.go b/go/cmd/gitter/gitter.go index 142cdbf2f8e..5ee05068484 100644 --- a/go/cmd/gitter/gitter.go +++ b/go/cmd/gitter/gitter.go @@ -269,8 +269,7 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { } forceUpdate := r.URL.Query().Get("force-update") == "true" - ctx := r.Context() - ctx = context.WithValue(ctx, urlKey, url) + ctx := context.WithValue(r.Context(), urlKey, url) logger.Info("Received request", slog.String("url", url), slog.Bool("forceUpdate", forceUpdate), slog.String("remoteAddr", r.RemoteAddr)) // If request came from a local ip, don't do the check @@ -288,7 +287,6 @@ func gitHandler(w http.ResponseWriter, r *http.Request) { // That is highly unlikely in our use case, as importer only queries // the repo once, and always with force update. // This is a tradeoff for simplicity to avoid having to setup locks per repo. - //nolint:contextcheck // I can't change singleflight's interface fileData, err, _ := g.Do(url, func() (any, error) { return fetchBlob(ctx, url, forceUpdate) })