fix(purchase): surface save error on failed-status execution record#1237
fix(purchase): surface save error on failed-status execution record#1237cristim wants to merge 1 commit into
Conversation
The credential-resolution failure branch in executeForAccount discarded the SavePurchaseExecution error (_ =), while the post-purchase save in the same function treats a save failure as AUDIT LOSS. If the DB write failed on the credential-failure path, the per-account "failed" row was silently lost with no trace, leaving a gap in the History audit trail. Handle the failure-path save exactly like the success path: when the save fails, return an AUDIT LOSS error joined with the original credential-resolution error so neither failure is masked and the save error stays inspectable via errors.Is. Regression test asserts both errors surface; it fails on the previous code (save error silently dropped) and passes with the fix. Closes #1184
|
Warning Review limit reached
More reviews will be available in 57 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Problem
COR-10 (#1184): in
executeForAccount(internal/purchase/execution.go), the credential-resolution failure branch discarded theSavePurchaseExecutionerror (_ =), while the post-purchase save thirty lines later treats a save failure asAUDIT LOSSand returns it. If the DB write failed on the credential-failure path, the per-account "failed" row was silently lost with no log and no error, leaving a gap in the History audit trail.Fix
Handle the failure-path save exactly like the success path: when the save fails, return an
AUDIT LOSS: failed to save execution record ...error joined (viaerrors.Join) with the original credential-resolution error, so neither failure is masked and the underlying save error stays inspectable viaerrors.Is.Test evidence
TestExecuteForAccount_CredentialFailure_SaveErrorSurfacedreplicates the real scenario (credential resolution fails AND the failed-row save fails) and asserts both the AUDIT LOSS error and the credential error surface.go build ./...clean;go test ./internal/purchase/... -count=1: 207 passed;go vetclean.execution_test.gotiming assertions were not touched (TEST-05 / TEST-05: Elapsed-wall-time parallelism assertion in purchase fan-out test is CI-load sensitive #1158 is separate).Closes #1184