Skip to content

Commit 369763d

Browse files
Copilotneongreen
andcommitted
Address code review feedback: fix return type and exponential backoff formula
Co-authored-by: neongreen <[email protected]>
1 parent e732464 commit 369763d

3 files changed

Lines changed: 13 additions & 13 deletions

File tree

dissect/pkg/gopls/add_import.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func AddImport(goplsPath string, filePath string, importPath string, moduleRoot
3131
var stderr bytes.Buffer
3232
cmd.Stderr = &stderr
3333

34-
if _, err := runWithTextFileBusyRetry(cmd); err != nil {
34+
if err := runWithTextFileBusyRetry(cmd); err != nil {
3535
return fmt.Errorf("error executing gopls: %w\n%s", err, stderr.String())
3636
}
3737

dissect/pkg/gopls/extract_to_new_file.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func ExtractToNewFile(goplsPath string, filePath string, funcName string, module
5555
var stderr bytes.Buffer
5656
cmd.Stderr = &stderr
5757

58-
if _, err := runWithTextFileBusyRetry(cmd); err != nil {
58+
if err := runWithTextFileBusyRetry(cmd); err != nil {
5959
return "", fmt.Errorf("error executing gopls: %w\n%s", err, stderr.String())
6060
}
6161

dissect/pkg/gopls/rename.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func Rename(goplsPath string, filePath string, oldName string, newName string, m
4040
cmd.Stdout = &combinedOutput
4141
cmd.Stderr = &combinedOutput
4242

43-
_, err = runWithTextFileBusyRetry(cmd)
43+
err = runWithTextFileBusyRetry(cmd)
4444
if err != nil {
4545
return fmt.Errorf("gopls rename failed: %w\nOutput: %s", err, combinedOutput.String())
4646
}
@@ -119,36 +119,36 @@ func findSymbolOffset(filePath string, symbolName string) (int, error) {
119119
// This error can occur when a binary was just installed by "go install" and the OS hasn't
120120
// fully released the file handle yet. The function retries up to 3 times with exponential backoff.
121121
// Note: This function uses cmd.Run(), so caller should capture output via cmd.Stdout/Stderr if needed.
122-
func runWithTextFileBusyRetry(cmd *exec.Cmd) ([]byte, error) {
122+
func runWithTextFileBusyRetry(cmd *exec.Cmd) error {
123123
const maxRetries = 3
124124
var lastErr error
125125

126126
for attempt := 0; attempt < maxRetries; attempt++ {
127-
// Create a new command for each attempt since exec.Cmd can only be used once
127+
// Create a new command for each retry since exec.Cmd can only be used once
128128
if attempt > 0 {
129+
// Wait before retrying with exponential backoff (100ms, 200ms, 400ms)
130+
waitTime := time.Duration(100*(1<<(attempt-1))) * time.Millisecond
131+
slog.Debug("Retrying command after text file busy error", "attempt", attempt+1, "wait", waitTime)
132+
time.Sleep(waitTime)
133+
129134
newCmd := exec.Command(cmd.Path, cmd.Args[1:]...)
130135
newCmd.Dir = cmd.Dir
131136
newCmd.Env = cmd.Env
132137
newCmd.Stdout = cmd.Stdout
133138
newCmd.Stderr = cmd.Stderr
134139
cmd = newCmd
135-
136-
// Wait before retrying with exponential backoff
137-
waitTime := time.Duration(50*(1<<attempt)) * time.Millisecond
138-
slog.Debug("Retrying command after text file busy error", "attempt", attempt+1, "wait", waitTime)
139-
time.Sleep(waitTime)
140140
}
141141

142142
lastErr = cmd.Run()
143143
if lastErr == nil {
144-
return nil, nil
144+
return nil
145145
}
146146

147147
// Check if this is a "text file busy" error
148148
if !strings.Contains(lastErr.Error(), "text file busy") {
149-
return nil, lastErr
149+
return lastErr
150150
}
151151
}
152152

153-
return nil, lastErr
153+
return lastErr
154154
}

0 commit comments

Comments
 (0)