End a subscription reconcile's Stripe calls when its caller's deadline passes
#285. The subscription reconcile read the subscription from Stripe, set or cleared its collection pause, released a spent schedule, and, from the checkout return, listed the customer's latest subscription with no context on any of these calls. Each ran under stripe-go's own 80-second timeout and two retries, about four minutes, whatever bound its caller had set: the webhook's one-minute attempt and the outbox's two-minute attempt bounded the reconcile's database work but not its Stripe calls. Now fetchSubscription, applyCollectionAction, the release in releaseSpentSchedule and the list in ReconcileLatestSubscriptionForCustomer send the reconcile's context as the call's, so a call ends when its caller's deadline passes or its caller is cancelled, and a reconcile whose context is already done fails at its fetch and writes nothing, as it already failed at its transaction. The reconcile sets no deadline of its own. A collection call or a release cut mid-flight may have landed at Stripe; the outbox's retry, Stripe's event for the caller's own write or the next reconcile decides again from a fresh fetch, so each caller converges. The Stripe mock records each call's context deadline (Call.Deadline) and whether the context was already done (Call.ContextDone), and answers a call whose context is done with the context's error, as the SDK's HTTP client does, ahead of Err, PathErrs and Handle. Line prints neither field, and the release's new params encode the same empty form nil did, so no golden changes. docs/testing.md says so. Tests: TestReconcileCallsCarryTheContext (the fetch and the set, the release of a spent schedule, the checkout return's list), TestReconcileWithADoneContextFailsAtItsFetch, and TestCallRecordsTheContextsDeadline (a deadline, a cancelled context, no context). TestNextReconcileSupersedesADowngradeLeftAfterItsRelease keeps its result. Full suite on reset databases: 4939 passed, 14 skipped, none failed. Breaks, all CAUGHT (12): - the fetch without the context: by the deadline test and the done-context test - the collection update without the context - the release with nil params - the list without the context - newCall leaving Deadline zero - newCall leaving ContextDone false: by the mock test and the done-context test - a done context answered as live: by the mock test and the done-context test - every call that carries a context answered with its error - the reconcile run under a context its caller cannot cancel
This commit is contained in:
@@ -195,6 +195,11 @@ deterministically and offline, with **no live Stripe account**.
|
||||
prints one as text, so
|
||||
`m.OnCall = func(c stripetest.Call) { stmts.Note(stripetest.Line(c)) }` puts
|
||||
each Stripe call among the statements of a `dbtest.Counted` handle.
|
||||
A call records the deadline its request's context carried
|
||||
(`Call.Deadline`) and whether that context was already done
|
||||
(`Call.ContextDone`); a call whose context was already done answers the
|
||||
context's error, as the SDK's HTTP client does. The mock never waits, so a
|
||||
test reads a deadline as recorded.
|
||||
`stripetest.KeyMode` answers the mode of the key the SDK holds when it is
|
||||
called (`"test"` under `Install`); a test passes its result,
|
||||
`stripetest.KeyMode()`, where the console passes the mode boot derived.
|
||||
|
||||
@@ -105,9 +105,13 @@ func collectionParams(action collectionAction) *stripe.SubscriptionParams {
|
||||
// and returns the error wrapped so the outbox's dead-letter table names the
|
||||
// Stripe subscription. The call sets a state, so it needs no idempotency key
|
||||
// of its own: stripe-go adds one per request and reuses it across its own
|
||||
// retries, and repeating the call is harmless.
|
||||
func applyCollectionAction(logger *slog.Logger, stripeSubscriptionID string, action collectionAction) error {
|
||||
if _, err := subscription.Update(stripeSubscriptionID, collectionParams(action)); err != nil {
|
||||
// retries, and repeating the call is harmless. The call carries the
|
||||
// reconcile's context; a call it cuts may have landed at Stripe, and the
|
||||
// outbox's retry decides again from a fresh fetch.
|
||||
func applyCollectionAction(ctx context.Context, logger *slog.Logger, stripeSubscriptionID string, action collectionAction) error {
|
||||
params := collectionParams(action)
|
||||
params.Context = ctx
|
||||
if _, err := subscription.Update(stripeSubscriptionID, params); err != nil {
|
||||
return fmt.Errorf("reconcile: apply collection to %s: %w", stripeSubscriptionID, err)
|
||||
}
|
||||
message := "reconcile: subscription collection paused for a grant"
|
||||
|
||||
@@ -54,6 +54,9 @@ import (
|
||||
// outbox entry in its transaction instead (ApplySubscriptionCollection makes
|
||||
// the call). A member's move calls this after its own Stripe call, so a pause
|
||||
// failure never reports a successful cancellation or switch as failed.
|
||||
//
|
||||
// Every Stripe call it makes carries ctx, so the caller's deadline bounds
|
||||
// them; it sets no deadline of its own.
|
||||
func ReconcileSubscription(ctx context.Context, db *sql.DB, logger *slog.Logger, keyMode string, stripeSubscriptionID, reason string) error {
|
||||
_, err := reconcile(ctx, db, logger, keyMode, stripeSubscriptionID, reason, false)
|
||||
return err
|
||||
@@ -82,7 +85,7 @@ func reconcile(ctx context.Context, db *sql.DB, logger *slog.Logger, keyMode str
|
||||
return false, nil
|
||||
}
|
||||
|
||||
sub, err := fetchSubscription(stripeSubscriptionID)
|
||||
sub, err := fetchSubscription(ctx, stripeSubscriptionID)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
@@ -99,7 +102,7 @@ func reconcile(ctx context.Context, db *sql.DB, logger *slog.Logger, keyMode str
|
||||
if !applyCollection || plan.collection == collectionNone {
|
||||
return false, nil
|
||||
}
|
||||
if err := applyCollectionAction(logger, stripeSubscriptionID, plan.collection); err != nil {
|
||||
if err := applyCollectionAction(ctx, logger, stripeSubscriptionID, plan.collection); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return true, nil
|
||||
@@ -225,9 +228,12 @@ func skipUnreachableSubscription(ctx context.Context, db *sql.DB, logger *slog.L
|
||||
// SDK-pinned version (1.3). The price is expanded so each item carries its
|
||||
// product linkage, and the schedule so its metadata says which price a
|
||||
// scheduled downgrade was made from (plan-switch-timing D4 rule 4);
|
||||
// unexpanded it is only an id.
|
||||
func fetchSubscription(stripeSubscriptionID string) (*stripe.Subscription, error) {
|
||||
// unexpanded it is only an id. The call carries the reconcile's context, so a
|
||||
// reconcile whose context is already done fails here, before its transaction
|
||||
// opens, and writes nothing.
|
||||
func fetchSubscription(ctx context.Context, stripeSubscriptionID string) (*stripe.Subscription, error) {
|
||||
getParams := &stripe.SubscriptionParams{}
|
||||
getParams.Context = ctx
|
||||
getParams.AddExpand("items.data.price")
|
||||
getParams.AddExpand("schedule")
|
||||
sub, err := subscription.Get(stripeSubscriptionID, getParams)
|
||||
@@ -462,7 +468,8 @@ func collectionInputs(
|
||||
// return path, which knows the customer but not the subscription id. It returns
|
||||
// nil (no error) when the customer has no subscription yet. keyMode is the
|
||||
// mode of the key the process runs under; it is handed to
|
||||
// ReconcileSubscription.
|
||||
// ReconcileSubscription. The list carries ctx, the context of the reconcile it
|
||||
// starts.
|
||||
func ReconcileLatestSubscriptionForCustomer(ctx context.Context, db *sql.DB, logger *slog.Logger, keyMode string, stripeCustomerID, reason string) error {
|
||||
if stripeCustomerID == "" {
|
||||
return fmt.Errorf("reconcile: empty stripe customer id")
|
||||
@@ -472,6 +479,7 @@ func ReconcileLatestSubscriptionForCustomer(ctx context.Context, db *sql.DB, log
|
||||
Status: stripe.String("all"),
|
||||
}
|
||||
listParams.Limit = stripe.Int64(1)
|
||||
listParams.Context = ctx
|
||||
iter := subscription.List(listParams)
|
||||
for iter.Next() {
|
||||
s := iter.Subscription()
|
||||
@@ -1281,12 +1289,15 @@ func findSpentSchedule(
|
||||
// supersedes the downgrades the schedule held, in a transaction of its own.
|
||||
// When that transaction fails, the rows stay scheduled with no schedule
|
||||
// behind them, and the next reconcile, finding none attached, supersedes
|
||||
// them.
|
||||
// them. The call carries the reconcile's context; a release it cuts is
|
||||
// handled as a refused one: it may have landed, and the next reconcile either
|
||||
// finds no schedule attached and supersedes the downgrades, or releases the
|
||||
// schedule again.
|
||||
func releaseSpentSchedule(ctx context.Context, database *sql.DB, logger *slog.Logger, stripeSubscriptionID string, spent spentSchedule) error {
|
||||
if spent.id == "" {
|
||||
return nil
|
||||
}
|
||||
if _, err := subscriptionschedule.Release(spent.id, nil); err != nil {
|
||||
if _, err := subscriptionschedule.Release(spent.id, &stripe.SubscriptionScheduleReleaseParams{Params: stripe.Params{Context: ctx}}); err != nil {
|
||||
logger.Warn("reconcile: release a spent subscription schedule",
|
||||
slog.String("stripe_subscription_id", stripeSubscriptionID), slog.Any("error", err))
|
||||
return nil
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package fulfillment_test
|
||||
|
||||
// reconcile_context_test.go pins openspec change reconcile-calls-carry-context:
|
||||
// each Stripe call the subscription reconcile makes carries the reconcile's
|
||||
// context, so a reconcile whose context is already done fails at its fetch and
|
||||
// writes nothing.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
stripe "github.com/stripe/stripe-go/v81"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/fulfillment"
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/stripetest"
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest"
|
||||
)
|
||||
|
||||
// TestReconcileCallsCarryTheContext: under a context with a deadline, each of
|
||||
// the reconcile's Stripe calls carries that deadline.
|
||||
func TestReconcileCallsCarryTheContext(t *testing.T) {
|
||||
rows := []struct {
|
||||
name string
|
||||
// run builds the fixture, makes the entry's call under ctx, and
|
||||
// returns the mock, the number of calls it recorded before that
|
||||
// call, and the names of the calls the entry must send.
|
||||
run func(t *testing.T, ctx context.Context) (mock *stripetest.MockBackend, mark int, want []string)
|
||||
}{
|
||||
{"the fetch and the set", func(t *testing.T, ctx context.Context) (*stripetest.MockBackend, int, []string) {
|
||||
database, fx, mock, _ := grantOver(t, false)
|
||||
stripeAppliesCollection(mock, fx.stripeSubID)
|
||||
mark := len(mock.Calls)
|
||||
if err := fulfillment.ApplySubscriptionCollection(ctx, database, discardLogger(), stripetest.KeyMode(),
|
||||
coreSubID(t, database, fx.stripeSubID), "test:apply"); err != nil {
|
||||
t.Fatalf("apply the collection: %v", err)
|
||||
}
|
||||
return mock, mark, []string{"the fetch", "the set", "the fetch"}
|
||||
}},
|
||||
{"the release of a spent schedule", func(t *testing.T, ctx context.Context) (*stripetest.MockBackend, int, []string) {
|
||||
database, fx, mock, _ := laterUpgrade(t)
|
||||
mark := len(mock.Calls)
|
||||
if err := fulfillment.ApplySubscriptionCollection(ctx, database, discardLogger(), stripetest.KeyMode(),
|
||||
coreSubID(t, database, fx.stripeSubID), "test:apply"); err != nil {
|
||||
t.Fatalf("apply the collection: %v", err)
|
||||
}
|
||||
return mock, mark, []string{"the fetch", "the release"}
|
||||
}},
|
||||
{"the list of the checkout return", func(t *testing.T, ctx context.Context) (*stripetest.MockBackend, int, []string) {
|
||||
database := dbtest.Open(t)
|
||||
fx := seedFixture(t, database)
|
||||
sub := fx.activeSub(fx.planItem())
|
||||
mock := &stripetest.MockBackend{Sub: sub, List: []*stripe.Subscription{sub}}
|
||||
t.Cleanup(stripetest.Install(mock))
|
||||
mark := len(mock.Calls)
|
||||
if err := fulfillment.ReconcileLatestSubscriptionForCustomer(ctx, database, discardLogger(), stripetest.KeyMode(),
|
||||
fx.stripeCustomerID, "test:eager"); err != nil {
|
||||
t.Fatalf("reconcile the latest subscription: %v", err)
|
||||
}
|
||||
return mock, mark, []string{"the list", "the fetch"}
|
||||
}},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
ctx, stop := context.WithTimeout(context.Background(), time.Hour)
|
||||
defer stop()
|
||||
mock, mark, want := row.run(t, ctx)
|
||||
checkCallsCarryTheContext(t, ctx, row.name, mock.Calls[mark:], want)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// callName is what the row calls a request: the fetch, the set, the release or
|
||||
// the list.
|
||||
func callName(c stripetest.Call) string {
|
||||
switch {
|
||||
case c.Method == "GET" && c.Path == "/v1/subscriptions":
|
||||
return "the list"
|
||||
case c.Method == "GET" && strings.HasPrefix(c.Path, "/v1/subscriptions/"):
|
||||
return "the fetch"
|
||||
case c.Method == "POST" && strings.HasPrefix(c.Path, "/v1/subscriptions/"):
|
||||
return "the set"
|
||||
case c.Method == "POST" && strings.HasSuffix(c.Path, "/release"):
|
||||
return "the release"
|
||||
}
|
||||
return c.Method + " " + c.Path
|
||||
}
|
||||
|
||||
// checkCallsCarryTheContext checks that the row sent the calls it names, in
|
||||
// that order, and that each carried ctx's deadline.
|
||||
func checkCallsCarryTheContext(t *testing.T, ctx context.Context, row string, calls []stripetest.Call, want []string) {
|
||||
t.Helper()
|
||||
got := make([]string, len(calls))
|
||||
for i, c := range calls {
|
||||
got[i] = callName(c)
|
||||
}
|
||||
if !slices.Equal(got, want) {
|
||||
t.Errorf("%s: the reconcile sent %v, want %v", row, got, want)
|
||||
return
|
||||
}
|
||||
deadline, _ := ctx.Deadline()
|
||||
for i, c := range calls {
|
||||
if !c.Deadline.Equal(deadline) {
|
||||
t.Errorf("%s: %s carried deadline %v, want %v", row, got[i], c.Deadline, deadline)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestReconcileWithADoneContextFailsAtItsFetch: a reconcile whose context is
|
||||
// already cancelled fails at its fetch, before its transaction opens, and
|
||||
// writes nothing.
|
||||
func TestReconcileWithADoneContextFailsAtItsFetch(t *testing.T) {
|
||||
database := dbtest.Open(t)
|
||||
fx := seedFixture(t, database)
|
||||
mock := &stripetest.MockBackend{Sub: fx.activeSub(fx.planItem())}
|
||||
t.Cleanup(stripetest.Install(mock))
|
||||
ctx, stop := context.WithCancel(context.Background())
|
||||
stop()
|
||||
|
||||
err := fulfillment.ReconcileSubscription(ctx, database, discardLogger(), stripetest.KeyMode(), fx.stripeSubID, "test:done")
|
||||
if err == nil || !errors.Is(err, context.Canceled) || !strings.Contains(err.Error(), "reconcile: fetch subscription") {
|
||||
t.Errorf("a reconcile under a done context returned %v, want the fetch's error", err)
|
||||
}
|
||||
if len(mock.Calls) != 1 || callName(mock.Calls[0]) != "the fetch" || !mock.Calls[0].ContextDone {
|
||||
t.Errorf("a reconcile under a done context sent %v, want the fetch alone, done", mock.Calls)
|
||||
}
|
||||
if n := mappingRows(t, database, fx.stripeSubID); n != 0 {
|
||||
t.Errorf("a reconcile under a done context left %d mapping rows, want 0", n)
|
||||
}
|
||||
}
|
||||
|
||||
func mappingRows(t *testing.T, database *sql.DB, stripeSub string) int {
|
||||
t.Helper()
|
||||
return scalarInt(t, database, `SELECT count(*) FROM stripe.subscription_mappings WHERE stripe_subscription_id = $1`, stripeSub)
|
||||
}
|
||||
+42
-11
@@ -10,6 +10,7 @@ package stripetest
|
||||
import (
|
||||
"bytes"
|
||||
"cmp"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"maps"
|
||||
@@ -20,6 +21,7 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
stripe "github.com/stripe/stripe-go/v81"
|
||||
"github.com/stripe/stripe-go/v81/form"
|
||||
@@ -29,13 +31,14 @@ import (
|
||||
|
||||
// MockBackend implements stripe.Backend in process. It records every request
|
||||
// (Calls, OnCall) and answers it from the fields below, each of which names
|
||||
// the requests it answers; Err, PathErrs and Handle answer ahead of them, in
|
||||
// that order. A request no field answers is an error naming its path.
|
||||
// the requests it answers; a request whose context is already done answers
|
||||
// ahead of them, and Err, PathErrs and Handle after that, in that order. A
|
||||
// request no field answers is an error naming its path.
|
||||
type MockBackend struct {
|
||||
Sub *stripe.Subscription // returned for Get
|
||||
List []*stripe.Subscription // returned for List
|
||||
Invoice *stripe.Invoice // returned for invoice create-preview / upcoming
|
||||
Err error // if set, every call returns it
|
||||
Err error // if set, every call whose context is live returns it
|
||||
// Products and Prices answer product.Get and price.Get, keyed by
|
||||
// Stripe id, for the environment check's read-back of the mapped ids.
|
||||
Products map[string]*stripe.Product
|
||||
@@ -71,9 +74,9 @@ type MockBackend struct {
|
||||
// has_more set, as Stripe does.
|
||||
Invoices map[string]*stripe.Invoice
|
||||
InvoiceLines map[string][]*stripe.InvoiceLineItem
|
||||
// PathErrs answers one API path with one error, ahead of everything
|
||||
// else, so a test can refuse the invoice preview or the charge list
|
||||
// the way Stripe refuses a key without the permission.
|
||||
// PathErrs answers one API path with one error, ahead of Handle and the
|
||||
// canned fields, so a test can refuse the invoice preview or the charge
|
||||
// list the way Stripe refuses a key without the permission.
|
||||
PathErrs map[string]error
|
||||
// UpdateSub, when set, answers a subscription update (POST
|
||||
// /v1/subscriptions/{id}) in place of Sub, so a test can return what
|
||||
@@ -94,10 +97,10 @@ type MockBackend struct {
|
||||
// once they are done, or through Sent and CallsTo while they run.
|
||||
Calls []Call
|
||||
// OnCall, when set, sees every request as soon as it is recorded, before
|
||||
// it is answered, including a request Err or PathErrs refuses. A test
|
||||
// uses it to note the call among the statements of a counted database
|
||||
// handle, so the record shows where each Stripe call falls against the
|
||||
// transactions around it.
|
||||
// it is answered, including a request whose context is already done and
|
||||
// one Err or PathErrs refuses. A test uses it to note the call among the
|
||||
// statements of a counted database handle, so the record shows where
|
||||
// each Stripe call falls against the transactions around it.
|
||||
OnCall func(c Call)
|
||||
mu sync.Mutex
|
||||
}
|
||||
@@ -107,6 +110,13 @@ type Call struct {
|
||||
Method string
|
||||
Path string
|
||||
Form url.Values
|
||||
// Deadline is the deadline of the request's Params.Context: zero when
|
||||
// the call carries no context, or one without a deadline. ContextDone
|
||||
// is true when that context was already done as the mock received the
|
||||
// call. The mock never waits, so a test sees a deadline that is still
|
||||
// in the future only as recorded here.
|
||||
Deadline time.Time
|
||||
ContextDone bool
|
||||
}
|
||||
|
||||
// Line is c as one line of text: "stripe", the method, the path and, when
|
||||
@@ -167,12 +177,17 @@ func resourceMissing(id string) error {
|
||||
}
|
||||
}
|
||||
|
||||
// Call records the request, then answers it: Err refuses every request,
|
||||
// Call records the request, then answers it: a request whose context is
|
||||
// already done answers with the context's error, as the SDK's HTTP client
|
||||
// returns it without sending the request; then Err refuses every request,
|
||||
// PathErrs refuses its path, Handle answers ahead of the canned fields, and
|
||||
// otherwise the first route below that matches the path answers.
|
||||
func (m *MockBackend) Call(method, path, key string, params stripe.ParamsContainer, v stripe.LastResponseSetter) error {
|
||||
c := newCall(method, path, params)
|
||||
m.record(c)
|
||||
if ctx := requestContext(params); ctx != nil && ctx.Err() != nil {
|
||||
return ctx.Err()
|
||||
}
|
||||
if m.Err != nil {
|
||||
return m.Err
|
||||
}
|
||||
@@ -202,6 +217,10 @@ func newCall(method, path string, params stripe.ParamsContainer) Call {
|
||||
// A call made with nil params arrives as a typed nil inside the
|
||||
// interface, which the form encoder would dereference.
|
||||
if params != nil && !reflect.ValueOf(params).IsNil() {
|
||||
if ctx := requestContext(params); ctx != nil {
|
||||
c.Deadline, _ = ctx.Deadline()
|
||||
c.ContextDone = ctx.Err() != nil
|
||||
}
|
||||
fv := &form.Values{}
|
||||
form.AppendTo(fv, params)
|
||||
if q, err := url.ParseQuery(fv.Encode()); err == nil {
|
||||
@@ -220,6 +239,18 @@ func newCall(method, path string, params stripe.ParamsContainer) Call {
|
||||
return c
|
||||
}
|
||||
|
||||
// requestContext is the context the request's params carry, or nil when the
|
||||
// params are nil or carry none.
|
||||
func requestContext(params stripe.ParamsContainer) context.Context {
|
||||
if params == nil || reflect.ValueOf(params).IsNil() {
|
||||
return nil
|
||||
}
|
||||
if p := params.GetParams(); p != nil {
|
||||
return p.Context
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// route answers c from the canned fields. The first case that matches wins,
|
||||
// so a path two cases could match (GET /v1/invoices/upcoming, say) is
|
||||
// answered by the earlier one.
|
||||
|
||||
@@ -4,9 +4,11 @@
|
||||
package stripetest_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/url"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
stripe "github.com/stripe/stripe-go/v81"
|
||||
"github.com/stripe/stripe-go/v81/subscription"
|
||||
@@ -108,3 +110,57 @@ func TestKeyModeIsTheModeOfTheSDKsKey(t *testing.T) {
|
||||
t.Errorf("KeyMode after the restore = %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCallRecordsTheContextsDeadline: a call records the deadline its
|
||||
// request's context carried and whether that context was already done, and a
|
||||
// call whose context is already done answers the context's error. A call with
|
||||
// no context records neither and answers as live.
|
||||
func TestCallRecordsTheContextsDeadline(t *testing.T) {
|
||||
live, stopLive := context.WithTimeout(context.Background(), time.Hour)
|
||||
defer stopLive()
|
||||
liveDeadline, _ := live.Deadline()
|
||||
cancelled, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
rows := []struct {
|
||||
name string
|
||||
ctx context.Context
|
||||
wantDeadline time.Time
|
||||
wantDone bool
|
||||
wantErr error
|
||||
}{
|
||||
{"a deadline", live, liveDeadline, false, nil},
|
||||
{"a cancelled context", cancelled, time.Time{}, true, context.Canceled},
|
||||
{"no context", nil, time.Time{}, false, nil},
|
||||
}
|
||||
for _, row := range rows {
|
||||
t.Run(row.name, func(t *testing.T) {
|
||||
m := &stripetest.MockBackend{Sub: &stripe.Subscription{ID: "sub_1"}}
|
||||
t.Cleanup(stripetest.Install(m))
|
||||
|
||||
params := &stripe.SubscriptionParams{}
|
||||
params.Context = row.ctx
|
||||
got, err := subscription.Get("sub_1", params)
|
||||
|
||||
if len(m.Calls) != 1 {
|
||||
t.Fatalf("%s: the mock recorded %d calls, want 1", row.name, len(m.Calls))
|
||||
}
|
||||
c := m.Calls[0]
|
||||
if !c.Deadline.Equal(row.wantDeadline) || c.ContextDone != row.wantDone {
|
||||
t.Errorf("%s: call deadline %v and done %v, want %v and %v", row.name, c.Deadline, c.ContextDone, row.wantDeadline, row.wantDone)
|
||||
}
|
||||
if !errors.Is(err, row.wantErr) {
|
||||
t.Errorf("%s: the call answered %v, want %v", row.name, err, row.wantErr)
|
||||
}
|
||||
if row.wantErr == nil {
|
||||
id := ""
|
||||
if got != nil {
|
||||
id = got.ID
|
||||
}
|
||||
if id != "sub_1" {
|
||||
t.Errorf("%s: the call answered subscription %q, want sub_1", row.name, id)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
schema: spec-driven
|
||||
created: 2026-10-29
|
||||
@@ -0,0 +1,106 @@
|
||||
## Context
|
||||
|
||||
`reconcile` (`internal/fulfillment/reconcile.go:67-106`), the body of `ReconcileSubscription` (`:57-60`) and of the outbox's `ApplySubscriptionCollection` (`internal/fulfillment/collection.go:132-156`), runs in this order. It checks the subscription's mapping against the key's environment (`skipUnreachableSubscription`, `:81`, `:212-222`), a database read under `ctx` that does not skip when it fails. It fetches the subscription from Stripe (`fetchSubscription`, `:85`, `:229-238`). It runs its transaction (`reconcileInTx`, `:119-207`), whose first statements, `BeginMaterializing` (`:121`) and the subscription's advisory lock (`:129-132`), take `ctx`. After the commit it releases a spent schedule (`releaseSpentSchedule`, `:96`, `:1285-1298`) and, for the outbox only, pauses or clears the subscription's collection (`applyCollectionAction`, `:102`, `collection.go:109-119`). `ReconcileLatestSubscriptionForCustomer` (`reconcile.go:466-484`), the checkout return's entry, lists the customer's latest subscription (`:470-475`) and reconciles it under the same `ctx`.
|
||||
|
||||
These four Stripe calls are the only ones on the reconcile's path. `reconcile.go` and `collection.go` import no Stripe resource package other than `subscription` and `subscriptionschedule` (`reconcile.go:33-34`, `collection.go:14`), and the transaction makes no Stripe call: the reconcile calls Stripe only before the transaction opens (`reconcile.go:85`) and after it commits (`:94-105`). No call sets `Params.Context`: the fetch builds `&stripe.SubscriptionParams{}` (`:230`), the list `&stripe.SubscriptionListParams{...}` (`:470-474`), the collection call `collectionParams(action)` (`collection.go:110`), and the release passes `nil` (`reconcile.go:1289`).
|
||||
|
||||
stripe-go v81.4.0 attaches a context to a request only when `Params.Context` is set (`stripe.go:564-565`); a list's context reaches each page's request through `ListParams.ToParams` (`params.go:161-166`). Without one, a call runs under the client's 80-second HTTP timeout (`stripe.go:1394`) and two network retries (`DefaultMaxNetworkRetries`, `:54`), with 0.5 to 5 seconds between them (`:1398-1399`), about four minutes in all. With one, `shouldRetry` stops at once when the context is done (`:954-962`). stripe-go's own comment on the field says that a cancelled or timed-out context gives no guarantee whether Stripe completed the operation, and that the caller must retry with the same idempotency key or query the state (`params.go:186-190`).
|
||||
|
||||
So a reconcile's Stripe call keeps running after the reconcile's context ends. Today a context that is already done fails the reconcile at `BeginMaterializing` (`reconcile.go:121-124`), after the fetch has run to its end; a context that ends after the commit does not stop the release or the collection call. The invoice reconcile already does what this change does: its reads carry `ctx`, "so the activity's attempt timeout bounds them" (`internal/integrations/stripe/workflows/invoice_reconcile.go:190-198`, `:209`, `:225`).
|
||||
|
||||
The invoice sweep's 90-second bound (`invoice_sweep.go:48-50`, applied at `:260`) does not reach the subscription reconcile. The sweep reconciles each invoice under `InvoiceTrigger{Ref: InvoiceSweepRef}` (`:261`), which leaves `EnsureSubscription` unset, and only that flag lets the invoice reconcile reach `ReconcileSubscription` (`invoice_reconcile.go:50-54`, `:331`). The invoice event sets it (`webhook_invoice.go:107-110`), so the call at `invoice_reconcile.go:363` runs under the webhook's attempt, not under the 90-second bound.
|
||||
|
||||
## Goals / Non-Goals
|
||||
|
||||
**Goals:**
|
||||
- Every Stripe call on the subscription reconcile's path carries the reconcile's context: the fetch, the release, the collection call, and the checkout return's list.
|
||||
- A test reads, through the Stripe mock, the deadline each call carried, and another shows that a reconcile whose context is already done fails at its fetch and writes nothing.
|
||||
|
||||
**Non-Goals:**
|
||||
- The member moves' own Stripe calls (`internal/fulfillment/plan_change.go`) and the scheduled-change sweep's own calls (`internal/fulfillment/sweep.go:174`, `:208`).
|
||||
- A deadline of the reconcile's own. #278 sets the paused switch's deadlines around the reconcile it runs.
|
||||
- Any change to what a reconcile decides or writes.
|
||||
|
||||
## Decisions
|
||||
|
||||
### D1. The four calls and their params
|
||||
|
||||
Pick: each call sets the reconcile's `ctx` as its `Params.Context`.
|
||||
|
||||
- `fetchSubscription(ctx, stripeSubscriptionID)` sets `getParams.Context = ctx`; `reconcile` passes its own `ctx` (`reconcile.go:85`).
|
||||
- `applyCollectionAction(ctx, logger, stripeSubscriptionID, action)` sets `Context = ctx` on the params `collectionParams(action)` builds, before `subscription.Update`; `reconcile` passes its own `ctx` (`:102`). `collectionParams` keeps building only the form.
|
||||
- `releaseSpentSchedule` sends `&stripe.SubscriptionScheduleReleaseParams{Params: stripe.Params{Context: ctx}}` in place of `nil`. `Context` is tagged `form:"-"` and no other field is set, so the release's form stays empty, as `nil` left it. The params are no longer nil, so stripe-go now adds its automatic `Idempotency-Key` header to the release (`stripe.go:563-577`) and reuses it across its own retries; `nil` sent none. The mock reads no header.
|
||||
- `ReconcileLatestSubscriptionForCustomer` sets `listParams.Context = ctx`. The reconcile it starts takes the same `ctx`, so the list and the reconcile share one deadline.
|
||||
|
||||
`ReconcileSubscription`'s doc comment states the rule once: every Stripe call it makes carries `ctx`, so its caller's deadline bounds them. The four functions' comments say that they carry the reconcile's context.
|
||||
|
||||
Rejected: a deadline of the reconcile's own, a `context.WithTimeout` inside `reconcile`. Every caller already sets one (D3), and a second deadline would have to be chosen against each of them.
|
||||
|
||||
Rejected: `context.WithoutCancel(ctx)` for the two writes after the commit, so that a member who closes the page cannot cut the release. Whether a Stripe write may outlive the request that asked for it is the decision the member moves' own calls still need, and this change does not make it. This change can bound the reconcile's two writes by the caller's context because the next reconcile decides both again from a fresh fetch (D4). A member move's own write is different: it reports its outcome to the member, so a write cut by the request's end changes what the member is told.
|
||||
|
||||
### D2. The mock records the context and answers a done one
|
||||
|
||||
`stripetest.Call` (`internal/stripetest/mock.go:106-110`) gains `Deadline time.Time`, the deadline of the request's `Params.Context` (zero when the call carries no context, or a context without a deadline), and `ContextDone bool`, true when that context was already done as the mock received the call. `newCall` (`:200-221`) reads both from `params.GetParams().Context` inside its typed-nil guard (`:204-210`), which the member moves' releases, still sent with `nil`, need. A list's context arrives the same way: `CallRaw` (`:454-463`) passes on the `*stripe.Params` that stripe-go built with `ListParams.ToParams`. `Line` (`:117-129`) prints neither field, so no golden changes. The mock never waits, so a deadline still in the future is visible only as recorded, and a test reads it there.
|
||||
|
||||
`MockBackend.Call` (`:173-195`) answers a call whose context is already done with that context's error. It answers after `record`, so `Calls` and `OnCall` still see the call, and ahead of `Err`, `PathErrs` and `Handle`, because stripe-go's HTTP client returns the context's error without sending the request. `MockBackend`'s doc comment ("Err, PathErrs and Handle answer ahead of them, in that order", `:30-33`) and `Call`'s (`:170-172`) name the done context first.
|
||||
|
||||
Rejected: recording only, with no answer. Under a done context the fetch would then succeed in the mock, and the reconcile would fail at `BeginMaterializing`, as it does today; a test could not tell a reconcile that fails at its fetch from one that fails at its transaction.
|
||||
|
||||
One existing Stripe-mocked test cancels a context that reaches the mock: `TestNextReconcileSupersedesADowngradeLeftAfterItsRelease` (`internal/fulfillment/release_schedule_after_commit_test.go:86-122`). It cancels the context inside `Handle`, after the mock has received the release with a live context, so the release still answers, the write after it still fails with `context.Canceled`, and the test keeps its result.
|
||||
|
||||
### D3. What each caller now gets
|
||||
|
||||
| Caller | Context it passes | Deadline that now bounds the reconcile's calls | What a cancelled or expired context now does |
|
||||
|---|---|---|---|
|
||||
| Webhook: `checkout.session.completed` (`internal/integrations/stripe/workflows/webhook_subscription.go:59`) and `customer.subscription.*` (`:96`) | The activity context of `ProcessStripeWebhookEvent`'s attempt | The attempt's one minute (`webhookAttemptTimeout`, `webhook.go:63`, the `StartToCloseTimeout` of `budgetedOptions` at `:74`); the Temporal SDK gives the activity context the deadline of that timeout from the attempt's start, or of the schedule-to-close budget when that comes first (`go.temporal.io/sdk` v1.38.0, `internal/activity.go:442-452`) | The call ends and the reconcile returns its error, which is retryable; Temporal retries the event on its one-day budget. Today the fetch runs on past the attempt, Temporal's next attempt can start beside it, and the first attempt fails at its transaction. |
|
||||
| Invoice event: `ensureSubscription` (`invoice_reconcile.go:329-374`, the call at `:363`), reached only with `EnsureSubscription` (`webhook_invoice.go:107-110`) | The same webhook attempt's context | One minute, shared with the invoice's own reads | `transientError` takes a context error as transient (`invoice_reconcile.go:381-384`), so the attempt fails and Temporal retries it, as it does today when the transaction cannot begin. |
|
||||
| Invoice sweep (`invoice_sweep.go:260-261`) | None: the sweep never reaches the subscription reconcile (Context) | None | None |
|
||||
| Outbox executor (`outbox.go:349-358`, the call at `:356`) | The activity context of `ExecuteStripeOutboxEntry`'s attempt | The attempt's two minutes (`outboxAttemptTimeout`, `outbox.go:69`, used at `:128`), shared by both runs of `ApplySubscriptionCollection`'s loop (`collection.go:146-154`) | The fetch, the release or the collection call ends at the deadline. A failed fetch or collection call is returned; `classifyStripeError` leaves an error that is not a `*stripe.Error` retryable (`outbox.go:295-305`), and Temporal retries the entry. A collection call cut in flight is D4's. |
|
||||
| Scheduled-change sweep: `reconcilePeriodEndDowngrade` (`internal/fulfillment/sweep.go:146`), `fireCancellation` (`:178`), `firePlanSwitch` (`:217`) | The activity context of `SweepScheduledChangesActivity` (`internal/workflows/billing/activities.go:41-42`) | The attempt's one minute (`common.DefaultActivityOptions`, `internal/workflows/common/options.go:16`, used at `sweep_workflow.go:23`), shared by every due row of the run | The row's reconcile fails at its fetch; the sweep logs it and goes on (`sweep.go:49-54`), and the later rows' database reads fail on the same context, as they do today. A claimed cancellation or switch has made its own Stripe call by then, and Stripe's event for that call reconciles the subscription; a period-end downgrade is never claimed, so the next hourly run reconciles it again. |
|
||||
| Member moves: `upgradeNow` (`plan_change.go:373`), `downgradeAtRenewal` (`:458`), `KeepCurrentPlan` (`:516`), `CancelSubscription` (`:665`, `:685`), from `member_plan_moves.go:220`, `:264`, `:317` | `r.Context()` | The request's 32 seconds: `middleware.Timeout(32*time.Second)` (`internal/server/server.go:786`) wraps the handler in `http.TimeoutHandler`, which runs it with a context carrying that deadline (`internal/middleware/timeout.go:11-24`); the context is also cancelled when the member's connection closes | The reconcile fails at its fetch, and the move returns that error as it returns the transaction's error today. The move's own Stripe call has landed by then, and Stripe's event for it reconciles the subscription. |
|
||||
| Checkout return: `eagerReconcileOnReturn` (`server.go:1029-1053`) | `rctx`, five seconds from the request's context (`:1045`) | Five seconds, now for the list and the fetch as well as the transaction | The eager reconcile gives up after five seconds and logs that the webhook will reconcile (`:1050-1051`). Today a slow Stripe holds the dashboard past the five seconds, up to the request's 32-second timeout, which answers 503, and the eager reconcile then fails at its transaction; after the change, the dashboard renders after at most five seconds, with the same result for the reconcile. |
|
||||
|
||||
### D4. A write cut in flight converges
|
||||
|
||||
Two of the four calls write: the collection call (the outbox's reconcile only) and the release (every caller). A context that ends while either is in flight leaves the write's outcome unknown, as stripe-go's comment says (`params.go:186-190`). The reconcile queries the state: every run fetches before it decides.
|
||||
|
||||
- **The collection call** sets a state, so repeating it is harmless (`collection.go:103-108`). It runs only in the outbox's reconcile, whose context ends when the attempt's deadline passes, the worker stops, or the one-day budget is spent. In the first two cases Temporal retries the entry, and the retry's fetch either finds Stripe agreeing with the rule, so it makes no call, or makes the call again. When the budget is spent, the entry dead-letters and the Stripe provider page lists it, as for any entry whose budget ran out.
|
||||
- **The release** is handled as a release Stripe refuses: logged, with the downgrades it held left scheduled (`reconcile.go:1289-1293`). If it landed, Stripe holds no schedule, and the next reconcile finds none attached and supersedes those downgrades; `TestNextReconcileSupersedesADowngradeLeftAfterItsRelease` pins that state. If it did not land, the schedule is still spent, and the next reconcile releases it. A next reconcile comes in each case:
|
||||
- **Webhook, invoice event and outbox:** at the attempt's deadline Temporal has already timed the attempt out and retries it, whatever the activity returns; at the worker's stop the activity's next database write (the event's status, the invoice's transaction, the entry's status) fails on the same context, so the attempt fails and Temporal retries it. **Sweep:** at the deadline Temporal retries the attempt; at the worker's stop the activity can return nil and complete, and the next hourly run reconciles a period-end downgrade again, while a claimed cancellation or switch is reconciled by Stripe's event for the sweep's own call.
|
||||
- **Member moves:** the move's own Stripe call comes before its reconcile, and Stripe's event for that call reconciles the subscription from a fetch taken after the call. `upgradeNow`, `KeepCurrentPlan` and `CancelSubscription` also release a schedule themselves before they reconcile (`plan_change.go:363`, `:504`, `:652`), and `upgradeNow` already leaves a failed release "to reconcile's rule 4, which releases it on the next event or sweep" (`:355-357`).
|
||||
- **Checkout return:** the checkout's own events, `checkout.session.completed` and `customer.subscription.created`, reconcile the same subscription.
|
||||
|
||||
A landed release also reaches the console as an event of its own: `docs/stripe.md:246` says that a release made in the Dashboard reaches the console, which records the downgrade as taken back.
|
||||
|
||||
### D5. The documents
|
||||
|
||||
No document states a timeout for the reconcile's Stripe calls; `docs/stripe.md` says nothing about Stripe call timeouts or about the reconcile's calls. `docs/testing.md`, "Stripe-mocked tests", describes what `MockBackend` records and answers (`:186-200`), and it gains the two fields and the answer to a done context (`CONTRIBUTING.md`, "Docs move with the code").
|
||||
|
||||
## Failure table
|
||||
|
||||
| # | State when the reconcile runs | Today | After |
|
||||
|---|---|---|---|
|
||||
| F1 | Stripe answers slowly while a webhook event's or an outbox entry's reconcile fetches | The fetch runs up to about four minutes, past the attempt; Temporal's next attempt can start beside it; the first attempt fails at its transaction | The fetch ends at the attempt's deadline; the attempt fails with the fetch's error and Temporal retries it |
|
||||
| F2 | An outbox entry's collection call is in flight at the attempt's deadline | The call runs to its end, beside Temporal's retry | The call ends at the deadline; it may have landed, and the retry's fetch decides again (D4) |
|
||||
| F3 | A reconcile starts with a context already done (a member closed the page during the move's own Stripe call; the sweep's attempt ran out during its own cancel or update) | The fetch runs; the reconcile fails at `BeginMaterializing` and writes nothing | The fetch fails at once with the context's error; nothing is written; no request reaches Stripe |
|
||||
| F4 | The checkout return, with Stripe slower than five seconds | The dashboard waits on the list and the fetch, up to the request's 32-second timeout; the eager reconcile then fails at its transaction | The eager reconcile gives up after five seconds and the dashboard renders; the webhook reconciles, as before |
|
||||
| F5 | The context ends after the commit, while the release is in flight | The release runs to its end | The release ends; it is logged as a refused one, and the next reconcile supersedes the downgrades or releases the schedule (D4) |
|
||||
| F6 | The context ends inside the transaction | The transaction fails; nothing commits | Unchanged |
|
||||
|
||||
## Locks and transactions
|
||||
|
||||
No lock or transaction changes. The four calls run outside every transaction: the list and the fetch before `reconcileInTx` opens, the release and the collection call after it commits (`reconcile.go:94-105`). No lock is held while a call waits, so a call that ends early holds nothing. `supersedeReleasedDowngrades` (`:1306-1323`) runs only after a release that Stripe answered, so a cut release never opens its transaction.
|
||||
|
||||
## Tests
|
||||
|
||||
Each test claim is pinned by a break (`docs/testing.md`, "Prove a test with deliberate breaks"); the tasks list each break with the text its failing test prints. The deadline test runs each row under a context whose deadline is an hour away, so the deadline never passes inside the test, and it compares each recorded deadline with the context's own. Each row first checks that it sent the calls it names, so a fixture that makes no such call cannot pass the row.
|
||||
|
||||
## What is unverified
|
||||
|
||||
- **That a landed release reaches the console as an event.** Read from `docs/stripe.md:246`, not observed by this change, which makes no Stripe call. D4 does not depend on it: a next reconcile comes from the caller's retry or from Stripe's event for the move's own call.
|
||||
- **That the real backend refuses a done context without sending the request.** Read from stripe-go v81.4.0 (`stripe.go:564-565`, `:954-962`) and from net/http, which returns a done context's error before it sends. The mock's answer imitates it, and no test runs the real backend.
|
||||
|
||||
## Risks / Trade-offs
|
||||
|
||||
- **A release on a request path now ends with the request.** A member who closes the page after a move's reconcile has committed can cut the release that the reconcile would have finished. The spent schedule then waits for Stripe's event for the move, as a release Stripe refuses waits today.
|
||||
- **The mock's answer to a done context is new behaviour for every Stripe-mocked test.** One test cancels a context that reaches the mock, and it keeps its result (D2).
|
||||
@@ -0,0 +1,44 @@
|
||||
## Why
|
||||
|
||||
The subscription reconcile (`ReconcileSubscription`, `internal/fulfillment/reconcile.go`) brings the console's record of a subscription in line with Stripe. It reads the subscription from Stripe before its transaction opens. After the transaction commits, it may release a subscription schedule that has nothing left to do, and when the outbox runs it, it may pause or clear the subscription's payment collection. The checkout return starts its reconcile with a fourth call, a list of the customer's latest subscription. None of these four calls carries a context, so stripe-go runs each one under its own defaults, an 80-second timeout and two retries, about four minutes, whatever deadline the caller set.
|
||||
|
||||
Every caller sets a deadline: a webhook event's attempt has one minute, an outbox entry's attempt two minutes, the scheduled-change sweep's attempt one minute, a member's request 32 seconds, and the checkout return five seconds. A call that outlives its caller's deadline keeps running. Temporal retries a timed-out attempt while that attempt's Stripe call is still in flight, and the checkout return's five seconds do not bound how long the dashboard waits on Stripe.
|
||||
|
||||
The paused switch of `grant-pauses-subscription-billing` (#278) restores a subscription's collection pause through the outbox's reconcile under a deadline of its own. That deadline bounds the restore only once the reconcile's calls carry the reconcile's context.
|
||||
|
||||
The invoice sweep's 90-second bound (`internal/integrations/stripe/workflows/invoice_sweep.go:50`) never reaches the subscription reconcile: the invoice reconcile reconciles the subscription first only when its trigger sets `EnsureSubscription` (`internal/integrations/stripe/workflows/invoice_reconcile.go:50-54`, `:331`), and only the invoice event sets it (`webhook_invoice.go:107-110`). That path runs under the webhook's one-minute attempt.
|
||||
|
||||
## What Changes
|
||||
|
||||
- **Each of the reconcile's Stripe calls carries the reconcile's context**, as the invoice reconcile's reads already do: the fetch (`fetchSubscription`), the release of a spent schedule (`releaseSpentSchedule`), the pause or clear of collection (`applyCollectionAction`) and the checkout return's list (`ReconcileLatestSubscriptionForCustomer`). Each call ends when its caller's deadline passes or its caller is cancelled. A reconcile whose context is already done fails at its fetch, or at the checkout return's list or the outbox's mapping read where one of those comes first, and writes nothing; today it fails at the start of its transaction after the fetch has run, with the same result. A release or a collection call that its context cuts may still have reached Stripe; the next reconcile fetches again and either finds the change made or makes it again (design D4).
|
||||
- **The Stripe mock records each call's deadline and whether its context was already done**, and it answers a call whose context is already done with the context's error, as stripe-go's HTTP client does.
|
||||
- **Three tests**: one reads the deadline each call carried under a reconcile with a deadline (the fetch, the collection call, the release, the list); one runs a reconcile whose context is already cancelled and finds that it fails at its fetch and writes nothing; one pins what the mock records and answers.
|
||||
- **`docs/testing.md`** describes the mock's two new fields and its answer to a done context.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- **The member moves' own Stripe calls** (`internal/fulfillment/plan_change.go`) and **the scheduled-change sweep's own cancel and update** (`internal/fulfillment/sweep.go:174`, `:208`). They are not the reconcile's calls, and a request's context on a Stripe write needs its own decision about cancellation, which this change does not make. #278 sets deadlines on the paused switch's calls itself.
|
||||
- **A deadline of the reconcile's own.** Each caller's deadline bounds the reconcile's calls; the reconcile sets none.
|
||||
|
||||
## Capabilities
|
||||
|
||||
### New Capabilities
|
||||
|
||||
None.
|
||||
|
||||
### Modified Capabilities
|
||||
|
||||
- `stripe-subscription-management`: added "A reconcile's Stripe calls end when its context does" (the four calls, a done context failing at the fetch, a cut write left to the next reconcile).
|
||||
|
||||
## Impact
|
||||
|
||||
- **Code:** `internal/fulfillment/reconcile.go` (the fetch, the release, the list and their doc comments), `internal/fulfillment/collection.go` (the collection update), `internal/stripetest/mock.go` (two fields of `Call`, the answer to a done context).
|
||||
- **Schema, routes:** none. **Stripe calls:** the same calls with the same forms; each now carries its caller's context, and the release, sent with params now, also carries the idempotency key stripe-go adds to a write.
|
||||
- **Behaviour a deployment sees:** a reconcile's Stripe call ends at its caller's deadline. A webhook or outbox attempt that times out no longer leaves its Stripe call running beside the retry. The checkout return waits at most five seconds for the eager reconcile. A release that its caller's end cuts waits for the next reconcile, as a release Stripe refuses does today.
|
||||
- **Tests:** two new tests in `internal/fulfillment` and one in `internal/stripetest`; no golden changes.
|
||||
- **Docs:** `docs/testing.md`.
|
||||
|
||||
## Related issues
|
||||
|
||||
- **#285** is this change; it closes with it.
|
||||
- **#278** (section 11 of `grant-pauses-subscription-billing`, #269) depends on it and lands after it: its restore is bounded only once the reconcile's calls carry the reconcile's context.
|
||||
+28
@@ -0,0 +1,28 @@
|
||||
## ADDED Requirements
|
||||
|
||||
### Requirement: A reconcile's Stripe calls end when its context does
|
||||
|
||||
Every Stripe call the subscription reconcile makes SHALL carry the reconcile's context as the request's context, so that the call ends when that context's deadline passes or the context is cancelled. The calls are the fetch of the subscription, the release of a spent subscription schedule after the commit, the pause or clear of the subscription's collection that the transactional outbox's reconcile makes after the commit, and the list of the customer's latest subscription with which the checkout return starts its reconcile; the list SHALL carry the same context as the reconcile it starts. The reconcile SHALL NOT set a deadline of its own: its caller's context bounds its calls.
|
||||
|
||||
A reconcile whose context is done before or during its fetch SHALL return an error that wraps the context's error and SHALL write nothing; the error is the fetch's, except where the checkout return's list or the outbox's read of the subscription's mapping comes first and fails on the same context. A release that its context cuts SHALL be handled as a release Stripe refuses: it is logged, and the downgrades the schedule held stay scheduled, so that the next reconcile, fetching again, supersedes them when Stripe holds no schedule or releases the schedule again. A collection call that its context cuts SHALL be returned to the outbox's workflow as a failed call, whose retry decides again from a fresh fetch.
|
||||
|
||||
#### Scenario: A reconcile under a deadline sends each call with that deadline
|
||||
|
||||
- **WHEN** the outbox's reconcile of a subscription a grant displaces runs under a context with a deadline, fetches the subscription, pauses its collection and fetches it again
|
||||
- **THEN** each of those three Stripe requests SHALL carry that deadline
|
||||
|
||||
#### Scenario: The release of a spent schedule carries the reconcile's deadline
|
||||
|
||||
- **WHEN** a reconcile under a context with a deadline finds the console's schedule spent and releases it after its commit
|
||||
- **THEN** the fetch and the release SHALL carry that deadline
|
||||
|
||||
#### Scenario: The checkout return's list carries its deadline
|
||||
|
||||
- **WHEN** the checkout return reconciles the customer's latest subscription under a context with a deadline
|
||||
- **THEN** the list of the customer's subscriptions and the reconcile's fetch SHALL carry that deadline
|
||||
|
||||
#### Scenario: A reconcile whose context is already done fails at its fetch
|
||||
|
||||
- **WHEN** a reconcile of a subscription the console has not yet recorded starts under a context that is already cancelled
|
||||
- **THEN** it SHALL return the fetch's error, which wraps the context's error
|
||||
- **AND** it SHALL write nothing, so the subscription still has no mapping row
|
||||
@@ -0,0 +1,57 @@
|
||||
Tests follow `docs/testing.md`: database tests open the database through `dbtest`, build their rows through the test kit or the package's fixtures, filter to the rows their own fixture made, and pass twice in a row without a reset. A Stripe test drives `stripetest.MockBackend` with the API state Stripe would return, never a hand-built event payload, and runs without `t.Parallel()`.
|
||||
|
||||
Sections 1 to 4 are the parts of one commit; section 5 is not a commit. The commit passes `make test` with the test stack up, which runs `make lint` first and then every package one at a time (`-p 1`). No golden, route table or existing test result changes: `stripetest.Line` prints neither new field of `Call`, and a release sent with params that carry only a context encodes the same empty form that `nil` encoded. Any such change is a stop. A script's usage is learned from its header or source, never by running it.
|
||||
|
||||
**Breaks.** The list of item 3.5 is written to a file outside the repository and run with `go run ./scripts/mutation-check -list <file> -side new` on the commit, from the repository root, with `test/.env` loaded as `make test` loads it (without it the database tests skip and every record reports SKIPPED), after its green control run; the tests are new, so there is no old side. A break of Go code is a test record (`file`, `old`, `new`, `pkg`, `run_new`, `want_new`). A break caught by two tests is two records, one per test, with the same `file`, `old` and `new`. Every record's `want_new` is a text the failing test prints only when it fails, never a test or subtest name alone. Every record must be CAUGHT. An `old` text below is the code as the section's items write it; when the committed code words it differently, the record is corrected to the committed text before the run, never dropped, and its `new` text keeps the package compiling. A break that no test of the commit sees is named with the reason, and the commit message says so. The commit message lists every break with its result.
|
||||
|
||||
## 1. The reconcile's Stripe calls carry its context
|
||||
|
||||
- [x] 1.1 `fetchSubscription(ctx, stripeSubscriptionID)` (`internal/fulfillment/reconcile.go:229-238`, design D1) sets `getParams.Context = ctx`; `reconcile` passes its own `ctx` (`:85`). Its doc comment says that the fetch carries the reconcile's context, so a reconcile whose context is already done fails here, before its transaction opens, and writes nothing.
|
||||
- [x] 1.2 `applyCollectionAction(ctx, logger, stripeSubscriptionID, action)` (`internal/fulfillment/collection.go:109-119`) sets `params.Context = ctx` on the params `collectionParams(action)` returns and sends them with `subscription.Update`; `reconcile` passes its own `ctx` (`reconcile.go:102`). `collectionParams` is unchanged. The doc comment adds that a call its context cuts may have landed at Stripe, and that the outbox's retry decides again from a fresh fetch (design D4).
|
||||
- [x] 1.3 `releaseSpentSchedule` (`reconcile.go:1285-1298`) sends `&stripe.SubscriptionScheduleReleaseParams{Params: stripe.Params{Context: ctx}}` in place of `nil` (`:1289`). Its doc comment adds that a release its context cuts is handled as a refused one: it may have landed, and the next reconcile either finds no schedule attached and supersedes the downgrades, or releases the schedule again.
|
||||
- [x] 1.4 `ReconcileLatestSubscriptionForCustomer` (`reconcile.go:466-484`) sets `listParams.Context = ctx` before `subscription.List` (`:470-475`). Its doc comment says that the list carries the context the reconcile it starts takes.
|
||||
- [x] 1.5 `ReconcileSubscription`'s doc comment (`reconcile.go:37-56`) gains one sentence: every Stripe call it makes carries `ctx`, so the caller's deadline bounds them, and it sets no deadline of its own.
|
||||
|
||||
## 2. The Stripe mock records the context and answers a done one
|
||||
|
||||
- [x] 2.1 `internal/stripetest/mock.go` (design D2): `Call` gains `Deadline time.Time`, the deadline of the request's `Params.Context` (zero when the call carries no context, or one without a deadline), and `ContextDone bool`, true when that context was already done as the mock received the call. `newCall` reads both from `params.GetParams().Context` inside its typed-nil guard (`:204-210`); a list's context arrives the same way, because `CallRaw` passes on the `*stripe.Params` that stripe-go built with `ListParams.ToParams`. `Line` prints neither field. The fields' doc comment says why a test reads them: the mock never waits, so a deadline is visible only as recorded.
|
||||
- [x] 2.2 `MockBackend.Call` answers a call whose context is already done with that context's error, after `record` and ahead of `Err`, `PathErrs` and `Handle`, reading the context from the params rather than from `ContextDone`. `MockBackend`'s doc comment (`:30-33`) and `Call`'s (`:170-172`) say that a done context answers first, as stripe-go's HTTP client returns the context's error without sending the request, and `OnCall`'s (`:96-101`) names a done context beside `Err` and `PathErrs`.
|
||||
|
||||
## 3. The tests and their breaks
|
||||
|
||||
- [x] 3.1 `TestReconcileCallsCarryTheContext` (a new `internal/fulfillment/reconcile_context_test.go`, package `fulfillment_test`), rows through a named function (`checkCallsCarryTheContext`). Each row runs its entry under `context.WithTimeout(context.Background(), time.Hour)`, takes the calls the mock recorded after a mark set before the run, and names each by its method and path: `the fetch` (`GET /v1/subscriptions/{id}`), `the set` (`POST /v1/subscriptions/{id}`), `the release` (`POST /v1/subscription_schedules/{id}/release`) and `the list` (`GET /v1/subscriptions`).
|
||||
- `the fetch and the set`: `grantOver(t, false)` (a subscription a grant displaces, no pause at Stripe) with `stripeAppliesCollection`, then `ApplySubscriptionCollection`; the calls are the fetch, the set and the fetch.
|
||||
- `the release of a spent schedule`: `laterUpgrade` (`release_schedule_after_commit_test.go:34-45`), then `ApplySubscriptionCollection`; the calls are the fetch and the release.
|
||||
- `the list of the checkout return`: `seedFixture` with a mock whose `Sub` is the fixture's active subscription and whose `List` holds it, then `ReconcileLatestSubscriptionForCustomer`; the calls are the list and the fetch.
|
||||
|
||||
A row first checks that it sent the calls it names, in that order, so a fixture that makes no such call cannot pass it; failure text `%s: the reconcile sent %v, want %v`. Then each call's `Deadline` equals the context's; failure text `%s: %s carried deadline %v, want %v` (the row, the call's name, the deadline recorded, the context's).
|
||||
- [x] 3.2 `TestReconcileWithADoneContextFailsAtItsFetch` (same file): `seedFixture`, a mock whose `Sub` is the fixture's active subscription, never reconciled, and `ReconcileSubscription` under a context cancelled before the call. The error wraps `context.Canceled` and is the fetch's (it contains `reconcile: fetch subscription`); failure text `a reconcile under a done context returned %v, want the fetch's error`. The mock received one call, the fetch, with `ContextDone` true; failure text `a reconcile under a done context sent %v, want the fetch alone, done`. `stripe.subscription_mappings` has no row for the fixture's Stripe subscription id; failure text `a reconcile under a done context left %d mapping rows, want 0`. The three checks report with `t.Errorf`, and the first tests `err` for nil before it reads its text.
|
||||
- [x] 3.3 `TestCallRecordsTheContextsDeadline` (`internal/stripetest/mock_test.go`), rows `a deadline` (a context whose deadline is an hour away), `a cancelled context` (a context cancelled before the call) and `no context`. Each row installs a mock whose `Sub` answers the read, so a call answered as live succeeds, sends `subscription.Get("sub_1", params)` with the row's context as `params.Context`, and reads the one call recorded. Failure texts: `%s: call deadline %v and done %v, want %v and %v`; `%s: the call answered %v, want %v` (no error for `a deadline` and `no context`, `context.Canceled` for `a cancelled context`, compared with `errors.Is`). For `a deadline` and `no context` it also checks that the answer is `Sub`: failure text `%s: the call answered subscription %q, want sub_1`. `no context` is a guard row: a call with no context takes the nil paths of `newCall` and of the answer, and an edit that breaks them panics, so it has no break, and the commit message says so.
|
||||
- [x] 3.4 `TestNextReconcileSupersedesADowngradeLeftAfterItsRelease` keeps its result (design D2): it cancels its context inside `Handle`, after the mock has received the release with a live context.
|
||||
- [x] 3.5 Breaks:
|
||||
- `fetchSubscription` sending `context.Background()` in place of its `ctx` (the record's `old` is `getParams.Context = ctx`): caught by `TestReconcileCallsCarryTheContext/the_fetch_and_the_set`, `want_new` `the fetch and the set: the fetch carried deadline`, and by `TestReconcileWithADoneContextFailsAtItsFetch`, `want_new` `want the fetch's error`;
|
||||
- `applyCollectionAction`'s update sent without the context (the record's `old` is its `params.Context = ctx`): caught by `TestReconcileCallsCarryTheContext/the_fetch_and_the_set`, `want_new` `the fetch and the set: the set carried deadline`;
|
||||
- the release sent with `nil` params again (the record's `old` is `&stripe.SubscriptionScheduleReleaseParams{Params: stripe.Params{Context: ctx}}`): caught by `TestReconcileCallsCarryTheContext/the_release_of_a_spent_schedule`, `want_new` `the release of a spent schedule: the release carried deadline`;
|
||||
- the list sent without the context (the record's `old` is `listParams.Context = ctx`): caught by `TestReconcileCallsCarryTheContext/the_list_of_the_checkout_return`, `want_new` `the list of the checkout return: the list carried deadline`;
|
||||
- `newCall` leaving `Deadline` zero: caught by `TestCallRecordsTheContextsDeadline/a_deadline`, `want_new` `a deadline: call deadline`;
|
||||
- `newCall` leaving `ContextDone` false: caught by `TestCallRecordsTheContextsDeadline/a_cancelled_context`, `want_new` `a cancelled context: call deadline`, and by `TestReconcileWithADoneContextFailsAtItsFetch`, `want_new` `want the fetch alone, done`;
|
||||
- `MockBackend.Call` answering a done context as a live one (the answer of item 2.2 removed): caught by `TestCallRecordsTheContextsDeadline/a_cancelled_context`, `want_new` `a cancelled context: the call answered`, and by `TestReconcileWithADoneContextFailsAtItsFetch`, `want_new` `want the fetch's error`.
|
||||
- `MockBackend.Call` answering every call that carries a context with that context's error (the done check's condition reduced to the context being set): caught by `TestCallRecordsTheContextsDeadline/a_deadline`, `want_new` `a deadline: the call answered subscription`;
|
||||
- `ReconcileSubscription` running its body under a context its caller cannot cancel (the record's `old` is `_, err := reconcile(ctx, db, logger, keyMode, stripeSubscriptionID, reason, false)`, its `new` the same with `context.WithoutCancel(ctx)` in place of `ctx`): caught by `TestReconcileWithADoneContextFailsAtItsFetch`, `want_new` `left 1 mapping rows, want 0`.
|
||||
|
||||
## 4. The documents
|
||||
|
||||
- [x] 4.1 `docs/testing.md`, "Stripe-mocked tests", the Helper bullet (`:186-200`, design D5): after the sentence on `m.OnCall` and `stripetest.Line`, it says that a call records the deadline its request's context carried (`Call.Deadline`) and whether that context was already done (`Call.ContextDone`), that a call whose context was already done answers the context's error, as the SDK's HTTP client does, and that the mock never waits, so a test reads a deadline as recorded.
|
||||
- [x] 4.2 No other document changes: `docs/stripe.md` and the rest of `docs/` state no timeout for the reconcile's Stripe calls.
|
||||
|
||||
## 5. Landing
|
||||
|
||||
- [x] 5.1 With the test stack up, `make test` passes (it runs `make lint` first), and `openspec validate reconcile-calls-carry-context --strict` passes.
|
||||
- [x] 5.2 The break list of item 3.5 reads CAUGHT in the commit message.
|
||||
- [ ] 5.3 After the push, post on #285 under `CONTRIBUTING.md`'s rules for forge text (item 1: it reads for someone who has never opened the repository, explains the console's terms once, and names no decision number, change name, date as narrative, deployment or count), and close it. Draft:
|
||||
|
||||
> The subscription reconcile is the step that reads a subscription from Stripe and brings the console's records and entitlements in line with it. Each Stripe call it makes now carries its caller's context, so the call ends when the caller's deadline passes or the caller is cancelled, instead of running under the Stripe library's own 80-second timeout and two retries. Its calls are the read of the subscription, the release of a subscription schedule the console attached that has nothing left to do, the pause or clear of the subscription's payment collection made from the outbox (the console's queue of Stripe actions, retried in the background), and the list of a customer's latest subscription with which the checkout return (the dashboard a member lands on after paying at Stripe) starts. A webhook event's reconcile now ends with its one-minute attempt, an outbox entry's with its two-minute attempt, a member's plan change with the request's 32 seconds, and the checkout return's after its five seconds. A reconcile whose context has already ended fails at the read and writes nothing. A release or a collection change that a deadline cuts may still have reached Stripe; the next reconcile reads the subscription again and either finds the change made or makes it again. Tests read, through the Stripe mock, the deadline each call carried.
|
||||
>
|
||||
> Commit: …
|
||||
|
||||
- [ ] 5.4 The archive waits for the maintainer's go; the delta carries the spec change, and no Purpose needs an edit a delta cannot make. Section 11 of `grant-pauses-subscription-billing` (#278) lands after this change.
|
||||
Reference in New Issue
Block a user