-
Notifications
You must be signed in to change notification settings - Fork 637
[SDK] Fix: Parse x402 payment header #8617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: f6b8b8f The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@jelilat is attempting to deploy a commit to the thirdweb Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughPrefer decoding x402 payment requirements from a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Decoder
Client->>Server: fetch(request)
Server-->>Client: 402 Payment Required (may include `payment-required` header or JSON body)
alt Header present
Client->>Decoder: read `payment-required` header
Decoder->>Decoder: safeBase64Decode(header) -> parsedPayload
else Header absent
Client->>Client: await response.json() -> parsedPayload
end
Client->>Client: normalize parsedPayload -> {x402Version, accepts, error}
Client->>Client: select requirement, possibly switch chain
Client->>Client: create X-PAYMENT header
Client->>Server: fetch(request + X-PAYMENT)
Server-->>Client: 200 OK / success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/thirdweb/src/x402/fetchWithPayment.ts:
- Around line 83-108: The code assumes parsed.accepts/body.accepts is always an
array before calling .map(), causing runtime errors if missing or invalid;
update the payment-required header branch (paymentRequiredHeader -> decoded ->
parsed) and the fallback body branch (await response.json() -> body) to
defensively validate that accepts is an array (Array.isArray(parsed.accepts) /
Array.isArray(body.accepts)) before mapping: either default to an empty array or
throw a clear error via processLogger/throw with contextual info, then pass each
item through RequestedPaymentRequirementsSchema.parse; also ensure error
handling for JSON.parse/safeBase64Decode and response.json failures is
preserved.
🧹 Nitpick comments (3)
packages/thirdweb/src/x402/fetchWithPayment.ts (1)
85-96: Consider wrapping header decoding in try-catch for robustness.If the
payment-requiredheader contains malformed base64 or invalid JSON,safeBase64DecodeorJSON.parsewill throw unhandled exceptions. Wrapping in try-catch with a descriptive error would improve debuggability.♻️ Optional: Add error handling for malformed headers
if (paymentRequiredHeader) { + let parsed: { x402Version: number; accepts: unknown[]; error?: string }; + try { const decoded = safeBase64Decode(paymentRequiredHeader); - const parsed = JSON.parse(decoded) as { - x402Version: number; - accepts: unknown[]; - error?: string; - }; + parsed = JSON.parse(decoded); + } catch (e) { + throw new Error( + `Failed to parse payment-required header: ${e instanceof Error ? e.message : String(e)}`, + ); + } x402Version = parsed.x402Version;packages/thirdweb/src/x402/fetchWithPayment.test.ts (2)
180-210: Duplicate test case.This test ("should parse payment requirements from payment-required header") is functionally identical to the test on lines 74-104 ("should parse payment requirements from payment-required header when present"). Both verify header parsing with the same assertions.
Consider removing one or differentiating them to test distinct edge cases.
19-59: Consider adding error scenario tests.The test suite covers happy paths well. Consider adding tests for edge cases:
- Malformed base64 in
payment-requiredheader- Missing or non-array
acceptsfield- Wallet not connected (
getAccountreturns undefined)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/thirdweb/src/x402/encode.tspackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/encode.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/encode.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/encode.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/x402/fetchWithPayment.tspackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/encode.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.tsin the same directory
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
Use predefined test accounts fromtest/src/test-wallets.tsin tests
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co-locate tests with source files using the pattern foo.ts ↔ foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
For network interactions in tests, use Mock Service Worker (MSW) to intercept fetch/HTTP calls, mocking only scenarios that are hard to reproduce
Keep tests deterministic and side-effect free; Vitest is pre-configured
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.ts
🧬 Code graph analysis (1)
packages/thirdweb/src/x402/fetchWithPayment.test.ts (1)
packages/thirdweb/src/x402/encode.ts (1)
safeBase64Decode(69-77)
🔇 Additional comments (4)
packages/thirdweb/src/x402/encode.ts (1)
69-77: LGTM!Exporting
safeBase64Decodeenables header-based payment requirement parsing infetchWithPayment.ts. The dual-environment implementation (browseratob/ Node.jsBuffer) is appropriate.packages/thirdweb/src/x402/fetchWithPayment.test.ts (3)
61-72: LGTM!Good coverage of the non-402 passthrough case. The test verifies the response is returned unchanged and fetch is only called once.
131-178: LGTM!Clever use of
maxValueconstraint to verify header takes precedence over body. This effectively proves the header path is used when both are present.
212-263: LGTM!Good integration test with a real base64-encoded payload. Testing with chain 8453 (Base) ensures the chain matching logic is exercised.
joaquim-verges
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, thank you for contributing @jelilat !
Thanks @joaquim-verges Quick question. Is coderabit going to make this change, or it's better if I do it |
|
did the change for you @jelilat and added a changeset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/thirdweb/src/x402/fetchWithPayment.ts:
- Around line 85-103: Wrap the existing header parsing (the block that reads
paymentRequiredHeader, calls safeBase64Decode, JSON.parse, and maps to
RequestedPaymentRequirementsSchema.parse) in a try-catch so malformed
base64/JSON or invalid schema does not throw and instead leaves
parsedPaymentRequirements undefined; initialize parsedPaymentRequirements (and
x402Version/error as needed) before the header block, catch any errors from
safeBase64Decode/JSON.parse/RequestedPaymentRequirementsSchema.parse and
swallow/log them as non-fatal, then only proceed to parse the body if
parsedPaymentRequirements is still undefined, and finally throw only if both
header and body parsing fail.
🧹 Nitpick comments (2)
packages/thirdweb/src/x402/fetchWithPayment.test.ts (2)
206-243: Duplicate test case.This test is nearly identical to the test at lines 78-115 ("should parse payment requirements from payment-required header when present"). Consider removing this duplicate to reduce maintenance burden.
19-59: Consider adding error case tests.The test suite covers happy paths well but lacks coverage for error scenarios:
- Malformed base64 in
payment-requiredheader- Invalid JSON after base64 decode
- Missing or non-array
acceptsfield- Zod schema validation failures
These would help verify error handling behavior and catch regressions.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/social-ads-arrive.mdpackages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each TypeScript file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes in TypeScript
Avoidanyandunknownin TypeScript unless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.) in TypeScript
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity and testability
Re-use shared types from @/types or local types.ts barrel exports
Prefer type aliases over interface except for nominal shapes
Avoid any and unknown unless unavoidable; narrow generics whenever possible
Choose composition over inheritance; leverage utility types (Partial, Pick, etc.)
Comment only ambiguous logic in TypeScript files; avoid restating TypeScript types and signatures in prose
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Place tests alongside code:foo.ts↔foo.test.tsin the same directory
Use real function invocations with stub data in tests; avoid brittle mocks
Use Mock Service Worker (MSW) for fetch/HTTP call interception in tests
Keep tests deterministic and side-effect free
Use predefined test accounts fromtest/src/test-wallets.tsin tests
UseFORKED_ETHEREUM_CHAINfor mainnet interactions andANVIL_CHAINfor isolated tests
**/*.test.{ts,tsx}: Co-locate tests with source files using the pattern foo.ts ↔ foo.test.ts
Use real function invocations with stub data in tests; avoid brittle mocks
For network interactions in tests, use Mock Service Worker (MSW) to intercept fetch/HTTP calls, mocking only scenarios that are hard to reproduce
Keep tests deterministic and side-effect free; Vitest is pre-configured
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.ts
packages/thirdweb/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/thirdweb/src/**/*.{ts,tsx}: Comment only ambiguous logic in SDK code; avoid restating TypeScript in prose
Load heavy dependencies inside async paths to keep initial bundle lean (e.g.const { jsPDF } = await import("jspdf");)Lazy-load heavy dependencies inside async paths to keep the initial bundle lean (e.g., const { jsPDF } = await import('jspdf');)
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome governs formatting and linting; its rules live in biome.json. Run
pnpm fix&pnpm lintbefore committing, ensure there are no linting errors
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lazy-import optional features; avoid top-level side-effects
Files:
packages/thirdweb/src/x402/fetchWithPayment.test.tspackages/thirdweb/src/x402/fetchWithPayment.ts
🧬 Code graph analysis (2)
packages/thirdweb/src/x402/fetchWithPayment.test.ts (1)
packages/thirdweb/src/x402/encode.ts (1)
safeBase64Decode(69-77)
packages/thirdweb/src/x402/fetchWithPayment.ts (3)
packages/thirdweb/src/x402/types.ts (1)
x402Version(14-14)packages/thirdweb/src/x402/schemas.ts (2)
RequestedPaymentRequirements(40-42)RequestedPaymentRequirementsSchema(34-38)packages/thirdweb/src/x402/encode.ts (1)
safeBase64Decode(69-77)
🔇 Additional comments (9)
.changeset/social-ads-arrive.md (1)
1-5: LGTM!Changeset is well-formed with appropriate patch-level bump for this backward-compatible bug fix.
packages/thirdweb/src/x402/fetchWithPayment.ts (3)
7-7: LGTM!Import correctly references the newly exported
safeBase64Decodefunction.
104-122: LGTM!Body parsing fallback logic correctly mirrors the header path and maintains consistent validation.
79-81: LGTM!Good approach normalizing the parsed data into shared variables before the common processing logic.
packages/thirdweb/src/x402/fetchWithPayment.test.ts (5)
5-17: LGTM!Mock setup correctly isolates external dependencies without exposing brittle implementation details.
19-59: LGTM!Well-structured test fixtures with appropriate minimal mocks that ensure test isolation via
beforeEach.
61-76: LGTM!Good baseline test verifying non-402 responses pass through unmodified.
117-204: LGTM!Good test coverage for body fallback and header preference. The maxValue-based verification at lines 188-198 is a clever technique to confirm header data takes precedence.
245-303: LGTM!Excellent end-to-end test with a realistic base64-encoded header, verifying both the decoding utility and full payment flow with chain matching.
| if (paymentRequiredHeader) { | ||
| const decoded = safeBase64Decode(paymentRequiredHeader); | ||
| const parsed = JSON.parse(decoded) as { | ||
| x402Version: number; | ||
| accepts: unknown[]; | ||
| error?: string; | ||
| }; | ||
|
|
||
| if (!Array.isArray(parsed.accepts)) { | ||
| throw new Error( | ||
| `402 response has no usable x402 payment requirements. ${parsed.error ?? ""}`, | ||
| ); | ||
| } | ||
|
|
||
| x402Version = parsed.x402Version; | ||
| parsedPaymentRequirements = parsed.accepts.map((x) => | ||
| RequestedPaymentRequirementsSchema.parse(x), | ||
| ); | ||
| error = parsed.error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for malformed header parsing.
If the payment-required header contains invalid base64 or malformed JSON, safeBase64Decode or JSON.parse will throw, preventing fallback to the body path. Consider wrapping header parsing in try-catch to gracefully fall back to body parsing when the header is present but malformed.
🛠️ Suggested fix
// Check payment-required header first before falling back to JSON body
const paymentRequiredHeader = response.headers.get("payment-required");
if (paymentRequiredHeader) {
+ try {
const decoded = safeBase64Decode(paymentRequiredHeader);
const parsed = JSON.parse(decoded) as {
x402Version: number;
accepts: unknown[];
error?: string;
};
if (!Array.isArray(parsed.accepts)) {
throw new Error(
`402 response has no usable x402 payment requirements. ${parsed.error ?? ""}`,
);
}
x402Version = parsed.x402Version;
parsedPaymentRequirements = parsed.accepts.map((x) =>
RequestedPaymentRequirementsSchema.parse(x),
);
error = parsed.error;
- } else {
+ } catch (headerError) {
+ // Header parsing failed, fall through to body parsing
+ }
+ }
+
+ // Fall back to body if header was absent or parsing failed
+ if (!parsedPaymentRequirements) {
const body = (await response.json()) as {
x402Version: number;
accepts: unknown[];
error?: string;
};
// ... rest of body parsing
}Note: This would require initializing parsedPaymentRequirements as undefined initially and checking it before body parsing.
🤖 Prompt for AI Agents
In @packages/thirdweb/src/x402/fetchWithPayment.ts around lines 85 - 103, Wrap
the existing header parsing (the block that reads paymentRequiredHeader, calls
safeBase64Decode, JSON.parse, and maps to
RequestedPaymentRequirementsSchema.parse) in a try-catch so malformed
base64/JSON or invalid schema does not throw and instead leaves
parsedPaymentRequirements undefined; initialize parsedPaymentRequirements (and
x402Version/error as needed) before the header block, catch any errors from
safeBase64Decode/JSON.parse/RequestedPaymentRequirementsSchema.parse and
swallow/log them as non-fatal, then only proceed to parse the body if
parsedPaymentRequirements is still undefined, and finally throw only if both
header and body parsing fail.
Closes #8616
PR-Codex overview
This PR introduces support for handling
402 payment-requiredheaders in thethirdwebpackage, enhancing the payment-fetching functionality by decoding payment requirements from both headers and JSON bodies.Detailed summary
safeBase64Decodefunction topackages/thirdweb/src/x402/encode.ts.wrapFetchWithPaymentinpackages/thirdweb/src/x402/fetchWithPayment.tsto:payment-requiredheader.packages/thirdweb/src/x402/fetchWithPayment.test.tsto verify:Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.