fix(ri-exchange): run ExecuteExchange outside the request context

internal/api/handler.go

@@ -129,7 +129,7 @@ type Handler struct {
 	commitmentOpts CommitmentOptsInterface
 
 	// executeExchangeFn is the RI exchange execution function injected by tests.
-	// When nil (the production default), executeApprovedExchange calls
+	// When nil (the production default), awsExchangeExecutor returns
 	// exchange.ExecuteExchange directly. Tests inject a stub so the handler
 	// can be exercised end-to-end without live AWS credentials or a real RI.
 	executeExchangeFn func(ctx context.Context, req exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error)

internal/api/handler_ri_exchange.go

@@ -28,6 +28,7 @@ import (
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/auth"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/credentials"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/ctxutil"
 )
 
 // reshapeEC2Client is the narrow slice of the EC2 client that
@@ -1234,12 +1235,17 @@ func (h *Handler) executeAzureExchange(ctx context.Context, req *events.LambdaFu
 		return nil, err
 	}
 
-	result, err := client.ExecuteExchange(ctx, preview.SessionID)
+	// Past the claim the exchange may commit at Azure; a cancelled request must
+	// not abort the submit or its long-running-operation poll (#790).
+	execCtx, cancelExec := ctxutil.DetachedWithTimeout(ctx, riExchangeExecutionTimeout)
+	defer cancelExec()
+	result, err := client.ExecuteExchange(execCtx, preview.SessionID)
 	if err != nil {
 		logging.Errorf("azure exchange execution failed: %v", err)
 		// Non-4xx failures here are ambiguous: BeginPost may already have
-		// submitted the exchange, and a ctx cancellation mid-poll looks
-		// identical to one that never reached Azure. Say so rather than
+		// submitted the exchange, and exhausting the riExchangeExecutionTimeout
+		// budget mid-poll looks identical to a call that never reached Azure
+		// (a request cancel no longer reaches execCtx). Say so rather than
 		// reporting a flat failure that invites the retry this claim now
 		// refuses (#1642).
 		return nil, mapAzureExchangeError(
@@ -1918,7 +1924,11 @@ func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctio
 		return nil, err
 	}
 
-	exchangeID, quote, err := exchange.ExecuteExchange(ctx, exchange.ExchangeExecuteRequest{
+	// Past the claim the exchange may commit at AWS; a cancelled request must
+	// not abort it midway (#790).
+	execCtx, cancelExec := ctxutil.DetachedWithTimeout(ctx, riExchangeExecutionTimeout)
+	defer cancelExec()
+	exchangeID, quote, err := h.awsExchangeExecutor()(execCtx, exchange.ExchangeExecuteRequest{
 		Region:           region,
 		ReservedIDs:      body.RIIDs,
 		Targets:          toExchangeTargets(body.Targets),
@@ -2302,6 +2312,9 @@ func (h *Handler) approveRIExchangeViaToken(ctx context.Context, req *events.Lam
 // stampRIExchangeApprover records session.Email as approved_by. Best-effort: the
 // exchange already executed, so a stamp failure is logged, not surfaced.
 func (h *Handler) stampRIExchangeApprover(ctx context.Context, id string, session *Session) {
+	// Runs after money moved, so the audit write must not die with the request (#790).
+	ctx, cancel := ctxutil.DetachedWithTimeout(ctx, riExchangeTerminalSaveTimeout)
+	defer cancel()
 	if err := h.config.StampRIExchangeApprovedBy(ctx, id, session.Email); err != nil {
 		logging.Errorf("failed to stamp approved_by on exchange %s: %v", id, err)
 	}
@@ -2566,7 +2579,16 @@ func (h *Handler) validateExchangeApproval(ctx context.Context, id, token string
 }
 
 // failExchange marks an exchange as failed, logging if the DB write also fails.
+//
+// Every caller runs after the pending -> processing transition committed, so
+// the write is detached from the request (#790): a cancel there would
+// otherwise strand the record in "processing", where the approval retry gets
+// 409 from the transition guard and the row keeps counting against the daily cap.
+//
+//nolint:unparam // (result, error) shape lets executeApprovedExchange tail-call it; a failed exchange is a 200 "failed" body, not an error
 func (h *Handler) failExchange(ctx context.Context, id, reason string) (any, error) {
+	ctx, cancel := ctxutil.DetachedWithTimeout(ctx, riExchangeTerminalSaveTimeout)
+	defer cancel()
 	if failErr := h.config.FailRIExchange(ctx, id, reason); failErr != nil {
 		logging.Errorf("failed to mark exchange %s as failed (DB may be unavailable): %v", id, failErr)
 	}
@@ -2659,19 +2681,21 @@ func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record
 		return h.failExchange(ctx, id, fmt.Sprintf("reserved ceiling %q is not a decimal", ceiling))
 	}
 
-	execFn := exchange.ExecuteExchange
-	if h.executeExchangeFn != nil {
-		execFn = h.executeExchangeFn
-	}
-	exchangeID, freshQ, execErr := execFn(ctx, exchange.ExchangeExecuteRequest{
+	// The provider call and the terminal ledger write that records its outcome
+	// must survive a cancelled approval request (#790).
+	execCtx, cancelExec := ctxutil.DetachedWithTimeout(ctx, riExchangeExecutionTimeout)
+	defer cancelExec()
+	exchangeID, freshQ, execErr := h.awsExchangeExecutor()(execCtx, exchange.ExchangeExecuteRequest{
 		Region:           region,
 		ReservedIDs:      record.SourceRIIDs,
 		TargetOfferingID: record.TargetOfferingID,
 		TargetCount:      int32(record.TargetCount), // #nosec G115 -- RI quantity stored from validated API request; AWS limits RI counts well below math.MaxInt32
 		MaxPaymentDueUSD: effectiveCap,
 	})
+	saveCtx, cancelSave := ctxutil.DetachedWithTimeout(ctx, riExchangeTerminalSaveTimeout)
+	defer cancelSave()
 	if execErr != nil {
-		return h.failExchange(ctx, id, execErr.Error())
+		return h.failExchange(saveCtx, id, execErr.Error())
 	}
 
 	// H3: persist the amount AWS actually accepted, not the reserved ceiling.
@@ -2680,7 +2704,7 @@ func (h *Handler) executeApprovedExchange(ctx context.Context, id string, record
 	// H4: retry the ledger write via retryCompleteWithPayment; persistent
 	// failure is returned as an error (HTTP 500) so the caller knows money
 	// moved but the record was not updated.
-	if completeErr := h.retryCompleteWithPayment(ctx, id, exchangeID, acceptedPaymentDue); completeErr != nil {
+	if completeErr := h.retryCompleteWithPayment(saveCtx, id, exchangeID, acceptedPaymentDue); completeErr != nil {
 		logging.Errorf("all ledger write attempts failed for exchange %s after money moved: %v",
 			id, completeErr)
 		return nil, fmt.Errorf("exchange executed (id=%s) but ledger update failed: %w",

internal/api/handler_ri_exchange_azure_test.go

@@ -579,7 +579,7 @@ func TestExecuteAzureExchange_InScopeSubscriptionAllowed(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-fresh", NetPayable: toPtr(10.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-fresh").Return(
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-fresh").Return(
 		&azurecompute.ExchangeResult{SessionID: "sess-fresh", Status: "Succeeded"}, nil,
 	)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -872,7 +872,7 @@ func TestExecuteAzureExchange_USDCurrencyCapStillEnforced(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-usd-ok", NetPayable: toPtr(50.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-usd-ok").Return(&azurecompute.ExchangeResult{
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-usd-ok").Return(&azurecompute.ExchangeResult{
 		SessionID: "sess-usd-ok", NetPayable: toPtr(50.00), NetPayableCurrency: "USD", Status: "Succeeded",
 	}, nil)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -917,7 +917,7 @@ func TestExecuteAzureExchange_NonUSDWithoutCapStillAllowed(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-kwd-ok", NetPayable: toPtr(500.00), NetPayableCurrency: "KWD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-kwd-ok").Return(&azurecompute.ExchangeResult{
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-kwd-ok").Return(&azurecompute.ExchangeResult{
 		SessionID: "sess-kwd-ok", NetPayable: toPtr(500.00), NetPayableCurrency: "KWD", Status: "Succeeded",
 	}, nil)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -1181,7 +1181,7 @@ func TestExecuteAzureExchange_HappyPath(t *testing.T) {
 		},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, mock.MatchedBy(func(sessionID string) bool {
+	opsClient.On("ExecuteExchange", mock.Anything, mock.MatchedBy(func(sessionID string) bool {
 		return sessionID == freshSessionID
 	})).Return(&azurecompute.ExchangeResult{
 		SessionID:          freshSessionID,
@@ -1267,7 +1267,7 @@ func TestExecuteAzureExchange_ConstraintSetPinsAllDimensions(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-dims", NetPayable: toPtr(10.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-dims").Return(
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-dims").Return(
 		&azurecompute.ExchangeResult{SessionID: "sess-dims", Status: "Succeeded"}, nil,
 	)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -1380,7 +1380,7 @@ func TestExecuteAzureExchange_ConstraintRegionsAreLowerCased(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-case", NetPayable: toPtr(10.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-case").Return(
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-case").Return(
 		&azurecompute.ExchangeResult{SessionID: "sess-case", Status: "Succeeded"}, nil,
 	)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -1439,7 +1439,7 @@ func TestExecuteAzureExchange_LowercaseUSDTakesUSDCapPath(t *testing.T) {
 		&azurecompute.ExchangePreview{SessionID: "sess-lower-usd", NetPayable: toPtr(50.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-lower-usd").Return(
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-lower-usd").Return(
 		&azurecompute.ExchangeResult{SessionID: "sess-lower-usd", Status: "Succeeded"}, nil,
 	)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })
@@ -1560,7 +1560,7 @@ func TestExecuteAzureExchange_CommitFailureClassification(t *testing.T) {
 				&azurecompute.ExchangePreview{SessionID: "sess-commit-fail", NetPayable: toPtr(10.00), NetPayableCurrency: "USD"},
 				[]azurecompute.CompatibleOffering{}, nil,
 			)
-			opsClient.On("ExecuteExchange", ctx, "sess-commit-fail").Return(nil, tt.commitErr)
+			opsClient.On("ExecuteExchange", mock.Anything, "sess-commit-fail").Return(nil, tt.commitErr)
 			t.Cleanup(func() { opsClient.AssertExpectations(t) })
 
 			h := newAzureExecuteMoneyPathHandler(t, opsClient)
@@ -2054,7 +2054,7 @@ func TestExecuteAzureExchange_UnreportedSourceRegionAllowedWithoutRegionScope(t
 		&azurecompute.ExchangePreview{SessionID: "sess-shared", NetPayable: toPtr(10.00), NetPayableCurrency: "USD"},
 		[]azurecompute.CompatibleOffering{}, nil,
 	)
-	opsClient.On("ExecuteExchange", ctx, "sess-shared").Return(
+	opsClient.On("ExecuteExchange", mock.Anything, "sess-shared").Return(
 		&azurecompute.ExchangeResult{SessionID: "sess-shared", Status: "Succeeded"}, nil,
 	)
 	t.Cleanup(func() { opsClient.AssertExpectations(t) })