diff --git a/pkg/appview/holdclient/tier_update_test.go b/pkg/appview/holdclient/tier_update_test.go index e279053..134a5fa 100644 --- a/pkg/appview/holdclient/tier_update_test.go +++ b/pkg/appview/holdclient/tier_update_test.go @@ -4,8 +4,10 @@ import ( "context" "net/http" "net/http/httptest" + "strings" "sync/atomic" "testing" + "time" "atcr.io/pkg/atproto" "github.com/bluesky-social/indigo/atproto/atcrypto" @@ -82,3 +84,149 @@ func TestUpdateCrewTierOnHold_PostsToEndpoint(t *testing.T) { t.Errorf("posted to %q, want %q", gotPath, atproto.HoldUpdateCrewTier) } } + +// The tests above cover the two helpers. UpdateCrewTierOnAllHolds — the +// function the Stripe webhook actually calls, and the one whose error decides +// whether a paid upgrade is retried or dropped — had no test at all. + +// didFor turns an httptest server URL into a did:web that ResolveHoldDIDToURL +// maps straight back to it. The port makes DIDWebToURL choose http, and test +// mode is what lets a did:web that no directory can resolve fall back to being +// decoded from the DID itself. +func didFor(t *testing.T, serverURL string) string { + t.Helper() + host := strings.TrimPrefix(serverURL, "http://") + return "did:web:" + strings.ReplaceAll(host, ":", "%3A") +} + +func testKey(t *testing.T) *atcrypto.PrivateKeyP256 { + t.Helper() + priv, err := atcrypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatalf("generate key: %v", err) + } + return priv +} + +// TestUpdateCrewTierOnAllHolds_JoinedErrorNamesEveryFailingHold: the caller +// 5xxs the Stripe webhook on any non-nil return, and the operator's only +// account of which holds are behind is this error. One failing hold must not +// mask another. +func TestUpdateCrewTierOnAllHolds_JoinedErrorNamesEveryFailingHold(t *testing.T) { + atproto.SetTestMode(true) + t.Cleanup(func() { atproto.SetTestMode(false) }) + + ok := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer ok.Close() + bad1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusServiceUnavailable) + })) + defer bad1.Close() + bad2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "broken", http.StatusInternalServerError) + })) + defer bad2.Close() + + okDID, bad1DID, bad2DID := didFor(t, ok.URL), didFor(t, bad1.URL), didFor(t, bad2.URL) + + err := UpdateCrewTierOnAllHolds(context.Background(), + []string{okDID, bad1DID, bad2DID}, "did:plc:user", 1, testKey(t), "did:web:appview") + if err == nil { + t.Fatal("expected an error when two of three holds fail") + } + for _, did := range []string{bad1DID, bad2DID} { + if !strings.Contains(err.Error(), did) { + t.Errorf("joined error does not name failing hold %s: %v", did, err) + } + } + if strings.Contains(err.Error(), okDID) { + t.Errorf("joined error names the hold that succeeded (%s): %v", okDID, err) + } +} + +// TestUpdateCrewTierOnAllHolds_SlowHoldDoesNotStarveOthers pins the concurrency +// the function's doc claims. +// +// Contacted serially, one hold that burns the whole deadline means the holds +// after it are never contacted at all — and since the webhook retries in the +// same order, a persistently slow first hold would mean later holds are never +// updated on any delivery. The assertion is that the healthy hold is reached +// even though the slow one is listed first and never answers. +func TestUpdateCrewTierOnAllHolds_SlowHoldDoesNotStarveOthers(t *testing.T) { + atproto.SetTestMode(true) + t.Cleanup(func() { atproto.SetTestMode(false) }) + + release := make(chan struct{}) + slow := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-release + })) + defer slow.Close() + defer close(release) + + var healthyHits atomic.Int32 + healthy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + healthyHits.Add(1) + w.WriteHeader(http.StatusOK) + })) + defer healthy.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + // Slow hold first: serial contact would spend the entire budget on it. + err := UpdateCrewTierOnAllHolds(ctx, + []string{didFor(t, slow.URL), didFor(t, healthy.URL)}, + "did:plc:user", 1, testKey(t), "did:web:appview") + + if err == nil { + t.Error("expected an error naming the slow hold") + } + if got := healthyHits.Load(); got != 1 { + t.Errorf("healthy hold contacted %d times, want 1 — it was starved by the slow hold", got) + } +} + +// TestUpdateCrewTierOnAllHolds_DeadlineCutsRetriesShort documents a real +// mismatch rather than asserting an intent. +// +// tierUpdateMaxAttempts is 3 and each attempt is bounded by a 5s client +// timeout, so three attempts against a hold that accepts and never answers +// need ~15s. The Stripe webhook allows the whole fan-out 10s. Under a hang the +// budget therefore funds two attempts, never three, and the caller gets the +// context error rather than the "after N attempts" wrapper. If the deadline or +// either constant changes, this test is where the arithmetic gets re-checked. +func TestUpdateCrewTierOnAllHolds_DeadlineCutsRetriesShort(t *testing.T) { + atproto.SetTestMode(true) + t.Cleanup(func() { atproto.SetTestMode(false) }) + + release := make(chan struct{}) + var attempts atomic.Int32 + hung := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + <-release + })) + defer hung.Close() + defer close(release) + + // Deadline deliberately shorter than tierUpdateMaxAttempts would need. + ctx, cancel := context.WithTimeout(context.Background(), 400*time.Millisecond) + defer cancel() + + start := time.Now() + err := UpdateCrewTierOnAllHolds(ctx, []string{didFor(t, hung.URL)}, + "did:plc:user", 1, testKey(t), "did:web:appview") + elapsed := time.Since(start) + + if err == nil { + t.Fatal("expected an error from a hold that never answers") + } + if elapsed > 3*time.Second { + t.Errorf("fan-out took %v; the context deadline did not abort the retry loop", elapsed) + } + if got := attempts.Load(); got >= int32(tierUpdateMaxAttempts) { + t.Errorf("hung hold was attempted %d times under a deadline that cannot fund %d", + got, tierUpdateMaxAttempts) + } +} diff --git a/pkg/billing/webhook_retry_test.go b/pkg/billing/webhook_retry_test.go index 96f168c..4169f6c 100644 --- a/pkg/billing/webhook_retry_test.go +++ b/pkg/billing/webhook_retry_test.go @@ -15,6 +15,8 @@ import ( "time" appdb "atcr.io/pkg/appview/db" + "atcr.io/pkg/atproto" + "github.com/bluesky-social/indigo/atproto/atcrypto" "github.com/stripe/stripe-go/v84" "github.com/stripe/stripe-go/v84/webhook" ) @@ -285,3 +287,48 @@ func TestHandleSubscriptionChange_NilCustomerDoesNotPanic(t *testing.T) { t.Errorf("HandleWebhook error = %v, want nil for an event with no customer", err) } } + +// TestHandleSubscriptionChange_HoldFanoutFailureIsRetryable closes the loop the +// other tests in this file only cover one half of. +// +// The tier is resolved, the customer is known, and the only thing that fails is +// the push to the managed hold. That has to reach Stripe as a 5xx and leave +// stripe_processed_events empty: a hold that is briefly down otherwise costs +// the customer their tier permanently, which is the same shape of loss as the +// customer-lookup hole above, one layer further out. +func TestHandleSubscriptionChange_HoldFanoutFailureIsRetryable(t *testing.T) { + atproto.SetTestMode(true) + t.Cleanup(func() { atproto.SetTestMode(false) }) + + const secret = "whsec_fanout_test" + m, database := newTestManager(t, secret) + + // The customer resolves cleanly — this test is about what happens after. + stripeAPIReturning(t, http.StatusOK, + `{"id":"cus_fanout","object":"customer","metadata":{"user_did":"did:plc:fanoutuser"}}`) + + hold := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "hold is down", http.StatusServiceUnavailable) + })) + defer hold.Close() + m.managedHolds = []string{"did:web:" + strings.ReplaceAll( + strings.TrimPrefix(hold.URL, "http://"), ":", "%3A")} + + priv, err := atcrypto.GeneratePrivateKeyP256() + if err != nil { + t.Fatalf("generate key: %v", err) + } + m.privateKey = priv + + err = postWebhook(t, m, secret, + signedSubscriptionEvent(t, secret, "evt_fanout_fail", "cus_fanout", time.Now().Unix())) + if err == nil { + t.Fatal("a hold that cannot be updated must fail the webhook so Stripe redelivers") + } + if !strings.Contains(err.Error(), "push tier to managed holds") { + t.Errorf("error does not identify the fan-out as the cause: %v", err) + } + if n := processedCount(t, database); n != 0 { + t.Errorf("stripe_processed_events holds %d rows; a failed event must stay redeliverable", n) + } +}