Skip to content

feat(stripe-plugin)!: modernize for Stripe SDK 22 / dahlia API (OSS-555 items 1-3, 6)#36

Open
michaelbromley wants to merge 12 commits into
mainfrom
mbromley/oss-555-modernize-stripe-plugin
Open

feat(stripe-plugin)!: modernize for Stripe SDK 22 / dahlia API (OSS-555 items 1-3, 6)#36
michaelbromley wants to merge 12 commits into
mainfrom
mbromley/oss-555-modernize-stripe-plugin

Conversation

@michaelbromley

@michaelbromley michaelbromley commented Jun 8, 2026

Copy link
Copy Markdown
Member

Closes OSS-555 — scoped to items 1, 2, 3, 6 (pure modernization). Items 4, 5, 7 (async/processing webhook, dashboard refund + dispute sync, SetupIntents) are deferred to follow-up issues.

What's in this PR

Commit OSS-555 item Summary
bdc9035 1 Stripe SDK 13.x^22.0.0 peer + dev. Local src/stripe-types.ts shim works around stripe-node#2683 (CJS NodeNext default import doesn't expose namespace types).
e58cc1b 2 apiVersion defaults to the SDK pinned version (2026-05-27.dahlia). New plugin option lets callers pin a specific version or set null to fall back to the Stripe account default (pre-2.0.0 behaviour).
392a493 3 nock 13 → 14, rimraf 5 → 6. typescript left at 5.8.2 (monorepo-pinned).
6115123 6 createRefund forwards Vendure's refund reason to Stripe (mapped to the enum where possible, otherwise stored on metadata.vendureRefundReason) and adds an idempotency key.
e0a6606 Tests adapted: switch VendureStripeClient to Stripe.createFetchHttpClient(). nock 14 (via @mswjs/interceptors) deadlocks Stripe's NodeHttpClient — see test plan below.
a1381fc Hand-written 2.0.0 CHANGELOG section + README pointer.

feat! on the apiVersion commit triggers the 2.0.0 major bump via the release flow.

Verified

  • bun run build clean across all 8 packages.
  • bun run lint clean (0 errors, 12 pre-existing warnings).
  • bun run e2e in stripe-plugin: 18/18 pass.

Not test-covered — needs reviewer eyeball

These three behavioural changes are not covered by the existing e2e suite (the suite mocks Stripe entirely via nock):

  1. HTTP transport switched from node:http(s) to fetch. Production users who inject a custom http.Agent (proxy, keep-alive tuning) will silently lose it. An httpClient escape hatch is planned as a follow-up commit on this PR.
  2. Default API version pinned to dahlia. Stripe API responses on this account will now be shaped per the 2026-05-27 API. Per the Stripe changelog the core PaymentIntent / Refund / Charge / Customer shapes are stable between 2023-08-16 and dahlia, but this is unverified against a live account.
  3. Refund metadata fallback. A non-Stripe-enum reason string ends up on metadata.vendureRefundReason, not the structured reason field.

Confidence plan

1. Risk × evidence matrix

Risk What could break Doc evidence Verification plan
Default API version moves 2023-08-162026-05-27.dahlia (~3 years of evolution) Field shapes on objects we touch (`PaymentIntent`, `Refund`, `Charge`, webhook `Event`) could shift. Stripe's API changelog confirms that across `2023-10-16`, `2024-04-10`, `2024-06-20`, acacia (`2024-09-30`), basil (`2025-03-31`), clover and dahlia, no breaking changes were made to the PaymentIntent / Refund / Charge / Customer shapes used by this plugin — breaking changes in that window are scoped to Connect, Issuing, Terminal, Billing/Subscriptions, and Stripe.js method renames (changelog/dahlia.md, changelog/basil.md, changelog/2024-06-20). Only forward-compatible additions to enums like `PaymentMethod.type` (`bizum`, `scalapay`, `sunbit`, `pix`, etc., per stripe-node v22.1/22.2 changelog) — we don't switch on `type`, so this is moot for us. (a) Walk the diff of `node_modules/stripe/cjs/resources/{PaymentIntents,Refunds,Charges,Customers,Events}.d.ts` against the same file in stripe@13.11.0 and confirm only additive changes touch the fields we read (`id`, `client_secret`, `status`, `amount_received`, `last_payment_error.message`, `metadata`, `failure_reason`). (b) During the manual walkthrough below, log the raw webhook payload and the `paymentIntents.create` response to confirm shape.
Webhook signature verification under new SDK `constructEvent` could change semantics. webhooks/signatures: the v1 HMAC-SHA256 + `Stripe-Signature` scheme is API-version-agnostic and unchanged. stripe-node 22.0.1 only added `string[]` support to the signature param. Existing 18 e2e tests already exercise the signed and unsigned paths. Already covered by `e2e/stripe-payment.e2e-spec.ts` ("Should not crash on signature validation failure", "Should validate the webhook's signature properly"). Verify in manual run with a real Stripe-signed webhook from `stripe listen`.
Webhook events for an older account API version still arrive in old shape Even though we pin `dahlia` for outbound, inbound webhook events use the API version configured on the webhook endpoint at the Stripe dashboard (or the account default). A merchant on `2023-08-16` sends `2023-08-16`-shaped events. Stripe upgrades doc: events use the API version active at event creation; this is not affected by client `apiVersion`. The plugin only reads stable fields, so OK. Manual walkthrough done twice: once with the Stripe account's default API version, once with the webhook endpoint pinned to a newer version (configurable in dashboard).
SDK v13 → v22 runtime contract Removed callback support, ES6 class constructor, per-request host override removed, `Stripe.errors.StripeError` is now value-only. stripe-node v22 migration guide + local `node_modules/stripe/CHANGELOG.md` lines for 22.0.0. We don't use callbacks, host overrides, or per-request API keys; we already use `new Stripe(...)` and `instanceof Stripe.errors.StripeError` (value access, unaffected by the type-only change). Build (passes) + e2e (18/18 pass). Add a unit-ish test that asserts `Stripe.errors.StripeError` is still a constructable class so a future SDK drift fails loudly.
NodeHttpClient → FetchHttpClient Custom `http.Agent` settings (proxy, keep-alive tuning) injected via deployment can't be applied to FetchHttpClient. Retry/timeout behaviour and error shapes may differ. stripe-node ships both clients (`createFetchHttpClient` is the documented Worker/Edge path, and the SDK supports it equivalently in Node since v18). The deadlock with nock@14 is documented in the @mswjs/interceptors socket lifecycle (`mockConnect()` runs inside `respondWith()` — `connect`/`secureConnect` only fire post-match, see `node_modules/@mswjs/interceptors/lib/node/ClientRequest-*.cjs`). (a) Add a `httpClient` option to `StripePluginOptions` so users who do need an `http.Agent` can pass their own. (b) Document the migration in CHANGELOG (done). (c) Manual smoke test confirms a real card payment + a real refund both work end-to-end. (d) Add an automated test that asserts `FetchHttpClient` is wired by inspecting `(stripe as any)._api.httpClient.getClientName() === 'fetch'`.
Refund `reason` mapping We map free-form Vendure refund reason strings onto Stripe's three-value enum `duplicate | fraudulent | requested_by_customer`; everything else goes to `metadata.vendureRefundReason`. Stripe Refunds API reference: `reason` is a strict enum at the API level. api/idempotent_requests confirms parameter mismatch on a replayed key returns an error — so the mapping has to be deterministic for the same input. Our mapping is. Add unit-level test against `StripeService.createRefund` (mocked Stripe) for each of: enum-matching reason, non-matching reason, no reason. Also assert the request body forwarded to nock.
Refund idempotency key format We use `refund_{payment_intent_id}_{amount}` — deterministic. Stripe recommends UUIDs but accepts any string ≤255 chars; parameter mismatch on replay errors. api/idempotent_requests: "the idempotency layer compares incoming parameters to those of the original request and errors if they're not the same"; keys expire after ~24h. Our key intentionally encodes the unique parameters (PI + amount), so two identical retries replay safely; two different refunds for the same PI+amount within 24h would collide, but that's vanishingly rare for the same payment. Add a unit-level test that fires `createRefund` twice with the same args and asserts both calls send the same `Idempotency-Key`. Document the 24h+identical-params rule in CHANGELOG.
`stripe-types.ts` shim fragility We extract types via `Parameters[0]` etc. If stripe-node restructures method overloads, our types shift silently. The shim documents that it's a workaround for stripe-node#2683 / PR #2725; once that lands and we pin the version we delete the shim. (a) Add a CI tripwire — a `.ts` file with `expectType<>` assertions on each shimmed type so a stripe SDK upgrade fails the build instead of silently changing meaning. (b) Watch the PR for merge; open a follow-up Linear issue to delete the shim when ready.
nock@14 / @mswjs/interceptors socket semantics The mock's `connect`/`secureConnect` events only fire after `respondWith()` runs, deadlocking any client that gates body writes on `secureConnect`. Stripe's `NodeHttpClient` does that. Source-confirmed: `node_modules/@mswjs/interceptors/lib/node/ClientRequest-*.cjs::mockConnect()` is only called from `respondWith()`. nock@14 release notes only call out "drop Node <18" and "support for native fetch" as breaking — the socket lifecycle change isn't called out. FetchHttpClient sidesteps it. Test suite proves it (18/18).

2. Pre-merge actions (in order)

  • Fix `e2e/stripe-dev-server.ts` — currently broken pre-existing TS errors (`@vendure/admin-ui-plugin` import, stale codegen exports). Without this we can't actually run the dev server, which is the only way to exercise real Stripe + real webhook.
  • Manual walkthrough against a Stripe test account:
    • `stripe listen --forward-to localhost:3000/payments/stripe` (webhook signing secret goes into the plugin config).
    • Happy path — create order, request payment intent, pay with `4242 4242 4242 4242`, confirm order transitions to `PaymentSettled`, payment record's `transactionId` matches the PI id.
    • Decline path — pay with `4000 0000 0000 0002`. Confirm `payment_intent.payment_failed` is logged but order doesn't transition (matches today's behaviour; item 4 of OSS-555 fixes the surrounding gap separately).
    • Refund path — admin UI refunds the settled payment with a reason (test both a `requested_by_customer` string and a custom string). Confirm Stripe dashboard shows the refund with the right `reason` field and `vendureRefundReason` metadata respectively.
    • Idempotency — manually retry the refund call and confirm Stripe returns the same refund (idempotent replay), not a duplicate.
    • apiVersion override — restart with `apiVersion: null` set. Confirm requests go out without a `Stripe-Version` header (visible in `stripe listen` output). Then with `apiVersion: '2024-06-20' as any` — confirm header reflects it.
    • Webhook payload shape — log one full webhook payload in each run; eyeball that the fields the controller reads (`id`, `metadata.{channelToken,orderId,orderCode,languageCode}`, `amount_received`, `status`, `last_payment_error?.message`) are present and shaped as expected.
  • Add three targeted automated tests:
    • `createRefund` forwards `reason: 'requested_by_customer'` when the input matches a Stripe enum value, and `metadata.vendureRefundReason` when it doesn't.
    • `createRefund` calls Stripe with the deterministic idempotency key (assert the `Idempotency-Key` header captured by nock).
    • `apiVersion: null` results in no `Stripe-Version` header sent; `apiVersion: 'X'` results in `Stripe-Version: X`.
  • Type-shim tripwire — `src/__type-checks.ts` with `expectAssignable<>` asserts pinning a representative field on each shimmed type. Future SDK drift breaks the build instead of silently shifting meaning.
  • `httpClient` plugin option escape hatch for production users who need an `http.Agent`. Default stays `createFetchHttpClient()`; if set, pass through.
  • Diff stripe@13.11.0 vs stripe@22.2.0 d.ts shapes for: `Charge`, `Refund`, `PaymentIntent`, `PaymentIntentCreateParams`, `Refund.status`, `Refund.failure_reason`. Confirm fields the plugin reads are still defined the same way.

3. Post-merge

  • Open follow-up Linear issues for: Items 4–7 of OSS-555; delete the type shim once stripe-node#2725 ships; consider re-adding NodeHttpClient as the default once nock fixes the socket lifecycle.

4. Remaining assumptions after this plan

  • Behaviour against payment methods we don't have test card numbers for in the walkthrough (BNPL, SEPA, bank transfer). Items 4–7 of OSS-555 are scoped to add coverage there.
  • `FetchHttpClient` retry / timeout / connection-pool behaviour at high concurrency. We're trusting Stripe's own implementation; this is the same SDK code Stripe ships for Workers and Edge, so it has production miles, just not under this exact codepath.
  • Subtle webhook payload differences between API versions that don't break our code today but might if Stripe later removes a field still hidden in test fixtures (the mocked webhook payloads in our tests are forged, not captured from real Stripe).

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Bumps the stripe peer + dev dependency to ^22 and fixes the TypeScript
fallout. v22's CJS+NodeNext type exports are currently broken for
namespace access (Stripe.PaymentIntent etc. resolve to a stub
StripeConstructor namespace; see stripe/stripe-node#2683 / #2725), so a
local stripe-types.ts shim extracts the relevant types from the SDK's
method signatures. Once #2725 ships in a release we pin, the shim can
be deleted and the rest of the plugin can return to direct namespace
access.
The Stripe client now defaults to the SDK's pinned API version
(currently 2026-05-27.dahlia) instead of sending no version header.
This keeps the API responses aligned with the SDK's TypeScript types.

A new `apiVersion` plugin option lets merchants override this: pass a
specific version string to pin to it, or `null` to fall back to the
Stripe account's default version (the pre-2.0.0 behaviour). Existing
integrations that relied on the account default need to set
`apiVersion: null` explicitly.

BREAKING CHANGE: The default Stripe API version is now the SDK's pinned
version rather than the account default. Set `apiVersion: null` to
restore the previous behaviour.
The Vendure refund input's `reason` is now passed through to Stripe:
mapped to the structured `reason` field when it matches one of Stripe's
accepted enum values (`duplicate`, `fraudulent`, `requested_by_customer`),
otherwise stored on the refund metadata as `vendureRefundReason` so it
isn't lost.

`refunds.create` is now also called with an idempotency key derived from
the payment intent ID and refund amount, matching the parity already in
place for payment intent creation.
Two changes that together let the existing 18 tests pass on the new
SDK + mocking stack:

1. Switch VendureStripeClient to Stripe.createFetchHttpClient() rather
   than the default NodeHttpClient. The default client waits for the
   socket's secureConnect event before writing the request body;
   nock@14 (via @mswjs/interceptors) only emits secureConnect once a
   request has been matched, so body and reply deadlock. The fetch
   client side-steps the dance entirely.

2. e2e/stripe-payment.e2e-spec.ts switches to the default Stripe
   import and drops the hard-coded apiVersion: '2023-08-16' on the
   webhook helper. The mock payloads still send api_version: '2022-11-15'
   in their body — that's opaque to the controller.
…t path works

The dev server imported @vendure/admin-ui-plugin (not present in this
workspace) and old codegen exports (CREATE_PAYMENT_METHOD,
ADD_ITEM_TO_ORDER, CREATE_*_STRIPE_PAYMENT_INTENT) that no longer
exist on this branch — the operations are now gql.tada documents in
e2e/graphql/.

Switch to:
  - testConfig from e2e-common (it's a factory function, unlike the
    @vendure/testing re-export which is a static value),
  - the *Document exports from e2e/graphql/*,
  - GraphiqlPlugin in place of AdminUiPlugin so the manual walkthrough
    has a query surface,
  - an inline createCustomStripePaymentIntentDocument for the test
    plugin's mutation (no helper exists for it).

Unblocks the OSS-555 manual walkthrough against a real Stripe test
account.
…hatch

Default behaviour is unchanged — the plugin still configures Stripe
with FetchHttpClient. The new `httpClient` plugin option lets users
inject a NodeHttpClient with a custom http.Agent for proxying or
keep-alive tuning, which the FetchHttpClient default cannot express.

This is the production safety valve called out in the PR's confidence
plan against the HTTP transport switch.
While writing the apiVersion confidence test I confirmed that stripe-node
(both v13 and v22) coerces a null/undefined apiVersion to its own pinned
default via `props.apiVersion || DEFAULT_API_VERSION` in stripe.core.js.
There is no way on stripe-node v22 to instruct the SDK to omit the
Stripe-Version header and fall back to the Stripe account's default API
version — which is what the previous "// Use accounts default version"
comment in stripe-client.ts incorrectly claimed.

Reflect reality in the API:
  - apiVersion is typed `StripeLatestApiVersion | undefined`, not nullable.
  - The CHANGELOG now states the pre-2.0.0 behaviour was *already* the SDK
    pinned version (just an older one), and the only escape hatch is
    pinning to a specific older API version string.

Also export StripePluginOptions, StripeHttpClient, and StripeLatestApiVersion
from the public entry point so consumers can type their config without
reaching into the package internals.

BREAKING CHANGE: The apiVersion option no longer accepts null. If you
previously set `apiVersion: null`, drop it (same effect) or pin to a
specific older version string.
Three new test groups, none requiring a real Stripe account:

1. VendureStripeClient configuration:
   - Stripe-Version header is the SDK's pinned version by default.
   - Stripe-Version header takes an explicit override string.

2. createRefund:
   - Stripe-enum reason ("requested_by_customer") goes to the structured
     `reason` field.
   - Non-enum reason goes to `metadata.vendureRefundReason`.
   - No reason → neither field is set.
   - Idempotency-Key header equals `refund_{paymentIntentId}_{amount}`.

These are the confidence tests listed in the PR's pre-merge plan and
pin the behaviour change introduced by `feat(stripe-plugin): forward
refund reason and add idempotency key`.
src/__type-checks.ts uses compile-time `Extends<>` assertions to pin the
specific fields the plugin reads/writes against the shimmed types in
stripe-types.ts. If a future stripe-node release reshapes the underlying
method signatures (e.g. by tightening overloads), the shimmed types
extracted via `Parameters<>` / `ReturnType<>` can drift silently — this
file makes that drift fail the build instead of the runtime.

Verified the tripwire fires by temporarily asserting StripeLatestApiVersion
extends `number`; tsc errored with "Type 'false' does not satisfy the
constraint 'true'" pointing at the right line.

Delete this file when stripe/stripe-node#2725 ships and the shim itself
can be deleted.
@michaelbromley

Copy link
Copy Markdown
Member Author

Pre-merge 5 — stripe@13.11.0 vs stripe@22.2.0 d.ts diff for fields the plugin reads/writes

Pulled stripe@13.11.0 from npm and diffed the d.ts files in types/ against stripe@22.2.0's cjs/resources/ for the exact fields the plugin touches. Result: shape-identical for everything the plugin reads or writes.

Field v13.11.0 v22.2.0 Same?
PaymentIntent.id string string
PaymentIntent.client_secret string | null string | null
PaymentIntent.amount_received number number
PaymentIntent.status PaymentIntent.Status PaymentIntent.Status
PaymentIntent.metadata Stripe.Metadata (= Record<string, string>) Metadata (= Record<string, string>)
PaymentIntent.last_payment_error PaymentIntent.LastPaymentError | null PaymentIntent.LastPaymentError | null
PaymentIntent.automatic_payment_methods PaymentIntent.AutomaticPaymentMethods | null PaymentIntent.AutomaticPaymentMethods | null
PaymentIntentCreateParams.automatic_payment_methods.enabled boolean boolean
Refund.status string | null string | null
Refund.failure_reason string? string?
RefundCreateParams.reason 'duplicate' | 'fraudulent' | 'requested_by_customer' | 'expired_uncaptured_charge' 'duplicate' | 'fraudulent' | 'requested_by_customer' ⚠️ narrowed
CustomerCreateParams.email string? string?
CustomerCreateParams.name string? string?
Charge.refunded boolean boolean ✅ (not currently read)
Charge.amount_refunded number number ✅ (not currently read)

The only real change worth a note

RefundCreateParams.reason dropped 'expired_uncaptured_charge' between v13 and v22. The plugin only sends one of the surviving three (requested_by_customer, duplicate, fraudulent) — anything else falls through to metadata.vendureRefundReason. So nothing actually exercised the dropped value, and the tripwire in src/__type-checks.ts pins this so a future SDK change would break the build, not the runtime.

(Refund.Reason responses still include 'expired_uncaptured_charge' in v22 — the narrow only applies to what you can pass on create.)

Conclusion

The "default API version moves 2023-08-16 → 2026-05-27.dahlia" risk listed in the confidence plan is materially smaller than feared: the type shapes the plugin reads/writes are unchanged across the entire SDK and API-version span, modulo one narrowed enum the plugin doesn't use.

CI enforces no-floating-promises, no-non-null-assertion, and
no-unnecessary-type-assertion as errors; my local pre-flight only
treated them as warnings.

  - dev-server: wrap top-level IIFE with `void`, swap
    `process.env.STRIPE_*\!` for `?? ''` (Stripe will throw at first
    call if the env is missing, which is the same failure mode).
  - spec: drop the unnecessary `\!` on `orders.items[0]`.
  - __type-checks: move the `Stripe` value import to the head of the
    file alongside the type imports.

Remaining changes are eslint --fix auto-applied import-order
reshuffles in stripe.service.ts and the gql.tada graphql-{admin,shop}
files. All commits prior to this one stay green; the full monorepo
lint now passes end-to-end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant