refactor(server): centralize db-error-to-HTTP mapping with writeDBError

internal/server/errors.go

@@ -2,8 +2,12 @@ package server
 
 import (
 	"crypto/sha256"
+	"errors"
 	"fmt"
+	"log/slog"
 	"net/http"
+
+	"github.com/dlorenc/docstore/internal/db"
 )
 
 // ErrorCode is a machine-readable code included in every API error response.
@@ -73,6 +77,65 @@ func writeAPIError(w http.ResponseWriter, code ErrorCode, status int, msg string
 	APIError{Code: code, Message: msg, Status: status}.write(w)
 }
 
+// writeDBError maps known db sentinel errors to typed API error responses.
+// For unrecognised errors it logs at Error level and writes HTTP 500.
+// The op and repo parameters are used only in the slog record for the default case.
+func writeDBError(w http.ResponseWriter, op, repo string, err error) {
+	switch {
+	case errors.Is(err, db.ErrBranchNotFound):
+		writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
+	case errors.Is(err, db.ErrBranchNotActive):
+		writeAPIError(w, ErrCodeBranchNotActive, http.StatusConflict, "branch is not active")
+	case errors.Is(err, db.ErrBranchExists):
+		writeAPIError(w, ErrCodeBranchExists, http.StatusConflict, "branch already exists")
+	case errors.Is(err, db.ErrBranchDraft):
+		writeAPIError(w, ErrCodeBranchDraft, http.StatusConflict, "branch is in draft state")
+	case errors.Is(err, db.ErrRepoNotFound):
+		writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found")
+	case errors.Is(err, db.ErrRepoExists):
+		writeAPIError(w, ErrCodeRepoExists, http.StatusConflict, "repo already exists")
+	case errors.Is(err, db.ErrOrgNotFound):
+		writeAPIError(w, ErrCodeOrgNotFound, http.StatusNotFound, "org not found")
+	case errors.Is(err, db.ErrOrgExists):
+		writeAPIError(w, ErrCodeOrgExists, http.StatusConflict, "org already exists")
+	case errors.Is(err, db.ErrOrgHasRepos):
+		writeAPIError(w, ErrCodeConflict, http.StatusConflict, "org has repos; delete them first")
+	case errors.Is(err, db.ErrOrgMemberNotFound):
+		writeAPIError(w, ErrCodeNotFound, http.StatusNotFound, "member not found")
+	case errors.Is(err, db.ErrRoleNotFound):
+		writeAPIError(w, ErrCodeRoleNotFound, http.StatusNotFound, "role not found")
+	case errors.Is(err, db.ErrInviteNotFound):
+		writeAPIError(w, ErrCodeInviteNotFound, http.StatusNotFound, "invite not found")
+	case errors.Is(err, db.ErrInviteExpired):
+		writeAPIError(w, ErrCodeGone, http.StatusGone, "invite expired")
+	case errors.Is(err, db.ErrInviteAlreadyAccepted):
+		writeAPIError(w, ErrCodeConflict, http.StatusConflict, "invite already accepted")
+	case errors.Is(err, db.ErrEmailMismatch):
+		writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, "identity does not match invite email")
+	case errors.Is(err, db.ErrReleaseNotFound):
+		writeAPIError(w, ErrCodeReleaseNotFound, http.StatusNotFound, "release not found")
+	case errors.Is(err, db.ErrReleaseExists):
+		writeAPIError(w, ErrCodeConflict, http.StatusConflict, "release already exists")
+	case errors.Is(err, db.ErrSelfApproval):
+		writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, "reviewer cannot approve their own commits")
+	case errors.Is(err, db.ErrCommentNotFound):
+		writeAPIError(w, ErrCodeCommentNotFound, http.StatusNotFound, "comment not found")
+	case errors.Is(err, db.ErrProposalNotFound):
+		writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found")
+	case errors.Is(err, db.ErrProposalExists):
+		writeAPIError(w, ErrCodeProposalExists, http.StatusConflict, "branch already has an open proposal")
+	case errors.Is(err, db.ErrSubscriptionNotFound):
+		writeAPIError(w, ErrCodeSubscriptionNotFound, http.StatusNotFound, "subscription not found")
+	case errors.Is(err, db.ErrIssueNotFound):
+		writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found")
+	case errors.Is(err, db.ErrIssueCommentNotFound):
+		writeAPIError(w, ErrCodeCommentNotFound, http.StatusNotFound, "comment not found")
+	default:
+		slog.Error("internal error", "op", op, "repo", repo, "error", err)
+		writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
+	}
+}
+
 // statusToCode maps an HTTP status to a generic ErrorCode.
 // Use writeAPIError with a specific code for domain-specific errors.
 func statusToCode(status int) ErrorCode {

internal/server/handlers.go

@@ -1,12 +1,8 @@
 package server
 
 import (
-	"errors"
-	"log/slog"
 	"net/http"
 	"strings"
-
-	"github.com/dlorenc/docstore/internal/db"
 )
 
 // parseRepoPath parses a /repos/... URL path into the full repo name and the
@@ -460,12 +456,7 @@ func writeError(w http.ResponseWriter, status int, msg string) {
 func (s *server) validateRepo(w http.ResponseWriter, r *http.Request, repo string) bool {
 	_, err := s.commitStore.GetRepo(r.Context(), repo)
 	if err != nil {
-		if errors.Is(err, db.ErrRepoNotFound) {
-			writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found")
-		} else {
-			slog.Error("internal error", "op", "validate_repo", "repo", repo, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "query failed")
-		}
+		writeDBError(w, "validate_repo", repo, err)
 		return false
 	}
 	return true

internal/server/handlers_branches.go

@@ -65,15 +65,7 @@ func (s *server) handleCreateBranch(w http.ResponseWriter, r *http.Request) {
 
 	resp, err := s.commitStore.CreateBranch(r.Context(), req)
 	if err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchExists):
-			writeAPIError(w, ErrCodeBranchExists, http.StatusConflict, "branch already exists")
-		case errors.Is(err, db.ErrRepoNotFound):
-			writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found")
-		default:
-			slog.Error("internal error", "op", "create_branch", "repo", repo, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "create_branch", repo, err)
 		return
 	}
 
@@ -112,13 +104,7 @@ func (s *server) handleUpdateBranch(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := s.commitStore.UpdateBranchDraft(r.Context(), repo, bname, req.Draft); err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		default:
-			slog.Error("internal error", "op", "update_branch", "repo", repo, "branch", bname, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "update_branch", repo, err)
 		return
 	}
 
@@ -146,13 +132,7 @@ func (s *server) handleEnableAutoMerge(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := s.commitStore.SetBranchAutoMerge(r.Context(), repo, bname, true); err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		default:
-			slog.Error("internal error", "op", "enable_auto_merge", "repo", repo, "branch", bname, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "enable_auto_merge", repo, err)
 		return
 	}
 
@@ -181,13 +161,7 @@ func (s *server) handleDisableAutoMerge(w http.ResponseWriter, r *http.Request)
 	}
 
 	if err := s.commitStore.SetBranchAutoMerge(r.Context(), repo, bname, false); err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		default:
-			slog.Error("internal error", "op", "disable_auto_merge", "repo", repo, "branch", bname, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "disable_auto_merge", repo, err)
 		return
 	}
 
@@ -216,15 +190,7 @@ func (s *server) handleDeleteBranch(w http.ResponseWriter, r *http.Request) {
 
 	err := s.commitStore.DeleteBranch(r.Context(), repo, bname)
 	if err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		case errors.Is(err, db.ErrBranchNotActive):
-			writeAPIError(w, ErrCodeBranchNotActive, http.StatusConflict, "branch is already merged or abandoned")
-		default:
-			slog.Error("internal error", "op", "delete_branch", "repo", repo, "branch", bname, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "delete_branch", repo, err)
 		return
 	}
 
@@ -346,8 +312,7 @@ func (s *server) handleRebase(w http.ResponseWriter, r *http.Request) {
 
 	resp, conflicts, err := s.commitStore.Rebase(r.Context(), req)
 	if err != nil {
-		switch {
-		case errors.Is(err, db.ErrRebaseConflict):
+		if errors.Is(err, db.ErrRebaseConflict) {
 			slog.Warn("rebase conflict", "repo", repo, "branch", req.Branch, "conflicts", len(conflicts))
 			apiConflicts := make([]model.ConflictEntry, len(conflicts))
 			for i, c := range conflicts {
@@ -358,13 +323,8 @@ func (s *server) handleRebase(w http.ResponseWriter, r *http.Request) {
 				}
 			}
 			writeJSON(w, http.StatusConflict, model.RebaseConflictError{Conflicts: apiConflicts})
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		case errors.Is(err, db.ErrBranchNotActive):
-			writeAPIError(w, ErrCodeBranchNotActive, http.StatusConflict, "branch is not active")
-		default:
-			slog.Error("internal error", "op", "rebase", "repo", repo, "branch", req.Branch, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
+		} else {
+			writeDBError(w, "rebase", repo, err)
 		}
 		return
 	}

internal/server/handlers_ci_request_token.go

@@ -126,13 +126,7 @@ func (s *server) handleCICheck(w http.ResponseWriter, r *http.Request) {
 	}
 	cr, err := s.commitStore.CreateCheckRun(r.Context(), repoName, req.Branch, req.CheckName, req.Status, reporter, req.LogURL, req.Sequence, attempt, req.Metadata)
 	if err != nil {
-		switch {
-		case errors.Is(err, db.ErrBranchNotFound):
-			writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found")
-		default:
-			slog.Error("internal error", "op", "ci_check", "repo", repoName, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "ci_check", repoName, err)
 		return
 	}
 

internal/server/handlers_events.go

@@ -12,7 +12,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/dlorenc/docstore/internal/db"
 	"github.com/dlorenc/docstore/internal/model"
 )
 
@@ -124,13 +123,7 @@ func (s *server) handleDeleteSubscription(w http.ResponseWriter, r *http.Request
 	if !isAdmin {
 		sub, err := s.commitStore.GetSubscription(r.Context(), id)
 		if err != nil {
-			switch {
-			case errors.Is(err, db.ErrSubscriptionNotFound):
-				writeAPIError(w, ErrCodeSubscriptionNotFound, http.StatusNotFound, "subscription not found")
-			default:
-				slog.Error("internal error", "op", "get_subscription", "id", id, "error", err)
-				writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-			}
+			writeDBError(w, "get_subscription", id, err)
 			return
 		}
 		if sub.CreatedBy != identity {
@@ -140,13 +133,7 @@ func (s *server) handleDeleteSubscription(w http.ResponseWriter, r *http.Request
 	}
 
 	if err := s.commitStore.DeleteSubscription(r.Context(), id); err != nil {
-		switch {
-		case errors.Is(err, db.ErrSubscriptionNotFound):
-			writeAPIError(w, ErrCodeSubscriptionNotFound, http.StatusNotFound, "subscription not found")
-		default:
-			slog.Error("internal error", "op", "delete_subscription", "id", id, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "delete_subscription", id, err)
 		return
 	}
 
@@ -165,13 +152,7 @@ func (s *server) handleResumeSubscription(w http.ResponseWriter, r *http.Request
 	if !isAdmin {
 		sub, err := s.commitStore.GetSubscription(r.Context(), id)
 		if err != nil {
-			switch {
-			case errors.Is(err, db.ErrSubscriptionNotFound):
-				writeAPIError(w, ErrCodeSubscriptionNotFound, http.StatusNotFound, "subscription not found")
-			default:
-				slog.Error("internal error", "op", "get_subscription", "id", id, "error", err)
-				writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-			}
+			writeDBError(w, "get_subscription", id, err)
 			return
 		}
 		if sub.CreatedBy != identity {
@@ -181,13 +162,7 @@ func (s *server) handleResumeSubscription(w http.ResponseWriter, r *http.Request
 	}
 
 	if err := s.commitStore.ResumeSubscription(r.Context(), id); err != nil {
-		switch {
-		case errors.Is(err, db.ErrSubscriptionNotFound):
-			writeAPIError(w, ErrCodeSubscriptionNotFound, http.StatusNotFound, "subscription not found")
-		default:
-			slog.Error("internal error", "op", "resume_subscription", "id", id, "error", err)
-			writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error")
-		}
+		writeDBError(w, "resume_subscription", id, err)
 		return
 	}