Skip to content

Commit 77dbfa8

Browse files
committed
handle rollback error
1 parent 9f6c597 commit 77dbfa8

File tree

5 files changed

+128
-8
lines changed

5 files changed

+128
-8
lines changed

forbidden_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//go:build windows
2-
// +build windows
32

43
package screw_test
54

screw_darwin.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//go:build darwin
2-
// +build darwin
32

43
package screw
54

@@ -28,12 +27,17 @@ char *GetCanonicalPath(char *cInputPath) {
2827
import "C"
2928

3029
import (
30+
"errors"
3131
"fmt"
3232
"os"
3333
"path/filepath"
3434
"unsafe"
3535
)
3636

37+
// osRename is a test seam so darwin-only tests can deterministically exercise
38+
// fallback and rollback branches in doRename without relying on filesystem quirks.
39+
var osRename = os.Rename
40+
3741
func sneakyLog(line string) {
3842
// nothing
3943
}
@@ -54,7 +58,7 @@ func TrueBaseName(path string) string {
5458
}
5559

5660
func doRename(oldpath, newpath string) error {
57-
err := os.Rename(oldpath, newpath)
61+
err := osRename(oldpath, newpath)
5862
if err != nil {
5963
// on macOS, renaming "foo" to "FOO" is fine,
6064
// but renaming "foo/" to "FOO/" fails with "file exists"
@@ -63,15 +67,19 @@ func doRename(oldpath, newpath string) error {
6367

6468
tmppath := fmt.Sprintf("%s__rename_pid%d", oldpath, os.Getpid())
6569

66-
err = os.Rename(oldpath, tmppath)
70+
err = osRename(oldpath, tmppath)
6771
if err != nil {
6872
return originalErr
6973
}
70-
err = os.Rename(tmppath, newpath)
74+
err = osRename(tmppath, newpath)
7175
if err != nil {
7276
// attempt to rollback, ignore returned error,
7377
// because what else can we do at this point?
74-
_ = os.Rename(tmppath, oldpath)
78+
rollbackErr := osRename(tmppath, oldpath)
79+
if rollbackErr != nil {
80+
return errors.Join(err, rollbackErr)
81+
}
82+
return err
7583
}
7684
// two-stage rename worked!
7785
return nil

screw_darwin_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
//go:build darwin
2+
3+
package screw
4+
5+
import (
6+
"errors"
7+
"os"
8+
"strings"
9+
"testing"
10+
)
11+
12+
func stubRename(t *testing.T, fn func(oldpath, newpath string) error) {
13+
t.Helper()
14+
15+
previous := osRename
16+
osRename = fn
17+
t.Cleanup(func() {
18+
osRename = previous
19+
})
20+
}
21+
22+
func TestDoRename_DarwinSecondStageFailure(t *testing.T) {
23+
oldpath := "/tmp/old"
24+
newpath := "/tmp/new"
25+
secondStageErr := errors.New("second stage failed")
26+
27+
var calls int
28+
var tmppath string
29+
30+
stubRename(t, func(old, new string) error {
31+
calls++
32+
switch calls {
33+
case 1:
34+
if old != oldpath || new != newpath {
35+
t.Fatalf("unexpected first rename call: %q => %q", old, new)
36+
}
37+
return os.ErrExist
38+
case 2:
39+
if old != oldpath {
40+
t.Fatalf("unexpected second rename source: %q", old)
41+
}
42+
if !strings.HasPrefix(new, oldpath+"__rename_pid") {
43+
t.Fatalf("unexpected second rename target: %q", new)
44+
}
45+
tmppath = new
46+
return nil
47+
case 3:
48+
if old != tmppath || new != newpath {
49+
t.Fatalf("unexpected third rename call: %q => %q", old, new)
50+
}
51+
return secondStageErr
52+
case 4:
53+
if old != tmppath || new != oldpath {
54+
t.Fatalf("unexpected rollback call: %q => %q", old, new)
55+
}
56+
return nil
57+
default:
58+
t.Fatalf("unexpected extra rename call: %d", calls)
59+
return nil
60+
}
61+
})
62+
63+
err := doRename(oldpath, newpath)
64+
if !errors.Is(err, secondStageErr) {
65+
t.Fatalf("expected second stage error, got: %v", err)
66+
}
67+
if calls != 4 {
68+
t.Fatalf("expected 4 rename calls, got: %d", calls)
69+
}
70+
}
71+
72+
func TestDoRename_DarwinSecondStageAndRollbackFailure(t *testing.T) {
73+
oldpath := "/tmp/old"
74+
newpath := "/tmp/new"
75+
secondStageErr := errors.New("second stage failed")
76+
rollbackErr := errors.New("rollback failed")
77+
78+
var calls int
79+
var tmppath string
80+
81+
stubRename(t, func(old, new string) error {
82+
calls++
83+
switch calls {
84+
case 1:
85+
return os.ErrExist
86+
case 2:
87+
tmppath = new
88+
return nil
89+
case 3:
90+
if old != tmppath || new != newpath {
91+
t.Fatalf("unexpected third rename call: %q => %q", old, new)
92+
}
93+
return secondStageErr
94+
case 4:
95+
if old != tmppath || new != oldpath {
96+
t.Fatalf("unexpected rollback call: %q => %q", old, new)
97+
}
98+
return rollbackErr
99+
default:
100+
t.Fatalf("unexpected extra rename call: %d", calls)
101+
return nil
102+
}
103+
})
104+
105+
err := doRename(oldpath, newpath)
106+
if !errors.Is(err, secondStageErr) {
107+
t.Fatalf("expected second stage error in chain, got: %v", err)
108+
}
109+
if !errors.Is(err, rollbackErr) {
110+
t.Fatalf("expected rollback error in chain, got: %v", err)
111+
}
112+
if calls != 4 {
113+
t.Fatalf("expected 4 rename calls, got: %d", calls)
114+
}
115+
}

screw_linux.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//go:build linux
2-
// +build linux
32

43
package screw
54

screw_windows.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
//go:build windows
2-
// +build windows
32

43
package screw
54

0 commit comments

Comments
 (0)