From 3d811988f9960cf9057e09f30b59ceb71a7623c2 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sat, 8 Aug 2026 22:56:58 +0300 Subject: [PATCH] service: wrap the merge and reject failures with culpa MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three sites, all of them the outermost wrap on an error that becomes a 500: the merge path's non-staleness arm, the reject path's default arm, and the merged-but-the-row-did-not case, whose remedy now rides on the error as a hint instead of sitting in a sentence one wrap away from being buried. Outermost is the whole criterion. scribe.Err type-asserts the error it is handed for slog.LogValuer rather than searching the chain, so a culpa error with an fmt.Errorf above it logs as a plain string and the stacktrace is lost — which is also why the sentinel arms stay fmt.Errorf: they carry no cause worth a stack, and their text is what a 404 or 409 shows a viewer. errors.Is and errors.As traverse culpa's wrap, so the sentinel mapping in the surfaces is unchanged. --- go.sum | 2 -- service/merge.go | 27 +++++++++++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/go.sum b/go.sum index 66a86c9a68024d6c6c76453f5c93cceb23f70dae..04a48b483c7b046389bedde68483ebb37cc5eae1 100644 --- a/go.sum +++ b/go.sum @@ -421,7 +421,5 @@ modernc.org/sqlite v1.38.2 h1:Aclu7+tgjgcQVShZqim41Bbw9Cho0y/7WzYptXqkEek= modernc.org/sqlite v1.38.2/go.mod h1:cPTJYSlgg3Sfg046yBShXENNtPrWrDX8bsbAQBzgQ5E= sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152 h1:9kQC+tDO2CO8avlKadb9Z0if4a6vJuEK80+4zcb6/fU= sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152/go.mod h1:Mu1Vx39ws/OTKWGoVERXvkdRSPLBdhuFTYv0ftVV31c= -sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808192241-9377c43a02ca h1:LCfxvF1VJl7djl7noAeXObfuY1csJr2OOFExwU4Y/N4= -sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808192241-9377c43a02ca/go.mod h1:KeoZjm+/nnsdtc1WxB7X/0EeC+Rggt2OkwJDEc6XWnw= sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808194355-f019dbe4ea3e h1:9yH4eagCWQMdFFbF+LQmvJjXWFdSDijhaxolrwfgatA= sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808194355-f019dbe4ea3e/go.mod h1:KeoZjm+/nnsdtc1WxB7X/0EeC+Rggt2OkwJDEc6XWnw= diff --git a/service/merge.go b/service/merge.go index 51e9b7bf7104b53e748cf58602b7bdb8f8131105..56e848fc3760d78ee46e9ca584136e3088f2494c 100644 --- a/service/merge.go +++ b/service/merge.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" + "go.bigb.es/auxilia/culpa" + "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/db" "sourcecraft.dev/bigbes/sr-ht-spec/gitx" @@ -143,8 +145,15 @@ func (s *Service) mergeProposal(ctx context.Context, sp *Space, row *db.Proposal // reconciler repairs (RepairMarkMerged). Surface it rather than // reporting a failed merge — the merge commit is on the approved branch // and reads already see it. - return Proposal{}, fmt.Errorf("service: proposal %d merged to %s in %s but its row could not be updated "+ - "(the reconciler will repair it): %w", row.ID, short(mergedRev), sp.Ref, err) + // + // The hint rides on the error rather than sitting in this sentence + // because the sentence is one wrap away from being buried under + // another, while a culpa detail survives every wrap above it and comes + // out as its own field wherever this is finally logged. + return Proposal{}, culpa.WithHint( + culpa.Wrapf(err, "service: proposal %d merged to %s in %s but its row could not be updated", + row.ID, short(mergedRev), sp.Ref), + "the merge is on the approved branch; the reconciler repairs the row (RepairMarkMerged)") } // mergeProposal is the single merge point — both the public Merge and @@ -195,7 +204,15 @@ func mergeErr(err error, ref core.SpaceRef, proposalID int) error { if errors.As(err, &stale) { return fmt.Errorf("%w: proposal %d against %s: %w", ErrStale, proposalID, ref, err) } - return fmt.Errorf("service: merge proposal %d in %s: %w", proposalID, ref, err) + // Everything that is not staleness is a git or database failure: the + // surfaces answer it as a 500, and nobody but an operator ever reads it. + // culpa.Wrapf and not fmt.Errorf because this is the *outermost* wrap, which + // is the one the log sees — it carries a stacktrace and scribe.Err expands + // it. The message chain says "merge proposal 7"; the stack says which of the + // merge's dozen git calls produced it, which is the question a 500 here + // actually raises. errors.Is and errors.As still traverse it, so the + // sentinel mapping above and in the surfaces is unaffected. + return culpa.Wrapf(err, "service: merge proposal %d in %s", proposalID, ref) } // resolveProposalErr maps a db resolution failure (reject) onto this package's @@ -208,6 +225,8 @@ func resolveProposalErr(err error, proposalID int) error { case errors.Is(err, db.ErrProposalNotOpen): return fmt.Errorf("%w: proposal %d", ErrProposalNotOpen, proposalID) default: - return fmt.Errorf("service: reject proposal %d: %w", proposalID, err) + // A database failure, for the reason mergeErr gives: outermost wrap, + // operator-only, so it carries a stack. + return culpa.Wrapf(err, "service: reject proposal %d", proposalID) } }