fix(azure): propagate reservation pager construction errors - #161
Conversation
Return wrapped errors from cache, Cosmos DB, database and search inventory clients so SDK construction failures cannot appear as empty inventory. Closes #79
cristim
added
triaged
Independent implementation review: go79Reviewer: gpt-6-astra. Date: 2026-09-30. Pass 1No actionable findings across completeness, correctness, security, bugs, duplication and over-engineering. Four error branches now return nil commitments and contextual %w errors, matching Synapse. Database's unused log import is removed; no unrelated production changes. Callers receive a failure rather than an apparently empty inventory. Existing collector behavior is untouched. Pass 2No actionable findings across all six dimensions. Regression uses public constructors, a real SDK configuration failure and assertions on nil result, context and unwrapped cause. It replaces the whole cloud configuration, restores via cleanup, and has no parallel test calls. The single comment explains required sequencing. No artificial factory or new dependency is introduced. Reviewed source and the five-file staged diff again; diff-check passes. Independent clone review-go79: installed only the new test over unchanged base, then ran focused race regression. All four cases failed because the error was nil, proving the baseline defect. Installed the four reviewed production files and reran regression plus provider service-constructor control: exit 0, 1.515s. Logs reviewer-go79-red.log and reviewer-go79-green.log. Author's full Azure race suite completed successfully; independent focused runtime verifies the changed path. Evidence uses local SDK construction, not Azure cloud execution. Verdict: implementation approved for normal commit. Final committed-SHA gate pending. Final committed-SHA gateFetched and checked out a9f1e2b in independent review-go79 clone. Prior review modifications preserved in a stash; checkout clean. Re-read the complete five-file committed diff. Its SHA256 is identical to the reviewed staged patch above. Fresh focused race regression and all-service constructor control passed on this exact SHA, 1.936s (reviewer-go79-final-sha.log). No actionable findings. Approved for publication under root's normal CI gates. Any later integration SHA requires review of the changed artifacts and fresh focused evidence before merge. |
|
Warning Review limit reached
This review includes 5 billable files and costs up to $1.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 40 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configurationConfiguration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (5)
Comment |
Integrate the reviewed AWS usage-history fix from main without rewriting the published Azure fix commit.
Independent implementation review: go79Reviewer: gpt-6-astra. Date: 2026-09-30. Pass 1No actionable findings across completeness, correctness, security, bugs, duplication and over-engineering. Four error branches now return nil commitments and contextual %w errors, matching Synapse. Database's unused log import is removed; no unrelated production changes. Callers receive a failure rather than an apparently empty inventory. Existing collector behavior is untouched. Pass 2No actionable findings across all six dimensions. Regression uses public constructors, a real SDK configuration failure and assertions on nil result, context and unwrapped cause. It replaces the whole cloud configuration, restores via cleanup, and has no parallel test calls. The single comment explains required sequencing. No artificial factory or new dependency is introduced. Reviewed source and the five-file staged diff again; diff-check passes. Independent clone review-go79: installed only the new test over unchanged base, then ran focused race regression. All four cases failed because the error was nil, proving the baseline defect. Installed the four reviewed production files and reran regression plus provider service-constructor control: exit 0, 1.515s. Logs reviewer-go79-red.log and reviewer-go79-green.log. Author's full Azure race suite completed successfully; independent focused runtime verifies the changed path. Evidence uses local SDK construction, not Azure cloud execution. Verdict: implementation approved for normal commit. Final committed-SHA gate pending. Final committed-SHA gateFetched and checked out a9f1e2b in independent review-go79 clone. Prior review modifications preserved in a stash; checkout clean. Re-read the complete five-file committed diff. Its SHA256 is identical to the reviewed staged patch above. Fresh focused race regression and all-service constructor control passed on this exact SHA, 1.936s (reviewer-go79-final-sha.log). No actionable findings. Approved for publication under root's normal CI gates. Any later integration SHA requires review of the changed artifacts and fresh focused evidence before merge. Integration with main 7e2ad63: precommit passesPass 1: no actionable findings across all six dimensions. Re-read the complete incoming AWS production/test diff and Azure diff against main. Incoming source is the previously independently verified go68 request/response correction; the Azure error handling and test remain intact. Pass 2: no actionable findings across all six dimensions or integration scope. Index comparison proves AWS exactly matches origin/main, Azure exactly matches prior HEAD, and no pkg/go.work changes exist. The Azure global SDK configuration mutation is confined to its separate test binary; AWS parallel HTTP fixtures do not share that global or module process. No overlapping production helpers, imports or dependency changes. Diff-check clean. Approved normal merge commit; final integrated-SHA runtime gate remains pending. Final integrated-SHA gateIndependently fetched and checked out fa3d2ce, parents a9f1e2b and 7e2ad63. Clean checkout. Re-read complete seven-file final delta from base. Committed Azure files exactly equal prior approved Azure HEAD; AWS files exactly equal merged main. Fresh GOWORK=off Go1.26.6 focused race tests passed on this exact merge SHA: Azure regression plus constructor control 2.061s; AWS daily-history regression and controls 1.548s. Logs reviewer-go79-integrated-azure.log and reviewer-go79-integrated-aws.log. No actionable findings. Exact merged SHA approved for normal publication and root's CI-gated merge process; no cloud execution claimed. |
1 participant
Cache, Cosmos DB, database and search clients now return a wrapped error when Azure reservation pager construction fails. Previously they returned an empty inventory with no error, so callers could mistake failed inventory collection for no existing reservations.
The regression uses the real pinned Azure SDK with an invalid local cloud configuration. It failed for all four clients before the fix and passes afterward. This construction path does not acquire credentials or contact Azure; permission failures occur during page fetch and retain their existing handling.
Validation: full Azure module race suite, build, vet, golangci-lint 2.10.1, and all installed commit hooks passed on Go 1.26.6. Independent gpt-6-astra review completed two clean implementation passes and independently verified the failing baseline and passing fix.
Closes #79