From d4fe95920a227829c7908d59caa5f60c80400587 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 21:16:50 +0000 Subject: [PATCH 1/2] fix(revoke): keep the in-flight flag on an ambiguous Azure Return failure and refuse a late Lambda claim A Return that failed without a definite answer from Azure (a transport error, a 5xx, the 3m Return deadline, a response that could not be read) cleared the in-flight flag and answered a flat failure. If Azure had taken the Return, a refunded purchase read as active and failed until an operator fixed it. Only a definite refusal now clears the flag: Azure answered the only request the client sent with a 4xx other than 408. A per-retry policy on the production Return client counts the sends, because azcore retries a POST on transport errors, 408, 429 and 5xx, and the 4xx that ends a retried call may be Azure refusing a second copy of a Return it already took. Every other failure keeps the flag (so no request can post a second Return), logs the row for an operator, and answers 500 "may already have been accepted by Azure; verify the reservation state in the Azure portal before retrying", the RI exchange handler's pattern for its ambiguous failures. A retry gets RECONCILE_PENDING; if no Return is recorded, finalize_revocations releases the flag after its 2h stale window, unrevoked, and logs it. On Lambda the claim is now refused with a retryable 503, before the flag is set or the Return posted, when less than 3m35s of invocation time is left: the 3m detached Return, one 30s post-Return write budget (the record of an accepted Return or the clear after a refusal, never both), and 5s for the response. Shipped Lambda timeouts are at least 4m (budgets plus the 30s lambdatimeout margin), so 25s or more remain for the work before the claim. Off Lambda the process outlives the request, the detached Return and writes finish after the 30s request deadline, and that deadline already guards the claim itself, which runs on the request context; no extra check applies. Tests (stub Azure clients, or the real armreservations Return client against a local TLS server; nothing calls Azure): 5xx, 408, timeout, transport error, cut-off response and a 4xx after a retried 5xx keep the flag and answer uncertain; a single 4xx clears it; a Lambda claim with too little time posts nothing; enough time, no deadline, or a non-Lambda runtime proceeds. On real Postgres an uncertain Return keeps the row flagged, a retry posts no second Return, and the sweep leaves the fresh flag and releases it past 2h without revoking. Closes #1529 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_purchases_revoke.go | 59 ++- ...purchases_revoke_claim_integration_test.go | 5 +- .../handler_purchases_revoke_detached_test.go | 12 +- internal/api/handler_purchases_revoke_test.go | 26 +- .../api/handler_purchases_revoke_uncertain.go | 147 +++++++ ...hases_revoke_uncertain_integration_test.go | 104 +++++ ...handler_purchases_revoke_uncertain_test.go | 366 ++++++++++++++++++ internal/purchase/finalize_revocations.go | 5 +- 8 files changed, 690 insertions(+), 34 deletions(-) create mode 100644 internal/api/handler_purchases_revoke_uncertain.go create mode 100644 internal/api/handler_purchases_revoke_uncertain_integration_test.go create mode 100644 internal/api/handler_purchases_revoke_uncertain_test.go diff --git a/internal/api/handler_purchases_revoke.go b/internal/api/handler_purchases_revoke.go index d7ebd387..db258827 100644 --- a/internal/api/handler_purchases_revoke.go +++ b/internal/api/handler_purchases_revoke.go @@ -28,6 +28,7 @@ import ( "time" "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" armreservations "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" @@ -240,10 +241,11 @@ func (h *Handler) loadAndRevokePurchaseHistory(ctx context.Context, req *events. // Partial-success reconciliation (issue #290 Finding #6): if the // revocation_in_flight flag is set but revoked_at is still NULL, Azure // already issued the refund but our DB write failed, or another request - // holds the claim and is posting the Return now (issue #1518). Return 207 - // so the frontend does not retry the Azure call (which would fail with - // "already returned"). The finalize_revocations sweep will reconcile the - // row. + // holds the claim and is posting the Return now (issue #1518), or an + // earlier Return failed without a definite answer and may have been + // accepted (issue #1529). Return 207 so the frontend does not retry the + // Azure call (which would fail with "already returned"). + // The finalize_revocations sweep will reconcile the row. if record.RevocationInFlight { return revocationInFlightResult(), nil } @@ -742,6 +744,13 @@ func (h *Handler) getAzureRevokeFactory() azureRevokeClientFactory { if h.azureRevokeFactory != nil { return *h.azureRevokeFactory } + return defaultAzureRevokeFactory(nil) +} + +// defaultAzureRevokeFactory builds the production clients. returnOpts is nil +// in production; tests point it at a local server to exercise the real Return +// client's pipeline, including returnAttemptPolicy. +func defaultAzureRevokeFactory(returnOpts *arm.ClientOptions) azureRevokeClientFactory { return azureRevokeClientFactory{ newCredential: func() (azcore.TokenCredential, error) { return azidentity.NewDefaultAzureCredential(nil) @@ -750,7 +759,7 @@ func (h *Handler) getAzureRevokeFactory() azureRevokeClientFactory { return armreservations.NewCalculateRefundClient(cred, nil) }, newReturnClient: func(cred azcore.TokenCredential) (azureReturnClient, error) { - return armreservations.NewReturnClient(cred, nil) + return newAzureReturnClient(cred, returnOpts) }, } } @@ -831,8 +840,8 @@ func (h *Handler) callAzureReturn( } // Step 2: Return (post the actual refund request). - if err := postAzureReturn(ctx, returnClient, orderID, reservationID, sessionID, quantity); err != nil { - return nil, h.handleAzureReturnError(ctx, record, err) + if attempts, err := postAzureReturn(ctx, returnClient, orderID, reservationID, sessionID, quantity); err != nil { + return nil, h.handleAzureReturnError(ctx, record, attempts, err) } return h.persistAzureRevocation(ctx, record, calcRefundAmount, calcRefundCurrency) @@ -845,9 +854,14 @@ func (h *Handler) callAzureReturn( // the RI exchange handler does once its claim is held (#790). A request // canceled mid-Return therefore still learns that Azure accepted it and // records it. -func postAzureReturn(reqCtx context.Context, returnClient azureReturnClient, orderID, reservationID, sessionID string, quantity int32) error { +// +// It also reports how many HTTP requests the Return client sent, counted by +// returnAttemptPolicy, so a failure can be judged definite or uncertain +// (issue #1529). +func postAzureReturn(reqCtx context.Context, returnClient azureReturnClient, orderID, reservationID, sessionID string, quantity int32) (int32, error) { ctx, cancel := ctxutil.DetachedWithTimeout(reqCtx, revokeReturnTimeout) defer cancel() + ctx, sends := withReturnAttemptCounter(ctx) _, err := returnClient.Post(ctx, orderID, armreservations.RefundRequest{ Properties: &armreservations.RefundRequestProperties{ ReservationToReturn: &armreservations.ReservationToReturn{ @@ -859,7 +873,7 @@ func postAzureReturn(reqCtx context.Context, returnClient azureReturnClient, ord Scope: toPtr("Reservation"), }, }, nil) - return err + return sends.Load(), err } // revocationInFlightResult is the answer for a row whose revocation another @@ -893,7 +907,14 @@ func revocationInFlightResult() *revokeReconcilePendingResult { // // Because only the claim holder reaches the Return, the ClearRevocationInFlight // in handleAzureReturnError only ever clears this request's own flag. +// +// On Lambda the claim is not taken at all when too little invocation time is +// left to post the Return and record its result (checkRevokeTimeLeft, issue +// #1529). func (h *Handler) claimRevocationInFlight(ctx context.Context, record *config.PurchaseHistoryRecord) (any, error) { + if err := h.checkRevokeTimeLeft(ctx, record.PurchaseID); err != nil { + return nil, err + } flipErr := h.config.FlipPurchaseRevocationInFlight(ctx, record.PurchaseID) switch { case flipErr == nil: @@ -974,18 +995,26 @@ func (h *Handler) azureCalculateRefund(ctx context.Context, calcClient azureCalc return sessionID, calcRefundAmount, calcRefundCurrency, nil } -// handleAzureReturnError clears the in-flight flag (no refund was issued) and -// maps the Return error to the right status: 422 on the 7-day window edge, 400 -// on other client errors, 500 otherwise. The caller holds the claim from -// claimRevocationInFlight, so the flag it clears is its own (issue #1518). +// handleAzureReturnError handles a failed Return. Only a definite refusal +// (isAzureDefiniteRefusal) proves no refund was issued; any other failure +// keeps the in-flight flag and answers that the outcome is uncertain +// (azureReturnUncertain, issue #1529). +// +// After a definite refusal it clears the in-flight flag and maps the error to +// the right status: 422 on the 7-day window edge, 400 on other client errors, +// 500 otherwise. The caller holds the claim from claimRevocationInFlight, so +// the flag it clears is its own (issue #1518). // // The Return ran detached, so by the time it fails the request may be gone; // the clear runs on its own detached revokePersistTimeout budget so it can // still land (issue #1527). -func (h *Handler) handleAzureReturnError(reqCtx context.Context, record *config.PurchaseHistoryRecord, err error) error { +func (h *Handler) handleAzureReturnError(reqCtx context.Context, record *config.PurchaseHistoryRecord, attempts int32, err error) error { + if !isAzureDefiniteRefusal(err, attempts) { + return azureReturnUncertain(record, attempts, err) + } ctx, cancel := ctxutil.DetachedWithTimeout(reqCtx, revokePersistTimeout) defer cancel() - // Azure Return failed. Clear the in-flight flag so the row is not left in + // Azure refused the Return. Clear the in-flight flag so the row is not left in // a permanently sticky state that would mislead the finalize_revocations // sweep into thinking Azure already issued a refund (Finding D, second-wave // CR). Best-effort: log and continue even if the clear fails. diff --git a/internal/api/handler_purchases_revoke_claim_integration_test.go b/internal/api/handler_purchases_revoke_claim_integration_test.go index c9b0da5d..4b4fa69d 100644 --- a/internal/api/handler_purchases_revoke_claim_integration_test.go +++ b/internal/api/handler_purchases_revoke_claim_integration_test.go @@ -249,7 +249,8 @@ func TestRevokeAzure_RequestCanceledAfterReturn_RecordsAndRevokes(t *testing.T) // TestRevokeAzure_RequestCanceledDuringFailedReturn_ClearsFlag covers issue // #1527's error path on real Postgres: the request is canceled while the -// Return is in flight and the Return then fails (Azure answered 503). The +// Return is in flight and Azure then refuses the Return (a definite 400; since +// issue #1529 an ambiguous failure keeps the flag on purpose). The // in-flight clear must still land, or the row stays flagged and unrevocable // until the sweep releases it 2h later. Before the fix the clear ran on the // canceled request context and pgx refused it. @@ -271,7 +272,7 @@ func TestRevokeAzure_RequestCanceledDuringFailedReturn_ClearsFlag(t *testing.T) parties: 1, ready: make(chan struct{}), } - ret := &cancelAfterReturnClient{cancel: cancelReq, err: &azcore.ResponseError{StatusCode: 503}} + ret := &cancelAfterReturnClient{cancel: cancelReq, err: &azcore.ResponseError{StatusCode: 400, ErrorCode: "InvalidReservationID"}} h := &Handler{config: store, azureRevokeFactory: &azureRevokeClientFactory{ newCredential: func() (azcore.TokenCredential, error) { return nil, nil }, newCalculateRefundClient: func(azcore.TokenCredential) (azureCalculateRefundClient, error) { return calc, nil }, diff --git a/internal/api/handler_purchases_revoke_detached_test.go b/internal/api/handler_purchases_revoke_detached_test.go index 421163d2..6caac1c4 100644 --- a/internal/api/handler_purchases_revoke_detached_test.go +++ b/internal/api/handler_purchases_revoke_detached_test.go @@ -37,10 +37,11 @@ func TestRevokePersistScheduleFitsBudget(t *testing.T) { assert.LessOrEqual(t, schedule, revokePersistTimeout) } -// A Return that fails after the request was canceled under it must still +// A Return Azure refuses after the request was canceled under it must still // clear this request's in-flight flag, on a live context: before the fix the // clear ran on the dead request context, failed, and left the row flagged -// until the sweep released it 2h later. +// until the sweep released it 2h later. The refusal is a definite 4xx: since +// issue #1529 an ambiguous failure (a 5xx, say) keeps the flag on purpose. func TestCallAzureReturn_RequestCanceledDuringFailedReturn_ClearsInFlight(t *testing.T) { t.Parallel() reqCtx, cancel := context.WithCancel(context.Background()) @@ -53,13 +54,14 @@ func TestCallAzureReturn_RequestCanceledDuringFailedReturn_ClearsInFlight(t *tes mockStore.On("ClearRevocationInFlight", mock.Anything, r.PurchaseID).Return(nil).Once() calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-cancel-fail"} - returnClient := &cancelAfterReturnClient{cancel: cancel, err: &azcore.ResponseError{StatusCode: 503}} + returnClient := &cancelAfterReturnClient{cancel: cancel, err: &azcore.ResponseError{StatusCode: 400, ErrorCode: "InvalidReservationID"}} h := &Handler{config: store} start := time.Now() _, err := h.callAzureReturn(reqCtx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) - _, isClientErr := IsClientError(err) - assert.False(t, isClientErr, "a 503 from Azure is a server-side failure") + ce, isClientErr := IsClientError(err) + require.True(t, isClientErr, "a 400 from Azure is a client error") + assert.Equal(t, 400, ce.code) returnClient.assertDetached(t, start) require.ErrorIs(t, reqCtx.Err(), context.Canceled) diff --git a/internal/api/handler_purchases_revoke_test.go b/internal/api/handler_purchases_revoke_test.go index ba45c9ce..d75c182f 100644 --- a/internal/api/handler_purchases_revoke_test.go +++ b/internal/api/handler_purchases_revoke_test.go @@ -320,9 +320,11 @@ func TestRevokePurchase_AzureReturnClientError(t *testing.T) { h := &Handler{config: mockStore} _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) - // 500 is not a client error -- expect wrapped error, not ClientError. - _, isClient := IsClientError(err) - assert.False(t, isClient, "server-side Azure errors should not be wrapped as 4xx ClientError") + // A server-side failure is not a 4xx: Azure may have taken the Return, so + // the answer is the uncertain 500 (issue #1529). + ce, isClient := IsClientError(err) + require.True(t, isClient, "want the uncertain-outcome ClientError, got %v", err) + assert.Equal(t, 500, ce.code, "server-side Azure errors must not be reported as 4xx") } // TestRevokePurchase_UsesStampedWindow asserts the window check reads @@ -1462,10 +1464,11 @@ func TestCallAzureReturn_RefundPolicyViolatedReturns422WindowEdge(t *testing.T) // --- Finding D: revocation_in_flight sticky on Azure error paths (second-wave CR) --- -// TestCallAzureReturn_TransientError_ClearsInFlight verifies that when the -// Azure Return call fails with a transient (non-window-edge, non-client) error, -// callAzureReturn calls ClearRevocationInFlight so the row is not left sticky. -func TestCallAzureReturn_TransientError_ClearsInFlight(t *testing.T) { +// TestCallAzureReturn_TransientError_KeepsInFlight verifies that when the +// Azure Return call fails with a transport error, which says nothing about +// whether Azure took the Return, the in-flight flag is kept (issue #1529) and +// the answer is the uncertain 500. Before #1529 the flag was cleared. +func TestCallAzureReturn_TransientError_KeepsInFlight(t *testing.T) { t.Parallel() ctx := context.Background() mockStore := new(MockConfigStore) @@ -1475,14 +1478,15 @@ func TestCallAzureReturn_TransientError_ClearsInFlight(t *testing.T) { returnClient := &stubReturnClient{err: errors.New("dial tcp: connection refused")} r := armReservationRecord() - mockStore.On("ClearRevocationInFlight", mock.Anything, r.PurchaseID).Return(nil).Once() h := &Handler{config: mockStore} _, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) require.Error(t, err) - // Must not be a ClientError -- transient errors surface as a 500 from the caller. - _, isClientErr := IsClientError(err) - assert.False(t, isClientErr, "transient Azure error must not be a ClientError") + ce, isClientErr := IsClientError(err) + require.True(t, isClientErr, "want the uncertain-outcome ClientError, got %v", err) + assert.Equal(t, 500, ce.code) + assert.Contains(t, ce.message, "verify the reservation state in the Azure portal") + mockStore.AssertNotCalled(t, "ClearRevocationInFlight") mockStore.AssertExpectations(t) } diff --git a/internal/api/handler_purchases_revoke_uncertain.go b/internal/api/handler_purchases_revoke_uncertain.go new file mode 100644 index 00000000..17f65ba7 --- /dev/null +++ b/internal/api/handler_purchases_revoke_uncertain.go @@ -0,0 +1,147 @@ +package api + +// handler_purchases_revoke_uncertain.go holds the two guards issue #1529 adds +// around the Azure Return in the revoke handler: +// +// - a Return that fails without a definite answer from Azure keeps the +// in-flight flag and tells the user the outcome is uncertain, as the RI +// exchange handler does for its ambiguous provider failures; +// - on Lambda, the claim is refused when too little invocation time is left +// to post the Return and record its result. + +import ( + "context" + "errors" + "net/http" + "sync/atomic" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + armreservations "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// revokeResponseMargin is the invocation time kept back, past the detached +// Return and the write that follows it, for building and returning the +// response. Nothing after that write talks to Azure or the database, so a few +// seconds is plenty; the 30s lambdatimeout.Margin also has to cover the work +// before the claim, which by the time of this check has already run. +const revokeResponseMargin = 5 * time.Second + +// revokeClaimMinRemaining is the least invocation time a Lambda revoke must +// have left to take the in-flight claim (issue #1529). Past the claim the +// handler runs the detached Return (revokeReturnTimeout), then exactly one of +// the writes recording an accepted Return or the in-flight clear after a +// refused one (each bounded by revokePersistTimeout; an uncertain failure +// writes nothing), then the response. Lambda ends the invocation at its +// timeout whatever a context says, so a claim taken with less time left could +// be cut off mid-Return or before the result is recorded. 3m35s with the +// current budgets. +const revokeClaimMinRemaining = revokeReturnTimeout + revokePersistTimeout + revokeResponseMargin + +// errRevokeTooLittleTime is the message for a claim refused for lack of time. +const errRevokeTooLittleTime = "not enough time was left in this request to return the reservation safely; " + + "nothing was sent to Azure and the purchase is unchanged, so retry the revoke" + +// errRevokeReturnUncertain mirrors the RI exchange handler's wording for an +// ambiguous provider failure (executeAzureExchange). +const errRevokeReturnUncertain = "Azure reservation return failed and may already have been accepted by Azure; " + + "verify the reservation state in the Azure portal before retrying" + +// checkRevokeTimeLeft refuses the claim on Lambda when the invocation has less +// than revokeClaimMinRemaining left. It runs before the claim, so a refusal +// leaves no flag behind and posts no Return; 503 says the request can simply +// be retried. +// +// Only Lambda needs it: elsewhere the process outlives the request, so the +// detached Return and writes finish after the request's 30s deadline +// (internal/server/http.go), and that deadline already guards the claim +// itself, which runs on the request context. +func (h *Handler) checkRevokeTimeLeft(ctx context.Context, purchaseID string) error { + if !h.isLambda { + return nil + } + deadline, ok := ctx.Deadline() + if !ok { + return nil + } + if left := time.Until(deadline); left < revokeClaimMinRemaining { + logging.Warnf("revoke azure: purchase_id=%s refused before the claim: %s of invocation time left, need %s (no Return posted)", + purchaseID, left.Round(time.Millisecond), revokeClaimMinRemaining) + return NewClientError(http.StatusServiceUnavailable, errRevokeTooLittleTime) + } + return nil +} + +// returnAttemptsKey carries a *atomic.Int32 that returnAttemptPolicy bumps once +// per HTTP request the Return client sends, retries included. +type returnAttemptsKey struct{} + +// withReturnAttemptCounter returns ctx carrying a fresh send counter. +func withReturnAttemptCounter(ctx context.Context) (context.Context, *atomic.Int32) { + n := new(atomic.Int32) + return context.WithValue(ctx, returnAttemptsKey{}, n), n +} + +// returnAttemptPolicy counts the sends of a Return. azcore retries a POST on +// transport errors and on 408, 429 and 5xx, so a 4xx that ends the call may +// answer a retry of a try Azure already accepted (a connection reset after +// the 202, say). Only a 4xx from the one and only send is a definite refusal. +// It sits among the per-retry policies, after the bearer-token policy, so it +// counts only requests that reach the transport. +type returnAttemptPolicy struct{} + +func (returnAttemptPolicy) Do(req *policy.Request) (*http.Response, error) { + if n, ok := req.Raw().Context().Value(returnAttemptsKey{}).(*atomic.Int32); ok { + n.Add(1) + } + return req.Next() +} + +// newAzureReturnClient builds the armreservations Return client with +// returnAttemptPolicy installed. opts may be nil; it is not modified. +func newAzureReturnClient(cred azcore.TokenCredential, opts *arm.ClientOptions) (*armreservations.ReturnClient, error) { + o := arm.ClientOptions{} + if opts != nil { + o = *opts + } + o.PerRetryPolicies = append(append([]policy.Policy(nil), o.PerRetryPolicies...), returnAttemptPolicy{}) + return armreservations.NewReturnClient(cred, &o) +} + +// isAzureDefiniteRefusal reports whether a failed Return is known not to have +// been accepted: Azure answered the only request sent with a 4xx other than +// 408 Request Timeout. Everything else (a transport error, a 5xx, the +// revokeReturnTimeout deadline, a response that could not be read, a 4xx +// after a retry, or any status other than the 202 the SDK expects) leaves +// open whether Azure took the Return. +// +// attempts is the send count from returnAttemptPolicy. A client built without +// the policy reports 0 and is judged on the status alone. +func isAzureDefiniteRefusal(err error, attempts int32) bool { + if attempts > 1 { + return false + } + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) { + return false + } + code := respErr.StatusCode + return code >= 400 && code < 500 && code != http.StatusRequestTimeout +} + +// azureReturnUncertain handles a Return that failed without a definite answer +// (issue #1529). The in-flight flag stays set, so no request can post a +// second Return for this purchase, and the user is told to check the Azure +// portal. If no Return gets recorded, the finalize_revocations sweep clears +// the flag after its 2h stale window and logs the row for an operator. +func azureReturnUncertain(record *config.PurchaseHistoryRecord, attempts int32, err error) error { + logging.Errorf("revoke azure: purchase_id=%s account_id=%s Return failed without a definite answer after %d send(s), outcome uncertain: %v. "+ + "Kept the in-flight flag so no second Return is posted; finalize_revocations releases it after 2h if no Return is recorded. "+ + "If Azure did accept the Return, reconcile the row by hand", + record.PurchaseID, record.AccountID, attempts, err) + return NewClientError(http.StatusInternalServerError, errRevokeReturnUncertain) +} diff --git a/internal/api/handler_purchases_revoke_uncertain_integration_test.go b/internal/api/handler_purchases_revoke_uncertain_integration_test.go new file mode 100644 index 00000000..53ab5279 --- /dev/null +++ b/internal/api/handler_purchases_revoke_uncertain_integration_test.go @@ -0,0 +1,104 @@ +//go:build integration + +package api + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/LeanerCloud/cloud-commitments-platform/internal/purchase" + "github.com/aws/aws-lambda-go/events" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRevokeAzure_UncertainReturn_FlagKeptUntilSweepReleases covers issue +// #1529 on real Postgres, from the revoke request to the sweep. Azure answers +// the Return with a 503, which does not say whether it took the Return: +// +// - the row stays flagged in flight with no recorded Return and not revoked, +// and the user is told the outcome is uncertain (before the fix the flag +// was cleared and the answer was a flat failure); +// - a retry is answered RECONCILE_PENDING and posts no second Return; +// - the finalize_revocations sweep leaves the fresh flag alone, and once it +// is older than the 2h stale window releases it without marking the +// purchase revoked. +// +// Stub Azure clients; nothing calls Azure. +func TestRevokeAzure_UncertainReturn_FlagKeptUntilSweepReleases(t *testing.T) { + pg, store := setupSubmitDB(t) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + record := withinWindowRecord("/providers/Microsoft.Capacity/reservationOrders/order-1529/reservations/res-1529") + record.AccountID = "acct-1529" + record.Region = "eastus" + record.ResourceType = "Standard_D2s_v3" + require.NoError(t, store.SavePurchaseHistory(ctx, record)) + + calc := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-1529"} + ret := &stubReturnClient{err: &azcore.ResponseError{StatusCode: 503, ErrorCode: "ServiceUnavailable"}} + h := &Handler{config: store, azureRevokeFactory: &azureRevokeClientFactory{ + newCredential: func() (azcore.TokenCredential, error) { return nil, nil }, + newCalculateRefundClient: func(azcore.TokenCredential) (azureCalculateRefundClient, error) { return calc, nil }, + newReturnClient: func(azcore.TokenCredential) (azureReturnClient, error) { return ret, nil }, + }} + body, err := json.Marshal(confirmedQuote(10, "USD")) + require.NoError(t, err) + req := &events.LambdaFunctionURLRequest{Body: string(body)} + + type rowState struct { + returnPostedAt, revokedAt *time.Time + inFlight bool + } + load := func() rowState { + t.Helper() + var s rowState + require.NoError(t, pg.DB.Pool().QueryRow(ctx, ` + SELECT return_posted_at, revoked_at, revocation_in_flight + FROM purchase_history WHERE purchase_id = $1`, record.PurchaseID). + Scan(&s.returnPostedAt, &s.revokedAt, &s.inFlight)) + return s + } + + _, err = h.loadAndRevokePurchaseHistory(ctx, req, revokeAdminSession(), record.PurchaseID) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "want the uncertain-outcome ClientError, got %v", err) + assert.Equal(t, 500, ce.code) + assert.Equal(t, errRevokeReturnUncertain, ce.message) + got := load() + assert.True(t, got.inFlight, "an uncertain Return must keep the in-flight flag") + assert.Nil(t, got.returnPostedAt, "an uncertain Return is not recorded as accepted") + assert.Nil(t, got.revokedAt, "an uncertain Return does not revoke the purchase") + + result, err := h.loadAndRevokePurchaseHistory(ctx, req, revokeAdminSession(), record.PurchaseID) + require.NoError(t, err) + pending, ok := result.(*revokeReconcilePendingResult) + require.True(t, ok, "want RECONCILE_PENDING on retry, got %T %+v", result, result) + assert.Equal(t, "RECONCILE_PENDING", pending.Code) + assert.Equal(t, 1, ret.calls, "the retry must not post a second Return") + + sweeper := purchase.NewManager(purchase.ManagerConfig{ConfigStore: store}) + _, err = sweeper.FinalizeInFlightRevocations(ctx) + require.NoError(t, err) + got = load() + assert.True(t, got.inFlight, "the sweep leaves a fresh uncertain flag alone") + assert.Nil(t, got.revokedAt, "the sweep must not revoke a purchase with no recorded Return") + + _, err = pg.DB.Pool().Exec(ctx, ` + UPDATE purchase_history + SET revocation_in_flight_at = now() - interval '2 hours 1 minute' + WHERE purchase_id = $1`, record.PurchaseID) + require.NoError(t, err) + res, err := sweeper.FinalizeInFlightRevocations(ctx) + require.NoError(t, err) + assert.GreaterOrEqual(t, res.Released, 1) + got = load() + assert.False(t, got.inFlight, "past the stale window the sweep releases the flag") + assert.Nil(t, got.revokedAt, "a released row is not revoked") + assert.Nil(t, got.returnPostedAt) +} diff --git a/internal/api/handler_purchases_revoke_uncertain_test.go b/internal/api/handler_purchases_revoke_uncertain_test.go new file mode 100644 index 00000000..344dfafa --- /dev/null +++ b/internal/api/handler_purchases_revoke_uncertain_test.go @@ -0,0 +1,366 @@ +package api + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/LeanerCloud/cloud-commitments-platform/internal/testutil/lambdatimeout" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Issue #1529. Stub Azure clients, or the real armreservations Return client +// pointed at a local TLS server, and a mock store; nothing here calls Azure. + +// assertUncertainReturn checks the uncertain-outcome answer: a 500 telling the +// user to check the Azure portal, the in-flight flag kept, and nothing +// recorded or revoked. +func assertUncertainReturn(t *testing.T, mockStore *MockConfigStore, err error) { + t.Helper() + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "want the uncertain-outcome ClientError, got %v", err) + assert.Equal(t, 500, ce.code) + assert.Equal(t, errRevokeReturnUncertain, ce.message) + mockStore.AssertCalled(t, "FlipPurchaseRevocationInFlight") + mockStore.AssertNotCalled(t, "ClearRevocationInFlight") + mockStore.AssertNotCalled(t, "MarkRevocationReturnPosted") + mockStore.AssertNotCalled(t, "MarkPurchaseRevoked") +} + +// A Return that fails without a definite answer may have been accepted by +// Azure, so the flag that stops a second Return must stay set. Before #1529 +// every one of these cleared the flag and answered a flat failure, so a +// refunded purchase could read as active and failed. +func TestCallAzureReturn_AmbiguousFailure_KeepsFlagAndAnswersUncertain(t *testing.T) { + t.Parallel() + cases := map[string]error{ + "500": &azcore.ResponseError{StatusCode: 500, ErrorCode: "InternalServerError"}, + "502": &azcore.ResponseError{StatusCode: 502}, + "503": &azcore.ResponseError{StatusCode: 503}, + "504": &azcore.ResponseError{StatusCode: 504, ErrorCode: "GatewayTimeout"}, + "408 request timeout": &azcore.ResponseError{StatusCode: 408}, + "unexpected 200": &azcore.ResponseError{StatusCode: 200}, + "Return deadline": context.DeadlineExceeded, + "Return deadline wrapped": &url.Error{Op: "Post", URL: "https://management.azure.com/x", Err: context.DeadlineExceeded}, + "transport error": errors.New("dial tcp: connection reset by peer"), + "response cut off": fmt.Errorf("reading response body: %w", io.ErrUnexpectedEOF), + } + for name, returnErr := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + mockStore := new(MockConfigStore) + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-uncertain"} + returnClient := &stubReturnClient{err: returnErr} + h := &Handler{config: mockStore} + + _, err := h.callAzureReturn(context.Background(), calcClient, returnClient, armReservationRecord(), "order-abc", "res-xyz", confirmedQuote(10, "USD")) + assertUncertainReturn(t, mockStore, err) + assert.Equal(t, 1, returnClient.calls, "exactly one Return") + }) + } +} + +// A definite refusal (Azure answered the only request with a 4xx) proves no +// refund was issued, so the flag is cleared and the purchase can be revoked +// again, as before #1529. +func TestCallAzureReturn_DefiniteRefusal_ClearsFlag(t *testing.T) { + t.Parallel() + cases := map[string]struct { + err error + code int + }{ + "400": {&azcore.ResponseError{StatusCode: 400, ErrorCode: "InvalidReservationID"}, 400}, + "403": {&azcore.ResponseError{StatusCode: 403, ErrorCode: "AuthorizationFailed"}, 400}, + "409": {&azcore.ResponseError{StatusCode: 409, ErrorCode: "ReservationAlreadyReturned"}, 400}, + "window edge": {&azcore.ResponseError{StatusCode: 400, ErrorCode: "RefundPolicyViolated"}, 422}, + "401": {&azcore.ResponseError{StatusCode: 401, ErrorCode: "InvalidAuthenticationToken"}, 0}, + "429": {&azcore.ResponseError{StatusCode: 429, ErrorCode: "TooManyRequests"}, 0}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + mockStore := new(MockConfigStore) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + r := armReservationRecord() + mockStore.On("ClearRevocationInFlight", mock.Anything, r.PurchaseID).Return(nil).Once() + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-refused"} + h := &Handler{config: mockStore} + + _, err := h.callAzureReturn(context.Background(), calcClient, &stubReturnClient{err: tc.err}, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) + require.Error(t, err) + ce, isClient := IsClientError(err) + if tc.code == 0 { + assert.False(t, isClient, "a refusal without a mapped status stays a plain 500, got %v", err) + } else { + require.True(t, isClient, "got %v", err) + assert.Equal(t, tc.code, ce.code) + } + assert.NotEqual(t, errRevokeReturnUncertain, err.Error(), "a definite refusal is not uncertain") + mockStore.AssertNotCalled(t, "MarkRevocationReturnPosted") + mockStore.AssertNotCalled(t, "MarkPurchaseRevoked") + }) + } +} + +func TestIsAzureDefiniteRefusal_AttemptsAndStatus(t *testing.T) { + t.Parallel() + refused := &azcore.ResponseError{StatusCode: 409} + assert.True(t, isAzureDefiniteRefusal(refused, 0), "a client without the counting policy is judged on the status") + assert.True(t, isAzureDefiniteRefusal(refused, 1)) + assert.True(t, isAzureDefiniteRefusal(fmt.Errorf("wrapped: %w", refused), 1)) + assert.False(t, isAzureDefiniteRefusal(refused, 2), "a 4xx after a retry may answer a retry of an accepted try") + assert.True(t, isAzureDefiniteRefusal(&azcore.ResponseError{StatusCode: 499}, 1)) + assert.False(t, isAzureDefiniteRefusal(&azcore.ResponseError{StatusCode: 500}, 1)) + assert.False(t, isAzureDefiniteRefusal(&azcore.ResponseError{StatusCode: 399}, 1)) + assert.False(t, isAzureDefiniteRefusal(context.Canceled, 1)) +} + +// fakeTokenCredential satisfies the bearer-token policy without Entra ID. +type fakeTokenCredential struct{} + +func (fakeTokenCredential) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { + return azcore.AccessToken{Token: "fake-token", ExpiresOn: time.Now().Add(time.Hour)}, nil +} + +// returnServer stands in for ARM's Return endpoint. It answers the n-th +// request with answers[n] (the last one repeats) and counts requests. +type returnServer struct { + *httptest.Server + requests atomic.Int32 +} + +func newReturnServer(t *testing.T, answers ...func(http.ResponseWriter)) *returnServer { + t.Helper() + s := &returnServer{} + s.Server = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + n := int(s.requests.Add(1)) - 1 + answers[min(n, len(answers)-1)](w) + })) + t.Cleanup(s.Close) + return s +} + +// returnClientFor builds the production Return client (the default factory's, +// with returnAttemptPolicy) against srv, with millisecond retry delays. +func returnClientFor(t *testing.T, srv *returnServer) azureReturnClient { + t.Helper() + opts := &arm.ClientOptions{ + ClientOptions: policy.ClientOptions{ + Cloud: cloud.Configuration{Services: map[cloud.ServiceName]cloud.ServiceConfiguration{ + cloud.ResourceManager: {Endpoint: srv.URL, Audience: "https://management.core.windows.net/"}, + }}, + Retry: policy.RetryOptions{RetryDelay: time.Millisecond, MaxRetryDelay: 5 * time.Millisecond}, + Transport: srv.Client(), + }, + DisableRPRegistration: true, + } + client, err := defaultAzureRevokeFactory(opts).newReturnClient(fakeTokenCredential{}) + require.NoError(t, err) + return client +} + +func azureError(status int, code string) func(http.ResponseWriter) { + return func(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("x-ms-error-code", code) + w.WriteHeader(status) + _, _ = fmt.Fprintf(w, `{"error":{"code":%q,"message":"stub"}}`, code) + } +} + +// truncatedAccepted answers 202 but closes the connection partway through the +// body, so the client cannot read Azure's answer to a Return Azure accepted. +func truncatedAccepted(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Content-Length", "500") + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{"id":"`) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + if hj, ok := w.(http.Hijacker); ok { + if conn, _, err := hj.Hijack(); err == nil { + _ = conn.Close() + } + } +} + +// The real Return client retries a 5xx, and the retry can meet Azure's +// refusal of a Return it already took. That final 4xx is not a definite +// refusal: the flag must stay. Nor does a 202 whose body was cut off prove +// anything. Before #1529 the flag was cleared in every case. +func TestCallAzureReturn_RealClient_AmbiguousKeepsFlag(t *testing.T) { + t.Parallel() + cases := map[string]struct { + answers []func(http.ResponseWriter) + requests int32 + }{ + "503 then 409": {[]func(http.ResponseWriter){azureError(503, "ServiceUnavailable"), azureError(409, "ReservationAlreadyReturned")}, 2}, + "504 then 400 RefundPol.": {[]func(http.ResponseWriter){azureError(504, "GatewayTimeout"), azureError(400, "RefundPolicyViolated")}, 2}, + "503 on every try": {[]func(http.ResponseWriter){azureError(503, "ServiceUnavailable")}, 4}, + "202 with its body cut off": {[]func(http.ResponseWriter){truncatedAccepted, azureError(400, "ReservationAlreadyReturned")}, 1}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + srv := newReturnServer(t, tc.answers...) + mockStore := new(MockConfigStore) + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-retry"} + h := &Handler{config: mockStore} + + _, err := h.callAzureReturn(context.Background(), calcClient, returnClientFor(t, srv), armReservationRecord(), "order-abc", "res-xyz", confirmedQuote(10, "USD")) + assertUncertainReturn(t, mockStore, err) + assert.Equal(t, tc.requests, srv.requests.Load(), "requests the client sent") + }) + } +} + +// A 4xx on the only request the real client sent is a definite refusal: the +// counting policy sees one send and the flag is cleared. +func TestCallAzureReturn_RealClient_Single4xxClearsFlag(t *testing.T) { + t.Parallel() + srv := newReturnServer(t, azureError(400, "InvalidReservationID")) + mockStore := new(MockConfigStore) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + r := armReservationRecord() + mockStore.On("ClearRevocationInFlight", mock.Anything, r.PurchaseID).Return(nil).Once() + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-single"} + h := &Handler{config: mockStore} + + _, err := h.callAzureReturn(context.Background(), calcClient, returnClientFor(t, srv), r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "got %v", err) + assert.Equal(t, 400, ce.code) + assert.Equal(t, int32(1), srv.requests.Load()) + mockStore.AssertNotCalled(t, "MarkPurchaseRevoked") +} + +// The real client's 202 still revokes; the counting policy changes nothing on +// the success path. +func TestCallAzureReturn_RealClient_AcceptedRevokes(t *testing.T) { + t.Parallel() + srv := newReturnServer(t, func(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{}`) + }) + mockStore := new(MockConfigStore) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + r := armReservationRecord() + mockStore.On("MarkRevocationReturnPosted", mock.Anything, r.PurchaseID).Return(nil).Once() + mockStore.On("MarkPurchaseRevoked", mock.Anything, r.PurchaseID, mock.AnythingOfType("time.Time"), + "direct-api", "", mock.Anything, mock.Anything).Return(nil).Once() + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-ok"} + h := &Handler{config: mockStore} + + result, err := h.callAzureReturn(context.Background(), calcClient, returnClientFor(t, srv), r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) + require.NoError(t, err) + rr, ok := result.(*revokePurchaseResult) + require.True(t, ok, "got %T %+v", result, result) + assert.Equal(t, "revoked", rr.Status) + assert.Equal(t, int32(1), srv.requests.Load()) +} + +// The claim threshold is the detached Return plus one post-Return write budget +// plus the response margin, and it must leave every shipped Lambda timeout +// (at least those budgets plus lambdatimeout.Margin) time for the work before +// the claim, or every Lambda revoke would be refused. +func TestRevokeClaimMinRemaining_Budget(t *testing.T) { + t.Parallel() + assert.Equal(t, revokeReturnTimeout+revokePersistTimeout+revokeResponseMargin, revokeClaimMinRemaining) + assert.Equal(t, 3*time.Minute+35*time.Second, revokeClaimMinRemaining) + assert.Less(t, revokeClaimMinRemaining, revokeReturnTimeout+revokePersistTimeout+lambdatimeout.Margin) +} + +// On Lambda, a revoke with too little invocation time left must not take the +// claim or post the Return. Before #1529 it did both, and Lambda could end the +// invocation mid-Return. +func TestCallAzureReturn_LambdaTooLittleTime_RefusesBeforeClaim(t *testing.T) { + t.Parallel() + for name, left := range map[string]time.Duration{ + "one minute": time.Minute, + "just under the needed": revokeClaimMinRemaining - time.Second, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), left) + defer cancel() + mockStore := new(MockConfigStore) + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-late"} + returnClient := &stubReturnClient{} + h := &Handler{config: mockStore, isLambda: true} + + result, err := h.callAzureReturn(ctx, calcClient, returnClient, armReservationRecord(), "order-abc", "res-xyz", confirmedQuote(10, "USD")) + require.Error(t, err) + assert.Nil(t, result) + ce, ok := IsClientError(err) + require.True(t, ok, "got %v", err) + assert.Equal(t, http.StatusServiceUnavailable, ce.code) + assert.Equal(t, errRevokeTooLittleTime, ce.message) + assert.Zero(t, returnClient.calls, "no Return may be posted") + mockStore.AssertNotCalled(t, "FlipPurchaseRevocationInFlight") + mockStore.AssertNotCalled(t, "ClearRevocationInFlight") + mockStore.AssertNotCalled(t, "MarkRevocationReturnPosted") + mockStore.AssertNotCalled(t, "MarkPurchaseRevoked") + }) + } +} + +// The check must not refuse a revoke that has the time, nor apply off Lambda, +// where the process outlives the request's 30s deadline and the detached +// Return finishes anyway. +func TestCallAzureReturn_EnoughTimeOrNotLambda_Proceeds(t *testing.T) { + t.Parallel() + cases := map[string]struct { + left time.Duration // 0: no deadline + isLambda bool + }{ + "lambda with time to spare": {revokeClaimMinRemaining + 30*time.Second, true}, + "lambda without a deadline": {0, true}, + "http server's 30s deadline": {20 * time.Second, false}, + "non-lambda with little time": {time.Second, false}, + "non-lambda without a deadline": {0, false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + if tc.left > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, tc.left) + defer cancel() + } + mockStore := new(MockConfigStore) + r := armReservationRecord() + mockStore.On("MarkPurchaseRevoked", mock.Anything, r.PurchaseID, mock.AnythingOfType("time.Time"), + "direct-api", "", mock.Anything, mock.Anything).Return(nil).Once() + calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-time"} + returnClient := &stubReturnClient{} + h := &Handler{config: mockStore, isLambda: tc.isLambda} + + result, err := h.callAzureReturn(ctx, calcClient, returnClient, r, "order-abc", "res-xyz", confirmedQuote(10, "USD")) + require.NoError(t, err) + rr, ok := result.(*revokePurchaseResult) + require.True(t, ok, "got %T %+v", result, result) + assert.Equal(t, "revoked", rr.Status) + assert.Equal(t, 1, returnClient.calls) + mockStore.AssertCalled(t, "FlipPurchaseRevocationInFlight") + }) + } +} diff --git a/internal/purchase/finalize_revocations.go b/internal/purchase/finalize_revocations.go index 8fbe7e42..b49c7511 100644 --- a/internal/purchase/finalize_revocations.go +++ b/internal/purchase/finalize_revocations.go @@ -33,7 +33,10 @@ type FinalizeResult struct { // (handleHTTPRequest in internal/server/http.go), Lambda ends an invocation at // its function timeout, and past the flip the revoke handler runs the Return // and the writes recording it detached under their own budgets (3m and 30s, -// issue #1527). 2h is a wide margin over all of these. Correctness does not +// issue #1527). 2h is a wide margin over all of these. It is also how a row +// whose Return failed without a definite answer gets released: the revoke +// handler keeps that row flagged and records no Return (issue #1529), so the +// sweep releases it here and logs it for an operator. Correctness does not // depend on it (issue #1525): a Return recorded after the release is still // finalized, flag or not, and if the user revokes again while the first // Return is in flight, Azure refuses a second Return of the same reservation, From 8ecf506ad93e84c8b2b23eb084affc3d0bc2616a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 21:25:15 +0000 Subject: [PATCH 2/2] test(revoke): fail the late-claim test cleanly instead of panicking on a regression Refs #1529 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_purchases_revoke_uncertain_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/api/handler_purchases_revoke_uncertain_test.go b/internal/api/handler_purchases_revoke_uncertain_test.go index 344dfafa..ecdfebb5 100644 --- a/internal/api/handler_purchases_revoke_uncertain_test.go +++ b/internal/api/handler_purchases_revoke_uncertain_test.go @@ -302,6 +302,10 @@ func TestCallAzureReturn_LambdaTooLittleTime_RefusesBeforeClaim(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), left) defer cancel() mockStore := new(MockConfigStore) + // Registered so a regression fails the assertions below instead + // of panicking the test binary on an unexpected call. + mockStore.On("MarkPurchaseRevoked", mock.Anything, mock.Anything, mock.Anything, + mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() calcClient := &stubCalcRefundClientWithAmount{amount: 10.0, currency: "USD", sessID: "s-late"} returnClient := &stubReturnClient{} h := &Handler{config: mockStore, isLambda: true}