Summary
db/integrity hand-rolls the same "report and carry on" accumulator in three places, each slightly differently:
caplin_blob_integrity.go — a local report := func(err error) error closure over a firstErrcaplin_state_integrity.go — firstErr inline, returned at the endcommitment_integrity.go — integrityErr inline, twice
#24397 adds a shared problems type for the five checks that were dropping their verdict entirely. These three are not in that set: they already return firstErr/integrityErr, so they report correctly today. Folding them in is a deduplication, not a bug fix.
Why it was left out of #24397
The semantics differ. The three keep the first error; problems returns a count summary wrapping ErrIntegrity. Absorbing them changes what those checks return to their callers, in checks that are currently correct — which does not belong in a change whose subject is "runs that found problems must not report success".
What it would buy
One accumulator instead of four, and the reason to have a shared type at all. Worth deciding whether the count or the first error is the better contract, and applying it uniformly:
- a count tells an operator how much is wrong
- the first error keeps the detail of what is wrong in the returned value rather than only in the log
problems could carry both — tally the count and keep the first error, so verdict returns a wrapped first error with the count in the message. That would let all four sites collapse onto it without losing anything.
Summary
db/integrityhand-rolls the same "report and carry on" accumulator in three places, each slightly differently:caplin_blob_integrity.go— a localreport := func(err error) errorclosure over afirstErrcaplin_state_integrity.go—firstErrinline, returned at the endcommitment_integrity.go—integrityErrinline, twice#24397 adds a shared
problemstype for the five checks that were dropping their verdict entirely. These three are not in that set: they already returnfirstErr/integrityErr, so they report correctly today. Folding them in is a deduplication, not a bug fix.Why it was left out of #24397
The semantics differ. The three keep the first error;
problemsreturns a count summary wrappingErrIntegrity. Absorbing them changes what those checks return to their callers, in checks that are currently correct — which does not belong in a change whose subject is "runs that found problems must not report success".What it would buy
One accumulator instead of four, and the reason to have a shared type at all. Worth deciding whether the count or the first error is the better contract, and applying it uniformly:
problemscould carry both — tally the count and keep the first error, soverdictreturns a wrapped first error with the count in the message. That would let all four sites collapse onto it without losing anything.