Skip to content

Commit f1cf81e

Browse files
leventovleventov
authored
Include file path in errors from PartialDriver.ReadUp() and ReadDown() (#421)
* Include file path in errors from PartialDriver.ReadUp() and ReadDown() * Use errors.Is(...) instead of os.Is...() in migrate.go: golang/go#41122
1 parent c6ebff6 commit f1cf81e

File tree

5 files changed

+45
-23
lines changed

5 files changed

+45
-23
lines changed

Diff for: ‎migrate.go

+12-11
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ func (m *Migrate) read(from int, to int, ret chan<- interface{}) {
487487
}
488488

489489
prev, err := m.sourceDrv.Prev(suint(from))
490-
if os.IsNotExist(err) && to == -1 {
490+
if errors.Is(err, os.ErrNotExist) && to == -1 {
491491
// apply nil migration
492492
migr, err := m.newMigration(suint(from), -1)
493493
if err != nil {
@@ -580,7 +580,7 @@ func (m *Migrate) readUp(from int, limit int, ret chan<- interface{}) {
580580

581581
// apply next migration
582582
next, err := m.sourceDrv.Next(suint(from))
583-
if os.IsNotExist(err) {
583+
if errors.Is(err, os.ErrNotExist) {
584584
// no limit, but no migrations applied?
585585
if limit == -1 && count == 0 {
586586
ret <- ErrNoChange
@@ -666,7 +666,7 @@ func (m *Migrate) readDown(from int, limit int, ret chan<- interface{}) {
666666
}
667667

668668
prev, err := m.sourceDrv.Prev(suint(from))
669-
if os.IsNotExist(err) {
669+
if errors.Is(err, os.ErrNotExist) {
670670
// no limit or haven't reached limit, apply "first" migration
671671
if limit == -1 || limit-count > 0 {
672672
firstVersion, err := m.sourceDrv.First()
@@ -785,9 +785,9 @@ func (m *Migrate) versionExists(version uint) (result error) {
785785
}
786786
}()
787787
}
788-
if os.IsExist(err) {
788+
if errors.Is(err, os.ErrExist) {
789789
return nil
790-
} else if !os.IsNotExist(err) {
790+
} else if !errors.Is(err, os.ErrNotExist) {
791791
return err
792792
}
793793

@@ -800,14 +800,15 @@ func (m *Migrate) versionExists(version uint) (result error) {
800800
}
801801
}()
802802
}
803-
if os.IsExist(err) {
803+
if errors.Is(err, os.ErrExist) {
804804
return nil
805-
} else if !os.IsNotExist(err) {
805+
} else if !errors.Is(err, os.ErrNotExist) {
806806
return err
807807
}
808808

809-
m.logErr(fmt.Errorf("no migration found for version %d", version))
810-
return os.ErrNotExist
809+
err = fmt.Errorf("no migration found for version %d: %w", version, err)
810+
m.logErr(err)
811+
return err
811812
}
812813

813814
// stop returns true if no more migrations should be run against the database
@@ -835,7 +836,7 @@ func (m *Migrate) newMigration(version uint, targetVersion int) (*Migration, err
835836

836837
if targetVersion >= int(version) {
837838
r, identifier, err := m.sourceDrv.ReadUp(version)
838-
if os.IsNotExist(err) {
839+
if errors.Is(err, os.ErrNotExist) {
839840
// create "empty" migration
840841
migr, err = NewMigration(nil, "", version, targetVersion)
841842
if err != nil {
@@ -855,7 +856,7 @@ func (m *Migrate) newMigration(version uint, targetVersion int) (*Migration, err
855856

856857
} else {
857858
r, identifier, err := m.sourceDrv.ReadDown(version)
858-
if os.IsNotExist(err) {
859+
if errors.Is(err, os.ErrNotExist) {
859860
// create "empty" migration
860861
migr, err = NewMigration(nil, "", version, targetVersion)
861862
if err != nil {

Diff for: ‎migrate_test.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package migrate
33
import (
44
"bytes"
55
"database/sql"
6+
"errors"
67
"io/ioutil"
78
"log"
89
"os"
@@ -468,7 +469,7 @@ func TestMigrate(t *testing.T) {
468469

469470
for i, v := range tt {
470471
err := m.Migrate(v.version)
471-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
472+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
472473
(v.expectErr != os.ErrNotExist && err != v.expectErr) {
473474
t.Errorf("expected err %v, got %v, in %v", v.expectErr, err, i)
474475

@@ -731,7 +732,7 @@ func TestSteps(t *testing.T) {
731732

732733
for i, v := range tt {
733734
err := m.Steps(v.steps)
734-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
735+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
735736
(v.expectErr != os.ErrNotExist && err != v.expectErr) {
736737
t.Errorf("expected err %v, got %v, in %v", v.expectErr, err, i)
737738

@@ -1143,7 +1144,7 @@ func TestRead(t *testing.T) {
11431144
go m.read(v.from, v.to, ret)
11441145
migrations, err := migrationsFromChannel(ret)
11451146

1146-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
1147+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
11471148
(v.expectErr != os.ErrNotExist && v.expectErr != err) {
11481149
t.Errorf("expected %v, got %v, in %v", v.expectErr, err, i)
11491150
t.Logf("%v, in %v", migrations, i)
@@ -1220,7 +1221,7 @@ func TestReadUp(t *testing.T) {
12201221
go m.readUp(v.from, v.limit, ret)
12211222
migrations, err := migrationsFromChannel(ret)
12221223

1223-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
1224+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
12241225
(v.expectErr != os.ErrNotExist && v.expectErr != err) {
12251226
t.Errorf("expected %v, got %v, in %v", v.expectErr, err, i)
12261227
t.Logf("%v, in %v", migrations, i)
@@ -1297,7 +1298,7 @@ func TestReadDown(t *testing.T) {
12971298
go m.readDown(v.from, v.limit, ret)
12981299
migrations, err := migrationsFromChannel(ret)
12991300

1300-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
1301+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
13011302
(v.expectErr != os.ErrNotExist && v.expectErr != err) {
13021303
t.Errorf("expected %v, got %v, in %v", v.expectErr, err, i)
13031304
t.Logf("%v, in %v", migrations, i)

Diff for: ‎source/file/file_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package file
22

33
import (
4+
"errors"
45
"fmt"
56
"io/ioutil"
67
"os"
@@ -235,7 +236,7 @@ func BenchmarkNext(b *testing.B) {
235236
b.ResetTimer()
236237
v, err := d.First()
237238
for n := 0; n < b.N; n++ {
238-
for !os.IsNotExist(err) {
239+
for !errors.Is(err, os.ErrNotExist) {
239240
v, err = d.Next(v)
240241
}
241242
}

Diff for: ‎source/httpfs/partial_driver.go

+20-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package httpfs
22

33
import (
4+
"errors"
45
"io"
56
"net/http"
67
"os"
@@ -108,7 +109,7 @@ func (p *PartialDriver) Next(version uint) (nextVersion uint, err error) {
108109
// ReadUp is part of source.Driver interface implementation.
109110
func (p *PartialDriver) ReadUp(version uint) (r io.ReadCloser, identifier string, err error) {
110111
if m, ok := p.migrations.Up(version); ok {
111-
body, err := p.fs.Open(path.Join(p.path, m.Raw))
112+
body, err := p.open(path.Join(p.path, m.Raw))
112113
if err != nil {
113114
return nil, "", err
114115
}
@@ -124,7 +125,7 @@ func (p *PartialDriver) ReadUp(version uint) (r io.ReadCloser, identifier string
124125
// ReadDown is part of source.Driver interface implementation.
125126
func (p *PartialDriver) ReadDown(version uint) (r io.ReadCloser, identifier string, err error) {
126127
if m, ok := p.migrations.Down(version); ok {
127-
body, err := p.fs.Open(path.Join(p.path, m.Raw))
128+
body, err := p.open(path.Join(p.path, m.Raw))
128129
if err != nil {
129130
return nil, "", err
130131
}
@@ -136,3 +137,20 @@ func (p *PartialDriver) ReadDown(version uint) (r io.ReadCloser, identifier stri
136137
Err: os.ErrNotExist,
137138
}
138139
}
140+
141+
func (p *PartialDriver) open(path string) (http.File, error) {
142+
f, err := p.fs.Open(path)
143+
if err == nil {
144+
return f, nil
145+
}
146+
// Some non-standard file systems may return errors that don't include the path, that
147+
// makes debugging harder.
148+
if !errors.As(err, new(*os.PathError)) {
149+
err = &os.PathError{
150+
Op: "open",
151+
Path: path,
152+
Err: err,
153+
}
154+
}
155+
return nil, err
156+
}

Diff for: ‎source/testing/testing.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package testing
55

66
import (
7+
"errors"
78
"os"
89
"testing"
910

@@ -56,7 +57,7 @@ func TestPrev(t *testing.T, d source.Driver) {
5657

5758
for i, v := range tt {
5859
pv, err := d.Prev(v.version)
59-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) && v.expectErr != err {
60+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) && v.expectErr != err {
6061
t.Errorf("Prev: expected %v, got %v, in %v", v.expectErr, err, i)
6162
}
6263
if err == nil && v.expectPrevVersion != pv {
@@ -85,7 +86,7 @@ func TestNext(t *testing.T, d source.Driver) {
8586

8687
for i, v := range tt {
8788
nv, err := d.Next(v.version)
88-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) && v.expectErr != err {
89+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) && v.expectErr != err {
8990
t.Errorf("Next: expected %v, got %v, in %v", v.expectErr, err, i)
9091
}
9192
if err == nil && v.expectNextVersion != nv {
@@ -113,7 +114,7 @@ func TestReadUp(t *testing.T, d source.Driver) {
113114

114115
for i, v := range tt {
115116
up, identifier, err := d.ReadUp(v.version)
116-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
117+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
117118
(v.expectErr != os.ErrNotExist && err != v.expectErr) {
118119
t.Errorf("expected %v, got %v, in %v", v.expectErr, err, i)
119120

@@ -150,7 +151,7 @@ func TestReadDown(t *testing.T, d source.Driver) {
150151

151152
for i, v := range tt {
152153
down, identifier, err := d.ReadDown(v.version)
153-
if (v.expectErr == os.ErrNotExist && !os.IsNotExist(err)) ||
154+
if (v.expectErr == os.ErrNotExist && !errors.Is(err, os.ErrNotExist)) ||
154155
(v.expectErr != os.ErrNotExist && err != v.expectErr) {
155156
t.Errorf("expected %v, got %v, in %v", v.expectErr, err, i)
156157

0 commit comments

Comments
 (0)