Centralize error handling patterns in storage layer (bd-bwk2)

Created internal/storage/sqlite/errors.go with:
- Sentinel errors: ErrNotFound, ErrInvalidID, ErrConflict, ErrCycle
- wrapDBError helpers that auto-convert sql.ErrNoRows to ErrNotFound
- Type-safe error checking with errors.Is() compatibility

Updated error handling across storage layer:
- dirty.go: Added context to error returns, converted sql.ErrNoRows checks
- util.go: Updated withTx to use wrapDBError
- batch_ops.go: Added context wrapping to batch operations
- dependencies.go: Wrapped errors from markIssuesDirtyTx calls
- ids.go: Added error wrapping for ID validation

Also restored sqlite.go that was accidentally deleted in previous commit.

All tests pass. Provides consistent error wrapping with operation context
for better debugging.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-11-20 19:17:57 -05:00
parent 221a7d4db6
commit 3b2cac4d8f
8 changed files with 1846 additions and 157 deletions

File diff suppressed because one or more lines are too long

View File

@@ -46,7 +46,7 @@ func (s *SQLiteStorage) generateBatchIDs(ctx context.Context, conn *sql.Conn, is
// Generate or validate IDs for all issues // Generate or validate IDs for all issues
if err := EnsureIDs(ctx, conn, prefix, issues, actor, orphanHandling); err != nil { if err := EnsureIDs(ctx, conn, prefix, issues, actor, orphanHandling); err != nil {
return err return wrapDBError("ensure IDs", err)
} }
// Compute content hashes // Compute content hashes
@@ -157,22 +157,22 @@ func (s *SQLiteStorage) CreateIssuesWithOptions(ctx context.Context, issues []*t
// Phase 3: Generate IDs for issues that need them // Phase 3: Generate IDs for issues that need them
if err := s.generateBatchIDs(ctx, conn, issues, actor, orphanHandling); err != nil { if err := s.generateBatchIDs(ctx, conn, issues, actor, orphanHandling); err != nil {
return err return wrapDBError("generate batch IDs", err)
} }
// Phase 4: Bulk insert issues // Phase 4: Bulk insert issues
if err := bulkInsertIssues(ctx, conn, issues); err != nil { if err := bulkInsertIssues(ctx, conn, issues); err != nil {
return err return wrapDBError("bulk insert issues", err)
} }
// Phase 5: Record creation events // Phase 5: Record creation events
if err := bulkRecordEvents(ctx, conn, issues, actor); err != nil { if err := bulkRecordEvents(ctx, conn, issues, actor); err != nil {
return err return wrapDBError("record creation events", err)
} }
// Phase 6: Mark issues dirty for incremental export // Phase 6: Mark issues dirty for incremental export
if err := bulkMarkDirty(ctx, conn, issues); err != nil { if err := bulkMarkDirty(ctx, conn, issues); err != nil {
return err return wrapDBError("mark issues dirty", err)
} }
// Phase 7: Commit transaction // Phase 7: Commit transaction

View File

@@ -147,7 +147,7 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency
// Mark both issues as dirty for incremental export // Mark both issues as dirty for incremental export
// (dependencies are exported with each issue, so both need updating) // (dependencies are exported with each issue, so both need updating)
if err := markIssuesDirtyTx(ctx, tx, []string{dep.IssueID, dep.DependsOnID}); err != nil { if err := markIssuesDirtyTx(ctx, tx, []string{dep.IssueID, dep.DependsOnID}); err != nil {
return err return wrapDBError("mark issues dirty after adding dependency", err)
} }
return nil return nil
@@ -184,7 +184,7 @@ func (s *SQLiteStorage) RemoveDependency(ctx context.Context, issueID, dependsOn
// Mark both issues as dirty for incremental export // Mark both issues as dirty for incremental export
if err := markIssuesDirtyTx(ctx, tx, []string{issueID, dependsOnID}); err != nil { if err := markIssuesDirtyTx(ctx, tx, []string{issueID, dependsOnID}); err != nil {
return err return wrapDBError("mark issues dirty after removing dependency", err)
} }
return nil return nil

View File

@@ -16,7 +16,7 @@ func (s *SQLiteStorage) MarkIssueDirty(ctx context.Context, issueID string) erro
VALUES (?, ?) VALUES (?, ?)
ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at ON CONFLICT (issue_id) DO UPDATE SET marked_at = excluded.marked_at
`, issueID, time.Now()) `, issueID, time.Now())
return err return wrapDBErrorf(err, "mark issue %s dirty", issueID)
} }
// MarkIssuesDirty marks multiple issues as dirty in a single transaction // MarkIssuesDirty marks multiple issues as dirty in a single transaction
@@ -68,7 +68,10 @@ func (s *SQLiteStorage) GetDirtyIssues(ctx context.Context) ([]string, error) {
issueIDs = append(issueIDs, issueID) issueIDs = append(issueIDs, issueID)
} }
return issueIDs, rows.Err() if err := rows.Err(); err != nil {
return nil, wrapDBError("iterate dirty issues", err)
}
return issueIDs, nil
} }
// GetDirtyIssueHash returns the stored content hash for a dirty issue, if it exists // GetDirtyIssueHash returns the stored content hash for a dirty issue, if it exists
@@ -78,11 +81,11 @@ func (s *SQLiteStorage) GetDirtyIssueHash(ctx context.Context, issueID string) (
SELECT content_hash FROM dirty_issues WHERE issue_id = ? SELECT content_hash FROM dirty_issues WHERE issue_id = ?
`, issueID).Scan(&hash) `, issueID).Scan(&hash)
if err == sql.ErrNoRows { if IsNotFound(wrapDBErrorf(err, "get dirty issue hash for %s", issueID)) {
return "", nil // Issue not dirty return "", nil // Issue not dirty
} }
if err != nil { if err != nil {
return "", fmt.Errorf("failed to get dirty issue hash: %w", err) return "", wrapDBErrorf(err, "get dirty issue hash for %s", issueID)
} }
if !hash.Valid { if !hash.Valid {
@@ -133,8 +136,11 @@ func (s *SQLiteStorage) ClearDirtyIssuesByID(ctx context.Context, issueIDs []str
func (s *SQLiteStorage) GetDirtyIssueCount(ctx context.Context) (int, error) { func (s *SQLiteStorage) GetDirtyIssueCount(ctx context.Context) (int, error) {
var count int var count int
err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM dirty_issues`).Scan(&count) err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM dirty_issues`).Scan(&count)
if err != nil && err != sql.ErrNoRows { if IsNotFound(wrapDBError("count dirty issues", err)) {
return 0, fmt.Errorf("failed to count dirty issues: %w", err) return 0, nil
}
if err != nil {
return 0, wrapDBError("count dirty issues", err)
} }
return count, nil return count, nil
} }

View File

@@ -0,0 +1,62 @@
package sqlite
import (
"database/sql"
"errors"
"fmt"
)
// Sentinel errors for common database conditions
var (
// ErrNotFound indicates the requested resource was not found in the database
ErrNotFound = errors.New("not found")
// ErrInvalidID indicates an ID format or validation error
ErrInvalidID = errors.New("invalid ID")
// ErrConflict indicates a unique constraint violation or conflicting state
ErrConflict = errors.New("conflict")
// ErrCycle indicates a dependency cycle would be created
ErrCycle = errors.New("dependency cycle detected")
)
// wrapDBError wraps a database error with operation context
// It converts sql.ErrNoRows to ErrNotFound for consistent error handling
func wrapDBError(op string, err error) error {
if err == nil {
return nil
}
if errors.Is(err, sql.ErrNoRows) {
return fmt.Errorf("%s: %w", op, ErrNotFound)
}
return fmt.Errorf("%s: %w", op, err)
}
// wrapDBErrorf wraps a database error with formatted operation context
// It converts sql.ErrNoRows to ErrNotFound for consistent error handling
func wrapDBErrorf(err error, format string, args ...interface{}) error {
if err == nil {
return nil
}
op := fmt.Sprintf(format, args...)
if errors.Is(err, sql.ErrNoRows) {
return fmt.Errorf("%s: %w", op, ErrNotFound)
}
return fmt.Errorf("%s: %w", op, err)
}
// IsNotFound checks if an error is or wraps ErrNotFound
func IsNotFound(err error) bool {
return errors.Is(err, ErrNotFound)
}
// IsConflict checks if an error is or wraps ErrConflict
func IsConflict(err error) bool {
return errors.Is(err, ErrConflict)
}
// IsCycle checks if an error is or wraps ErrCycle
func IsCycle(err error) bool {
return errors.Is(err, ErrCycle)
}

View File

@@ -205,7 +205,7 @@ func EnsureIDs(ctx context.Context, conn *sql.Conn, prefix string, issues []*typ
if issues[i].ID != "" { if issues[i].ID != "" {
// Validate that explicitly provided ID matches the configured prefix (bd-177) // Validate that explicitly provided ID matches the configured prefix (bd-177)
if err := ValidateIssueIDPrefix(issues[i].ID, prefix); err != nil { if err := ValidateIssueIDPrefix(issues[i].ID, prefix); err != nil {
return err return wrapDBErrorf(err, "validate ID prefix for %s", issues[i].ID)
} }
// For hierarchical IDs (bd-a3f8e9.1), ensure parent exists // For hierarchical IDs (bd-a3f8e9.1), ensure parent exists

File diff suppressed because it is too large Load Diff

View File

@@ -3,7 +3,6 @@ package sqlite
import ( import (
"context" "context"
"database/sql" "database/sql"
"fmt"
"strings" "strings"
) )
@@ -24,7 +23,7 @@ func (s *SQLiteStorage) BeginTx(ctx context.Context) (*sql.Tx, error) {
func (s *SQLiteStorage) withTx(ctx context.Context, fn func(*sql.Tx) error) error { func (s *SQLiteStorage) withTx(ctx context.Context, fn func(*sql.Tx) error) error {
tx, err := s.db.BeginTx(ctx, nil) tx, err := s.db.BeginTx(ctx, nil)
if err != nil { if err != nil {
return fmt.Errorf("failed to begin transaction: %w", err) return wrapDBError("begin transaction", err)
} }
defer func() { _ = tx.Rollback() }() defer func() { _ = tx.Rollback() }()
@@ -33,7 +32,7 @@ func (s *SQLiteStorage) withTx(ctx context.Context, fn func(*sql.Tx) error) erro
} }
if err := tx.Commit(); err != nil { if err := tx.Commit(); err != nil {
return fmt.Errorf("failed to commit transaction: %w", err) return wrapDBError("commit transaction", err)
} }
return nil return nil