Skip to content

Commit de2499b

Browse files
authored
Merge pull request #2466 from dgageot/dont-retry
Don't retry SQLITE_CANTOPEN (14) errors
2 parents 15945f4 + 1c0996d commit de2499b

2 files changed

Lines changed: 15 additions & 0 deletions

File tree

pkg/session/store.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,15 @@ func NewSQLiteSessionStore(path string) (Store, error) {
374374
return nil, err
375375
}
376376

377+
// Don't attempt recovery if we couldn't even open/create the database file
378+
// (e.g., permission denied, read-only filesystem, missing directory).
379+
// The backup+retry dance can't fix a filesystem-level problem, and would just
380+
// wrap the real error in a confusing "migration failed even after database reset"
381+
// message.
382+
if sqliteutil.IsCantOpenError(err) {
383+
return nil, err
384+
}
385+
377386
// If migrations failed, try to recover by backing up the database and starting fresh
378387
slog.Warn("Failed to open session store, attempting recovery", "error", err)
379388

pkg/session/store_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,12 @@ func TestNewSQLiteSessionStore_DirectoryNotWritable(t *testing.T) {
358358

359359
assert.Contains(t, err.Error(), "cannot create database")
360360
assert.Contains(t, err.Error(), "permission denied or file cannot be created")
361+
362+
// We should surface the real "cannot create database" error directly instead of
363+
// running the backup+retry recovery path (which cannot fix a filesystem-level
364+
// problem and would only wrap the real error in a confusing "migration failed"
365+
// message).
366+
assert.NotContains(t, err.Error(), "migration failed")
361367
}
362368

363369
func TestUpdateSession_LazyCreation(t *testing.T) {

0 commit comments

Comments
 (0)