detached 445b2ff

fix(recovery): reject ambiguous version errors

commit
445b2ffc83e24538ee009d7cfd291e1dab706638
author
brice · · 2026-07-27 13:31 UTC
parent
65de473
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
3 files · +87 -1
Unified
UnifiedSplit
M internal/usecase/daemon/coderabbit_regression_test.go +25 -0
@@ -17,6 +17,31 @@ import (
1717 snapcodec "github.com/bnema/vev/internal/usecase/snapshot"
1818 )
1919
20+func TestRestoreAmbiguousBadVersionLoadFailurePreservesCheckpoint(t *testing.T) {
21+ record := durableRecoveryRecord(0)
22+ repository := &durableRecoveryRepository{
23+ errors: map[string]error{record.Name: errors.Join(snapcodec.ErrBadVersion, syscall.ENOSPC)},
24+ loads: make(map[string]int),
25+ repairs: make(map[string]domain.CheckpointRef),
26+ }
27+ d, catalogue := newDurableRecoveryDaemon(t, []domain.CatalogueRecord{record}, repository)
28+
29+ d.restoreCatalogue(context.Background(), mustDurableRecords(t, catalogue))
30+
31+ persisted, ok, err := catalogue.Record(record.Name)
32+ require.NoError(t, err)
33+ require.True(t, ok)
34+ require.Equal(t, record.IncarnationID, persisted.IncarnationID)
35+ require.Equal(t, record.Committed, persisted.Committed)
36+ require.Equal(t, "checkpoint load failed", persisted.DegradedReason)
37+ require.Empty(t, repository.deleted)
38+ d.mu.Lock()
39+ entry := d.stopped[record.Name]
40+ d.mu.Unlock()
41+ require.Equal(t, ports.SessionBroken, entry.state)
42+ require.Equal(t, persisted, entry.record)
43+}
44+
2045 func TestRestoreTransientLoadFailureBecomesDiscardable(t *testing.T) {
2146 record := durableRecoveryRecord(0)
2247 repository := &durableRecoveryRepository{
@@ -17,6 +17,31 @@ import (
17 snapcodec "github.com/bnema/vev/internal/usecase/snapshot"17 snapcodec "github.com/bnema/vev/internal/usecase/snapshot"
18 )18 )
19 19
20+func TestRestoreAmbiguousBadVersionLoadFailurePreservesCheckpoint(t *testing.T) {
21+ record := durableRecoveryRecord(0)
22+ repository := &durableRecoveryRepository{
23+ errors: map[string]error{record.Name: errors.Join(snapcodec.ErrBadVersion, syscall.ENOSPC)},
24+ loads: make(map[string]int),
25+ repairs: make(map[string]domain.CheckpointRef),
26+ }
27+ d, catalogue := newDurableRecoveryDaemon(t, []domain.CatalogueRecord{record}, repository)
28+
29+ d.restoreCatalogue(context.Background(), mustDurableRecords(t, catalogue))
30+
31+ persisted, ok, err := catalogue.Record(record.Name)
32+ require.NoError(t, err)
33+ require.True(t, ok)
34+ require.Equal(t, record.IncarnationID, persisted.IncarnationID)
35+ require.Equal(t, record.Committed, persisted.Committed)
36+ require.Equal(t, "checkpoint load failed", persisted.DegradedReason)
37+ require.Empty(t, repository.deleted)
38+ d.mu.Lock()
39+ entry := d.stopped[record.Name]
40+ d.mu.Unlock()
41+ require.Equal(t, ports.SessionBroken, entry.state)
42+ require.Equal(t, persisted, entry.record)
43+}
44+
20 func TestRestoreTransientLoadFailureBecomesDiscardable(t *testing.T) {45 func TestRestoreTransientLoadFailureBecomesDiscardable(t *testing.T) {
21 record := durableRecoveryRecord(0)46 record := durableRecoveryRecord(0)
22 repository := &durableRecoveryRepository{47 repository := &durableRecoveryRepository{
M internal/usecase/daemon/durable_recovery_test.go +35 -0
@@ -538,6 +538,41 @@ type temporaryRestoreError struct{}
538538 func (temporaryRestoreError) Error() string { return "temporary restore failure" }
539539 func (temporaryRestoreError) Temporary() bool { return true }
540540
541+type badVersionLookalikeError struct{}
542+
543+func (badVersionLookalikeError) Error() string { return "bad version lookalike" }
544+func (badVersionLookalikeError) Is(target error) bool { return target == snapcodec.ErrBadVersion }
545+
546+type cyclicUnwrapError struct{}
547+
548+func (*cyclicUnwrapError) Error() string { return "cyclic unwrap" }
549+func (e *cyclicUnwrapError) Unwrap() error {
550+ return e
551+}
552+
553+func TestUnambiguousBadVersionErrorClassification(t *testing.T) {
554+ tests := []struct {
555+ name string
556+ err error
557+ incompatible bool
558+ }{
559+ {name: "nil"},
560+ {name: "exact sentinel", err: snapcodec.ErrBadVersion, incompatible: true},
561+ {name: "single cause chain", err: fmt.Errorf("load checkpoint: %w", fmt.Errorf("decode manifest: %w", snapcodec.ErrBadVersion)), incompatible: true},
562+ {name: "single entry multi cause", err: errors.Join(snapcodec.ErrBadVersion)},
563+ {name: "joined retryable branch", err: errors.Join(snapcodec.ErrBadVersion, syscall.ENOSPC)},
564+ {name: "custom Is lookalike", err: badVersionLookalikeError{}},
565+ {name: "wrapped custom Is lookalike", err: fmt.Errorf("load checkpoint: %w", badVersionLookalikeError{})},
566+ {name: "chain ending elsewhere", err: fmt.Errorf("load checkpoint: %w", errors.New("corrupt checkpoint"))},
567+ {name: "cyclic custom chain", err: &cyclicUnwrapError{}},
568+ }
569+ for _, tt := range tests {
570+ t.Run(tt.name, func(t *testing.T) {
571+ require.Equal(t, tt.incompatible, isUnambiguousBadVersionError(tt.err))
572+ })
573+ }
574+}
575+
541576 func TestRetryableRestoreLoadErrorClassification(t *testing.T) {
542577 tests := []struct {
543578 name string
@@ -538,6 +538,41 @@ type temporaryRestoreError struct{}
538 func (temporaryRestoreError) Error() string { return "temporary restore failure" }538 func (temporaryRestoreError) Error() string { return "temporary restore failure" }
539 func (temporaryRestoreError) Temporary() bool { return true }539 func (temporaryRestoreError) Temporary() bool { return true }
540 540
541+type badVersionLookalikeError struct{}
542+
543+func (badVersionLookalikeError) Error() string { return "bad version lookalike" }
544+func (badVersionLookalikeError) Is(target error) bool { return target == snapcodec.ErrBadVersion }
545+
546+type cyclicUnwrapError struct{}
547+
548+func (*cyclicUnwrapError) Error() string { return "cyclic unwrap" }
549+func (e *cyclicUnwrapError) Unwrap() error {
550+ return e
551+}
552+
553+func TestUnambiguousBadVersionErrorClassification(t *testing.T) {
554+ tests := []struct {
555+ name string
556+ err error
557+ incompatible bool
558+ }{
559+ {name: "nil"},
560+ {name: "exact sentinel", err: snapcodec.ErrBadVersion, incompatible: true},
561+ {name: "single cause chain", err: fmt.Errorf("load checkpoint: %w", fmt.Errorf("decode manifest: %w", snapcodec.ErrBadVersion)), incompatible: true},
562+ {name: "single entry multi cause", err: errors.Join(snapcodec.ErrBadVersion)},
563+ {name: "joined retryable branch", err: errors.Join(snapcodec.ErrBadVersion, syscall.ENOSPC)},
564+ {name: "custom Is lookalike", err: badVersionLookalikeError{}},
565+ {name: "wrapped custom Is lookalike", err: fmt.Errorf("load checkpoint: %w", badVersionLookalikeError{})},
566+ {name: "chain ending elsewhere", err: fmt.Errorf("load checkpoint: %w", errors.New("corrupt checkpoint"))},
567+ {name: "cyclic custom chain", err: &cyclicUnwrapError{}},
568+ }
569+ for _, tt := range tests {
570+ t.Run(tt.name, func(t *testing.T) {
571+ require.Equal(t, tt.incompatible, isUnambiguousBadVersionError(tt.err))
572+ })
573+ }
574+}
575+
541 func TestRetryableRestoreLoadErrorClassification(t *testing.T) {576 func TestRetryableRestoreLoadErrorClassification(t *testing.T) {
542 tests := []struct {577 tests := []struct {
543 name string578 name string
M internal/usecase/daemon/snapshot_restore_incremental.go +27 -1
@@ -6,6 +6,7 @@ import (
66 "errors"
77 "fmt"
88 "net"
9+ "reflect"
910 "sync"
1011 "syscall"
1112
@@ -178,7 +179,7 @@ func (d *Daemon) restoreRecord(ctx context.Context, record domain.CatalogueRecor
178179 if ctxErr := ctx.Err(); ctxErr != nil {
179180 return ctxErr
180181 }
181- if errors.Is(err, snapcodec.ErrBadVersion) {
182+ if isUnambiguousBadVersionError(err) {
182183 return d.resetIncompatibleCheckpoint(ctx, record, selected)
183184 }
184185 if isRetryableRestoreLoadError(err) {
@@ -257,6 +258,31 @@ func (d *Daemon) resetIncompatibleCheckpoint(ctx context.Context, record domain.
257258 return nil
258259 }
259260
261+func isUnambiguousBadVersionError(err error) bool {
262+ seen := make(map[error]struct{})
263+ for err != nil {
264+ if !reflect.TypeOf(err).Comparable() {
265+ return false
266+ }
267+ if _, duplicate := seen[err]; duplicate {
268+ return false
269+ }
270+ seen[err] = struct{}{}
271+ if err == snapcodec.ErrBadVersion {
272+ return true
273+ }
274+ if _, ambiguous := err.(interface{ Unwrap() []error }); ambiguous {
275+ return false
276+ }
277+ unwrapper, ok := err.(interface{ Unwrap() error })
278+ if !ok {
279+ return false
280+ }
281+ err = unwrapper.Unwrap()
282+ }
283+ return false
284+}
285+
260286 func isRetryableRestoreLoadError(err error) bool {
261287 if err == nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
262288 return false
@@ -6,6 +6,7 @@ import (
6 "errors"6 "errors"
7 "fmt"7 "fmt"
8 "net"8 "net"
9+ "reflect"
9 "sync"10 "sync"
10 "syscall"11 "syscall"
11 12
@@ -178,7 +179,7 @@ func (d *Daemon) restoreRecord(ctx context.Context, record domain.CatalogueRecor
178 if ctxErr := ctx.Err(); ctxErr != nil {179 if ctxErr := ctx.Err(); ctxErr != nil {
179 return ctxErr180 return ctxErr
180 }181 }
181- if errors.Is(err, snapcodec.ErrBadVersion) {182+ if isUnambiguousBadVersionError(err) {
182 return d.resetIncompatibleCheckpoint(ctx, record, selected)183 return d.resetIncompatibleCheckpoint(ctx, record, selected)
183 }184 }
184 if isRetryableRestoreLoadError(err) {185 if isRetryableRestoreLoadError(err) {
@@ -257,6 +258,31 @@ func (d *Daemon) resetIncompatibleCheckpoint(ctx context.Context, record domain.
257 return nil258 return nil
258 }259 }
259 260
261+func isUnambiguousBadVersionError(err error) bool {
262+ seen := make(map[error]struct{})
263+ for err != nil {
264+ if !reflect.TypeOf(err).Comparable() {
265+ return false
266+ }
267+ if _, duplicate := seen[err]; duplicate {
268+ return false
269+ }
270+ seen[err] = struct{}{}
271+ if err == snapcodec.ErrBadVersion {
272+ return true
273+ }
274+ if _, ambiguous := err.(interface{ Unwrap() []error }); ambiguous {
275+ return false
276+ }
277+ unwrapper, ok := err.(interface{ Unwrap() error })
278+ if !ok {
279+ return false
280+ }
281+ err = unwrapper.Unwrap()
282+ }
283+ return false
284+}
285+
260 func isRetryableRestoreLoadError(err error) bool {286 func isRetryableRestoreLoadError(err error) bool {
261 if err == nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {287 if err == nil || errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
262 return false288 return false

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close