From cb4bcbcd8d8d02aaf4f7d463b0bc39eca31e2932 Mon Sep 17 00:00:00 2001 From: Dan Lorenc Date: Sat, 9 May 2026 10:56:26 -0400 Subject: [PATCH] refactor(server): centralize db-error-to-HTTP mapping with writeDBError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add writeDBError(w, op, repo, err) to internal/server/errors.go with a single switch covering all 23 db sentinel errors (ErrBranchNotFound, ErrRepoNotFound, ErrSelfApproval, ErrBranchDraft, ErrOrgNotFound, ErrInviteExpired, ErrIssueNotFound, etc.). Each case maps to the correct ErrCode constant and HTTP status, with the default logging at slog.Error and returning 500. Replace ~30 repetitive inline switch blocks across 11 handler files with calls to writeDBError, removing the need to repeat the sentinel→code→status mapping at each call site. For switches containing non-db cases (service.ErrForbidden, service.ErrConflict) or custom JSON bodies (ErrMergeConflict, ErrRebaseConflict), those special cases are handled inline first and the remainder falls through to writeDBError. Clean up now-unused imports ("errors", "log/slog", db package) from affected files. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- internal/server/errors.go | 63 +++++++++++ internal/server/handlers.go | 11 +- internal/server/handlers_branches.go | 56 ++-------- internal/server/handlers_ci_request_token.go | 8 +- internal/server/handlers_events.go | 33 +----- internal/server/handlers_issues.go | 54 +++------ internal/server/handlers_orgs.go | 56 +--------- internal/server/handlers_proposals.go | 109 ++++--------------- internal/server/handlers_releases.go | 24 +--- internal/server/handlers_repos.go | 35 +----- internal/server/handlers_users.go | 10 +- internal/server/server.go | 10 +- 12 files changed, 125 insertions(+), 344 deletions(-) diff --git a/internal/server/errors.go b/internal/server/errors.go index 688fbcb7..e094b50c 100644 --- a/internal/server/errors.go +++ b/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 { diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 399ac031..cff16e70 100644 --- a/internal/server/handlers.go +++ b/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 diff --git a/internal/server/handlers_branches.go b/internal/server/handlers_branches.go index 8b4d8bab..5861a8bf 100644 --- a/internal/server/handlers_branches.go +++ b/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 } diff --git a/internal/server/handlers_ci_request_token.go b/internal/server/handlers_ci_request_token.go index 63fa8bb6..8380425c 100644 --- a/internal/server/handlers_ci_request_token.go +++ b/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 } diff --git a/internal/server/handlers_events.go b/internal/server/handlers_events.go index d0be373c..719ef7ff 100644 --- a/internal/server/handlers_events.go +++ b/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 } diff --git a/internal/server/handlers_issues.go b/internal/server/handlers_issues.go index 5b5bc2ec..d3b0412e 100644 --- a/internal/server/handlers_issues.go +++ b/internal/server/handlers_issues.go @@ -112,12 +112,7 @@ func (s *server) handleGetIssue(w http.ResponseWriter, r *http.Request) { iss, err := s.commitStore.GetIssue(r.Context(), repo, number) if err != nil { - if errors.Is(err, db.ErrIssueNotFound) { - writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found") - } else { - slog.Error("internal error", "op", "get_issue", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "get_issue", repo, err) return } writeJSON(w, http.StatusOK, iss) @@ -150,14 +145,10 @@ func (s *server) handleUpdateIssue(w http.ResponseWriter, r *http.Request) { } iss, err := s.svc.UpdateIssue(r.Context(), identity, role, repo, number, req.Title, req.Body, labelsPtr) if err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrIssueNotFound): - writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found") - default: - slog.Error("internal error", "op", "update_issue", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "update_issue", repo, err) } return } @@ -205,11 +196,8 @@ func (s *server) handleCloseIssue(w http.ResponseWriter, r *http.Request) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) case errors.Is(err, service.ErrConflict): writeAPIError(w, ErrCodeConflict, http.StatusConflict, err.Error()) - case errors.Is(err, db.ErrIssueNotFound): - writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found") default: - slog.Error("internal error", "op", "close_issue", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + writeDBError(w, "close_issue", repo, err) } return } @@ -239,11 +227,8 @@ func (s *server) handleReopenIssue(w http.ResponseWriter, r *http.Request) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) case errors.Is(err, service.ErrConflict): writeAPIError(w, ErrCodeConflict, http.StatusConflict, err.Error()) - case errors.Is(err, db.ErrIssueNotFound): - writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found") default: - slog.Error("internal error", "op", "reopen_issue", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + writeDBError(w, "reopen_issue", repo, err) } return } @@ -300,12 +285,7 @@ func (s *server) handleCreateIssueComment(w http.ResponseWriter, r *http.Request identity := IdentityFromContext(r.Context()) c, err := s.svc.CreateIssueComment(r.Context(), identity, repo, number, req.Body) if err != nil { - if errors.Is(err, db.ErrIssueNotFound) { - writeAPIError(w, ErrCodeIssueNotFound, http.StatusNotFound, "issue not found") - } else { - slog.Error("internal error", "op", "create_issue_comment", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_issue_comment", repo, err) return } @@ -340,14 +320,10 @@ func (s *server) handleUpdateIssueComment(w http.ResponseWriter, r *http.Request c, err := s.svc.UpdateIssueComment(r.Context(), identity, role, repo, number, commentID, req.Body) if err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrIssueCommentNotFound): - writeAPIError(w, ErrCodeCommentNotFound, http.StatusNotFound, "comment not found") - default: - slog.Error("internal error", "op", "update_issue_comment", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "update_issue_comment", repo, err) } return } @@ -372,14 +348,10 @@ func (s *server) handleDeleteIssueComment(w http.ResponseWriter, r *http.Request role := RoleFromContext(r.Context()) if err := s.svc.DeleteIssueComment(r.Context(), identity, role, repo, commentID); err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrIssueCommentNotFound): - writeAPIError(w, ErrCodeCommentNotFound, http.StatusNotFound, "comment not found") - default: - slog.Error("internal error", "op", "delete_issue_comment", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "delete_issue_comment", repo, err) } return } diff --git a/internal/server/handlers_orgs.go b/internal/server/handlers_orgs.go index 5cf89477..957ba528 100644 --- a/internal/server/handlers_orgs.go +++ b/internal/server/handlers_orgs.go @@ -38,13 +38,7 @@ func (s *server) handleCreateOrg(w http.ResponseWriter, r *http.Request) { identity := IdentityFromContext(r.Context()) org, err := s.svc.CreateOrg(r.Context(), identity, req.Name) if err != nil { - switch { - case errors.Is(err, db.ErrOrgExists): - writeAPIError(w, ErrCodeOrgExists, http.StatusConflict, "org already exists") - default: - slog.Error("internal error", "op", "create_org", "org", req.Name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_org", req.Name, err) return } writeJSON(w, http.StatusCreated, org) @@ -69,13 +63,7 @@ func (s *server) handleGetOrg(w http.ResponseWriter, r *http.Request) { name := r.PathValue("org") org, err := s.commitStore.GetOrg(r.Context(), name) if err != nil { - switch { - case errors.Is(err, db.ErrOrgNotFound): - writeAPIError(w, ErrCodeOrgNotFound, http.StatusNotFound, "org not found") - default: - slog.Error("internal error", "op", "get_org", "org", name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "query failed") - } + writeDBError(w, "get_org", name, err) return } writeJSON(w, http.StatusOK, org) @@ -86,15 +74,7 @@ func (s *server) handleDeleteOrg(w http.ResponseWriter, r *http.Request) { name := r.PathValue("org") err := s.commitStore.DeleteOrg(r.Context(), name) if err != nil { - switch { - case errors.Is(err, db.ErrOrgNotFound): - writeAPIError(w, ErrCodeOrgNotFound, http.StatusNotFound, "org not found") - case errors.Is(err, db.ErrOrgHasRepos): - writeAPIError(w, ErrCodeConflict, http.StatusConflict, "org has repos; delete them first") - default: - slog.Error("internal error", "op", "delete_org", "org", name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "delete_org", name, err) return } s.emit(r.Context(), evtypes.OrgDeleted{Org: name, DeletedBy: IdentityFromContext(r.Context())}) @@ -255,13 +235,7 @@ func (s *server) handleRemoveOrgMember(w http.ResponseWriter, r *http.Request) { } if err := s.commitStore.RemoveOrgMember(r.Context(), org, identity); err != nil { - switch { - case errors.Is(err, db.ErrOrgMemberNotFound): - writeAPIError(w, ErrCodeNotFound, http.StatusNotFound, "member not found") - default: - slog.Error("internal error", "op", "remove_org_member", "org", org, "identity", identity, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "remove_org_member", org, err) return } @@ -377,19 +351,7 @@ func (s *server) handleAcceptInvite(w http.ResponseWriter, r *http.Request) { identity := IdentityFromContext(r.Context()) if err := s.commitStore.AcceptInvite(r.Context(), org, token, identity); err != nil { - switch { - 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") - default: - slog.Error("internal error", "op", "accept_invite", "org", org, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "accept_invite", org, err) return } @@ -416,13 +378,7 @@ func (s *server) handleRevokeInvite(w http.ResponseWriter, r *http.Request) { } if err := s.commitStore.RevokeInvite(r.Context(), org, inviteID); err != nil { - switch { - case errors.Is(err, db.ErrInviteNotFound): - writeAPIError(w, ErrCodeInviteNotFound, http.StatusNotFound, "invite not found") - default: - slog.Error("internal error", "op", "revoke_invite", "org", org, "invite_id", inviteID, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "revoke_invite", org, err) return } diff --git a/internal/server/handlers_proposals.go b/internal/server/handlers_proposals.go index f0f461bf..e88c6088 100644 --- a/internal/server/handlers_proposals.go +++ b/internal/server/handlers_proposals.go @@ -44,15 +44,7 @@ func (s *server) handleReview(w http.ResponseWriter, r *http.Request) { review, err := s.svc.CreateReview(r.Context(), reviewer, repo, req.Branch, req.Status, req.Body) if err != nil { - switch { - case errors.Is(err, db.ErrBranchNotFound): - writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found") - case errors.Is(err, db.ErrSelfApproval): - writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, "reviewer cannot approve their own commits") - default: - slog.Error("internal error", "op", "review", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "review", repo, err) return } @@ -99,13 +91,7 @@ func (s *server) handleCheck(w http.ResponseWriter, r *http.Request) { } cr, err := s.commitStore.CreateCheckRun(r.Context(), repo, 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", "check", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "check", repo, err) return } @@ -197,13 +183,7 @@ func (s *server) handleRetryChecks(w http.ResponseWriter, r *http.Request) { attempt, err := s.commitStore.RetryChecks(r.Context(), repo, req.Branch, req.Sequence, req.Checks) if err != nil { - switch { - case errors.Is(err, db.ErrBranchNotFound): - writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found") - default: - slog.Error("internal error", "op", "retry_checks", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "retry_checks", repo, err) return } @@ -253,13 +233,7 @@ func (s *server) handleCreateReviewComment(w http.ResponseWriter, r *http.Reques comment, err := s.commitStore.CreateReviewComment(r.Context(), repo, req.Branch, req.Path, req.VersionID, req.Body, author, req.ReviewID) if err != nil { - switch { - case errors.Is(err, db.ErrBranchNotFound): - writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found") - default: - slog.Error("internal error", "op", "create_review_comment", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_review_comment", repo, err) return } @@ -300,14 +274,10 @@ func (s *server) handleDeleteReviewComment(w http.ResponseWriter, r *http.Reques role := RoleFromContext(r.Context()) if err := s.svc.DeleteReviewComment(r.Context(), identity, role, repo, commentID); err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrCommentNotFound): - writeAPIError(w, ErrCodeCommentNotFound, http.StatusNotFound, "comment not found") - default: - slog.Error("internal error", "op", "delete_review_comment", "repo", repo, "comment", commentID, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "delete_review_comment", repo, err) } return } @@ -349,15 +319,7 @@ func (s *server) handleCreateProposal(w http.ResponseWriter, r *http.Request) { p, err := s.svc.CreateProposal(r.Context(), author, repo, req.Branch, baseBranch, req.Title, req.Description) if err != nil { - switch { - case errors.Is(err, db.ErrProposalExists): - writeAPIError(w, ErrCodeProposalExists, http.StatusConflict, "branch already has an open proposal") - case errors.Is(err, db.ErrBranchNotFound): - writeAPIError(w, ErrCodeBranchNotFound, http.StatusNotFound, "branch not found") - default: - slog.Error("internal error", "op", "create_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_proposal", repo, err) return } @@ -410,12 +372,7 @@ func (s *server) handleGetProposal(w http.ResponseWriter, r *http.Request) { proposalID := r.PathValue("proposalID") p, err := s.commitStore.GetProposal(r.Context(), repo, proposalID) if err != nil { - if errors.Is(err, db.ErrProposalNotFound) { - writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found") - } else { - slog.Error("internal error", "op", "get_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "get_proposal", repo, err) return } etag := computeETag(p.ID, fmt.Sprintf("%d", p.UpdatedAt.UnixNano())) @@ -437,12 +394,7 @@ func (s *server) handleUpdateProposal(w http.ResponseWriter, r *http.Request) { // Fetch existing proposal for If-Match validation. existing, err := s.commitStore.GetProposal(r.Context(), repo, proposalID) if err != nil { - if errors.Is(err, db.ErrProposalNotFound) { - writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found") - } else { - slog.Error("internal error", "op", "update_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "update_proposal", repo, err) return } @@ -458,14 +410,10 @@ func (s *server) handleUpdateProposal(w http.ResponseWriter, r *http.Request) { p, err := s.svc.UpdateProposal(r.Context(), identity, role, repo, proposalID, req.Title, req.Description) if err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrProposalNotFound): - writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found") - default: - slog.Error("internal error", "op", "update_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "update_proposal", repo, err) } return } @@ -486,12 +434,7 @@ func (s *server) handleCloseProposal(w http.ResponseWriter, r *http.Request) { // Fetch existing proposal for If-Match validation. existing, err := s.commitStore.GetProposal(r.Context(), repo, proposalID) if err != nil { - if errors.Is(err, db.ErrProposalNotFound) { - writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found") - } else { - slog.Error("internal error", "op", "close_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "close_proposal", repo, err) return } @@ -500,14 +443,10 @@ func (s *server) handleCloseProposal(w http.ResponseWriter, r *http.Request) { } if err := s.svc.CloseProposal(r.Context(), identity, role, repo, proposalID); err != nil { - switch { - case errors.Is(err, service.ErrForbidden): + if errors.Is(err, service.ErrForbidden) { writeAPIError(w, ErrCodeForbidden, http.StatusForbidden, err.Error()) - case errors.Is(err, db.ErrProposalNotFound): - writeAPIError(w, ErrCodeProposalNotFound, http.StatusNotFound, "proposal not found") - default: - slog.Error("internal error", "op", "close_proposal", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "close_proposal", repo, err) } return } @@ -565,8 +504,7 @@ func (s *server) handleMerge(w http.ResponseWriter, r *http.Request) { resp, conflicts, err := s.commitStore.Merge(r.Context(), req) if err != nil { - switch { - case errors.Is(err, db.ErrMergeConflict): + if errors.Is(err, db.ErrMergeConflict) { slog.Warn("merge conflict", "repo", repo, "branch", req.Branch, "conflicts", len(conflicts)) // Convert conflicts to API response. apiConflicts := make([]model.ConflictEntry, len(conflicts)) @@ -578,15 +516,8 @@ func (s *server) handleMerge(w http.ResponseWriter, r *http.Request) { } } writeJSON(w, http.StatusConflict, model.MergeConflictError{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") - case errors.Is(err, db.ErrBranchDraft): - writeAPIError(w, ErrCodeBranchDraft, http.StatusConflict, "branch is in draft state") - default: - slog.Error("internal error", "op", "merge", "repo", repo, "branch", req.Branch, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") + } else { + writeDBError(w, "merge", repo, err) } return } diff --git a/internal/server/handlers_releases.go b/internal/server/handlers_releases.go index 621433ba..e6a7ad92 100644 --- a/internal/server/handlers_releases.go +++ b/internal/server/handlers_releases.go @@ -65,13 +65,7 @@ func (s *server) handleCreateRelease(w http.ResponseWriter, r *http.Request) { createdBy := IdentityFromContext(r.Context()) rel, err := s.commitStore.CreateRelease(r.Context(), repo, req.Name, sequence, req.Body, createdBy) if err != nil { - switch { - case errors.Is(err, db.ErrReleaseExists): - writeAPIError(w, ErrCodeConflict, http.StatusConflict, "release already exists") - default: - slog.Error("internal error", "op", "create_release", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_release", repo, err) return } @@ -123,13 +117,7 @@ func (s *server) handleGetRelease(w http.ResponseWriter, r *http.Request) { rel, err := s.commitStore.GetRelease(r.Context(), repo, releaseName) if err != nil { - switch { - case errors.Is(err, db.ErrReleaseNotFound): - writeAPIError(w, ErrCodeReleaseNotFound, http.StatusNotFound, "release not found") - default: - slog.Error("internal error", "op", "get_release", "repo", repo, "release", releaseName, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "query failed") - } + writeDBError(w, "get_release", repo, err) return } writeJSON(w, http.StatusOK, rel) @@ -144,13 +132,7 @@ func (s *server) handleDeleteRelease(w http.ResponseWriter, r *http.Request) { releaseName := r.PathValue("release") if err := s.commitStore.DeleteRelease(r.Context(), repo, releaseName); err != nil { - switch { - case errors.Is(err, db.ErrReleaseNotFound): - writeAPIError(w, ErrCodeReleaseNotFound, http.StatusNotFound, "release not found") - default: - slog.Error("internal error", "op", "delete_release", "repo", repo, "release", releaseName, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "delete_release", repo, err) return } diff --git a/internal/server/handlers_repos.go b/internal/server/handlers_repos.go index 7893d952..54ca18a6 100644 --- a/internal/server/handlers_repos.go +++ b/internal/server/handlers_repos.go @@ -2,7 +2,6 @@ package server import ( "encoding/json" - "errors" "fmt" "log/slog" "net/http" @@ -42,15 +41,7 @@ func (s *server) handleCreateRepo(w http.ResponseWriter, r *http.Request) { identity := IdentityFromContext(r.Context()) repo, err := s.svc.CreateRepo(r.Context(), identity, req) if err != nil { - switch { - 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") - default: - slog.Error("internal error", "op", "create_repo", "repo", req.Name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "create_repo", req.Name, err) return } @@ -77,13 +68,7 @@ func (s *server) handleGetRepo(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") repo, err := s.commitStore.GetRepo(r.Context(), name) if err != nil { - switch { - case errors.Is(err, db.ErrRepoNotFound): - writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found") - default: - slog.Error("internal error", "op", "get_repo", "repo", name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "query failed") - } + writeDBError(w, "get_repo", name, err) return } writeJSON(w, http.StatusOK, repo) @@ -94,13 +79,7 @@ func (s *server) handleDeleteRepo(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") err := s.commitStore.DeleteRepo(r.Context(), name) if err != nil { - switch { - case errors.Is(err, db.ErrRepoNotFound): - writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found") - default: - slog.Error("internal error", "op", "delete_repo", "repo", name, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "delete_repo", name, err) return } @@ -150,13 +129,7 @@ func (s *server) handlePurge(w http.ResponseWriter, r *http.Request) { DryRun: req.DryRun, }) if err != nil { - switch { - case errors.Is(err, db.ErrRepoNotFound): - writeAPIError(w, ErrCodeRepoNotFound, http.StatusNotFound, "repo not found") - default: - slog.Error("internal error", "op", "purge", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "purge", repo, err) return } diff --git a/internal/server/handlers_users.go b/internal/server/handlers_users.go index 22bf76f6..dcafd557 100644 --- a/internal/server/handlers_users.go +++ b/internal/server/handlers_users.go @@ -2,11 +2,9 @@ package server import ( "encoding/json" - "errors" "log/slog" "net/http" - "github.com/dlorenc/docstore/internal/db" evtypes "github.com/dlorenc/docstore/internal/events/types" "github.com/dlorenc/docstore/internal/model" ) @@ -81,13 +79,7 @@ func (s *server) handleDeleteRole(w http.ResponseWriter, r *http.Request) { } if err := s.commitStore.DeleteRole(r.Context(), repo, identity); err != nil { - switch { - case errors.Is(err, db.ErrRoleNotFound): - writeError(w, http.StatusNotFound, "role not found") - default: - slog.Error("internal error", "op", "delete_role", "repo", repo, "identity", identity, "error", err) - writeError(w, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "delete_role", repo, err) return } diff --git a/internal/server/server.go b/internal/server/server.go index d13c197b..cd72ef1c 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -706,15 +706,7 @@ func (s *server) handleCommit(w http.ResponseWriter, r *http.Request) { resp, err := s.svc.Commit(r.Context(), identity, req) 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 not active") - default: - slog.Error("internal error", "op", "commit", "repo", repo, "error", err) - writeAPIError(w, ErrCodeInternalError, http.StatusInternalServerError, "internal server error") - } + writeDBError(w, "commit", repo, err) return }