feat(treaty): operation-first client with discriminated results#78
Merged
Conversation
- Add errorStatusKeys to Operation and operationMetadata for runtime error Zod lookup
- Replace data/error envelope with kind: success | http | transport | parse | unknown
- Implement createTreatyClient({ operations, operationMetadata }) with Zod parsing
- Generate operations + operationMetadata; keep Eden-style chain as (documented)
- Export unwrap/orThrow, TreatyOperationsClient, TreatyResultFor, TreatyRouteTreeClient
- Update treaty and CLI tests; refresh snapshots
Co-authored-by: RawToast <RawToast@users.noreply.github.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 40 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR implements an operation-first Treaty HTTP client API, replaces shape-based results with a discriminated Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client Code
participant TC as TreatyClient
participant EO as executeOperation
participant Fetch as fetch API
participant Parser as Response Parser
participant Categorizer as Error Categorizer
App->>TC: client.someOperation({params, query, body, headers, init})
TC->>EO: executeOperation(operation, request, metadata, baseUrl)
EO->>EO: Build URL, headers, body
EO->>Fetch: fetch(url, init)
alt Success Path
Fetch-->>EO: Response (2xx)
EO->>Parser: Read & parse body (JSON or text)
Parser-->>EO: Parsed data / raw text
EO-->>TC: {kind: "success", status, data, response, headers}
else HTTP Error Path
Fetch-->>EO: Response (4xx/5xx)
EO->>Categorizer: Map status → specStatus via errorStatusKeys
EO->>Parser: Parse error body
EO-->>TC: {kind: "error", specStatus, error, status, response, headers}
else Parse Error
Parser-->>EO: parse fails
EO-->>TC: {kind: "unexpectedError", subtype: "parse", error, rawBody}
else Transport Error
Fetch-->>EO: fetch rejects
EO-->>TC: {kind: "unexpectedError", subtype: "transport", error}
end
TC-->>App: Promise<TreatyResult>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #78 +/- ##
==========================================
+ Coverage 96.20% 97.31% +1.11%
==========================================
Files 19 20 +1
Lines 2765 3130 +365
==========================================
+ Hits 2660 3046 +386
+ Misses 105 84 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/zenko/src/treaty-types.ts (1)
56-59: Enhanceunwraperror message with more context.The error message only includes
result.kind, but for debugging it would be helpful to include the actual error details when available. Consider includingresult.errororresult.statusinformation.♻️ Proposed enhancement for better debugging
export function unwrap<T>(result: TreatyResult<T>): T { if (result.kind === "success") return result.data - throw new Error(`Treaty unwrap failed: ${result.kind}`) + if (result.kind === "error") { + throw new Error(`Treaty unwrap failed: HTTP ${result.status} (${result.specStatus})`) + } + const err = result.error instanceof Error ? result.error.message : String(result.error) + throw new Error(`Treaty unwrap failed: ${result.subtype} - ${err}`) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/zenko/src/treaty-types.ts` around lines 56 - 59, The unwrap function throws a vague Error when TreatyResult is not a success; update unwrap (export function unwrap<T>(result: TreatyResult<T>)) to include additional context from the non-success result (e.g., result.error and/or result.status) in the thrown error message so logs show the underlying error/details; construct a descriptive message combining result.kind plus any available result.error and result.status (safely handling undefined) before throwing.packages/examples/generate.js (1)
149-170: Consider parallelizing treaty module generation.The three treaty module generations are independent and could run in parallel using
Promise.allfor faster execution.♻️ Optional: Parallelize treaty generation
- const trainTravelTreatySuccess = await generateTreatyModuleForGen( - "train-travel.gen.ts", - "train-travel.treaty.gen.ts", - "train-travel" - ) - const authApiTreatySuccess = await generateTreatyModuleForGen( - "auth-api.gen.ts", - "auth-api.treaty.gen.ts", - "auth-api" - ) - const petstoreTreatySuccess = await generateTreatyModuleForGen( - "petstore.gen.ts", - "petstore.treaty.gen.ts", - "petstore" - ) - if ( - !trainTravelTreatySuccess || - !authApiTreatySuccess || - !petstoreTreatySuccess - ) { + const treatyResults = await Promise.all([ + generateTreatyModuleForGen( + "train-travel.gen.ts", + "train-travel.treaty.gen.ts", + "train-travel" + ), + generateTreatyModuleForGen( + "auth-api.gen.ts", + "auth-api.treaty.gen.ts", + "auth-api" + ), + generateTreatyModuleForGen( + "petstore.gen.ts", + "petstore.treaty.gen.ts", + "petstore" + ), + ]) + if (treatyResults.some((success) => !success)) { process.exit(1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/generate.js` around lines 149 - 170, The three independent calls to generateTreatyModuleForGen (for "train-travel", "auth-api", "petstore") should be run in parallel instead of sequentially; replace the sequential awaits that set trainTravelTreatySuccess, authApiTreatySuccess, and petstoreTreatySuccess with a single Promise.all call that awaits all three promises and then destructures or assigns the three results, and keep the existing combined failure check (the if that calls process.exit(1)) unchanged to handle any failures.packages/examples/src/__tests__/petstore-treaty-fetch.test.ts (1)
206-208: Consider adding a comment explaining the unused client.This module-level client instantiation appears to be for compile-time type checking of the generated client's API surface rather than runtime testing.
📝 Suggested clarification
+/** Type-check only: verify the generated client API compiles. */ const _client = createClient("http://petstore.swagger.io/v1", { fetch: mock<typeof fetch>() as unknown as typeof fetch, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/__tests__/petstore-treaty-fetch.test.ts` around lines 206 - 208, The module-level const _client is instantiated only for compile-time type checking of the generated client API and not used at runtime; add a brief comment immediately above the createClient call (referencing _client and createClient) that explains this intent (e.g., "instantiate only for compile-time type checks of generated client surface; unused at runtime") and keep the leading underscore to suppress unused-variable warnings so linters/readers understand it's deliberate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/treaty-client.md`:
- Line 3: Update the opening sentence so it uses the correct indefinite article:
change "a Eden Treaty-style HTTP client" to "an Eden Treaty-style HTTP client"
in the phrase "Zenko can emit a Eden Treaty-style HTTP client" (look for the
exact string "Eden Treaty-style HTTP client" at the start of the document).
In `@packages/zenko/src/__tests__/treaty-generator.test.ts`:
- Around line 42-45: Replace the four permissive substring assertions in
treaty-generator.test.ts with a single snapshot assertion: remove the
expect(output).toContain(...) checks and call
expect(output).toMatchSnapshot("treaty-generated-module - full output") (or
another descriptive name) to capture the entire generated module; keep the test
harness that produces output and ensure the snapshot name references the
generated module so it is a descriptive, stable snapshot per the coding
guidelines.
---
Nitpick comments:
In `@packages/examples/generate.js`:
- Around line 149-170: The three independent calls to generateTreatyModuleForGen
(for "train-travel", "auth-api", "petstore") should be run in parallel instead
of sequentially; replace the sequential awaits that set
trainTravelTreatySuccess, authApiTreatySuccess, and petstoreTreatySuccess with a
single Promise.all call that awaits all three promises and then destructures or
assigns the three results, and keep the existing combined failure check (the if
that calls process.exit(1)) unchanged to handle any failures.
In `@packages/examples/src/__tests__/petstore-treaty-fetch.test.ts`:
- Around line 206-208: The module-level const _client is instantiated only for
compile-time type checking of the generated client API and not used at runtime;
add a brief comment immediately above the createClient call (referencing _client
and createClient) that explains this intent (e.g., "instantiate only for
compile-time type checks of generated client surface; unused at runtime") and
keep the leading underscore to suppress unused-variable warnings so
linters/readers understand it's deliberate.
In `@packages/zenko/src/treaty-types.ts`:
- Around line 56-59: The unwrap function throws a vague Error when TreatyResult
is not a success; update unwrap (export function unwrap<T>(result:
TreatyResult<T>)) to include additional context from the non-success result
(e.g., result.error and/or result.status) in the thrown error message so logs
show the underlying error/details; construct a descriptive message combining
result.kind plus any available result.error and result.status (safely handling
undefined) before throwing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be0aa5eb-6d6d-4200-8df2-f09d35e0e3d9
⛔ Files ignored due to path filters (13)
packages/zenko/src/__tests__/__snapshots__/anyof-combinations.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/cli.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/date-enum.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/inline-response-array.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/no-response-content.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/non-json-responses.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/oneof-discriminator.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/petstore.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/security-schemes.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/selective-operations.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/tictactoe.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/train-travel.test.ts.snapis excluded by!**/*.snappackages/zenko/src/__tests__/__snapshots__/zenko.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (24)
.bun-scan.config.json.github/workflows/ci.ymlREADME.mddocs/plans/2026-04-14-treaty-client-redesign.mddocs/treaty-client.mdpackages/examples/generate.jspackages/examples/src/__tests__/auth-api-treaty-fetch.test.tspackages/examples/src/__tests__/petstore-treaty-fetch.test.tspackages/examples/src/__tests__/train-travel-treaty-fetch.test.tspackages/zenko/index.tspackages/zenko/package.jsonpackages/zenko/src/__tests__/cli.test.tspackages/zenko/src/__tests__/treaty-client-types.test.tspackages/zenko/src/__tests__/treaty-generator.test.tspackages/zenko/src/__tests__/treaty-runtime.test.tspackages/zenko/src/core/operation-parser.tspackages/zenko/src/treaty-generator.tspackages/zenko/src/treaty-infer.tspackages/zenko/src/treaty-types.tspackages/zenko/src/treaty.tspackages/zenko/src/types/operation.tspackages/zenko/src/utils/treaty-tree.tspackages/zenko/src/zenko.tsturbo.json
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/examples/src/__tests__/petstore-treaty-fetch.test.ts (2)
1-1: Stop patchingglobal.fetchin this suite.Every test already injects
fetchviacreateClient(origin, { fetch }),
so the global override only adds shared state and potential bleed between
tests. Removing it also makes theafterEachreset unnecessary.♻️ Small cleanup
-import { afterEach, describe, expect, it, mock } from "bun:test" +import { describe, expect, it, mock } from "bun:test" ... - const originalFetch = global.fetch - - afterEach(() => { - global.fetch = originalFetch - }) ... function setupFetchMock() { - const fetchMock = mock<typeof fetch>() - global.fetch = fetchMock as unknown as typeof fetch - return fetchMock + return mock<typeof fetch>() }Also applies to: 6-10, 200-203
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/__tests__/petstore-treaty-fetch.test.ts` at line 1, The test suite is patching global.fetch unnecessarily; remove the global.fetch override and the related afterEach reset so each test uses the per-test injected fetch via createClient(origin, { fetch }); specifically, delete the global.fetch assignment and the afterEach cleanup in packages/examples/src/__tests__/petstore-treaty-fetch.test.ts (and the similar blocks at the other occurrences noted), and ensure tests construct clients with createClient(origin, { fetch }) as intended so no global shared state remains.
206-210: Avoid the unused_clientsentinel.This call is fine, but the binding itself is intentionally unread and will
fightnoUnusedLocalsanywhere this file is included in a strict TS
project. A no-binding expression keeps the same type-check without the dead
local.♻️ Minimal change
-const _client = createClient("http://petstore.swagger.io/v1", { +void createClient("http://petstore.swagger.io/v1", { fetch: mock<typeof fetch>() as unknown as typeof fetch, })As per coding guidelines:
**/*.{ts,tsx}: Enable TypeScript strict mode with flags including noUncheckedIndexedAccess, noUnusedLocals, noUnusedParameters, and noFallthroughCasesInSwitch🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/examples/src/__tests__/petstore-treaty-fetch.test.ts` around lines 206 - 210, The test creates a compile-time-only sentinel `_client` which triggers noUnusedLocals; replace the binding with an unassigned expression so types still check but no dead local remains: remove `const _client =` and instead call the `createClient("http://petstore.swagger.io/v1", { fetch: mock<typeof fetch>() as unknown as typeof fetch })` expression without assigning it (e.g., prefix with `void` or leave as a bare expression) so `createClient` and the `mock<typeof fetch>()` cast are preserved for type-checking but no unused local exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zenko/src/treaty-types.ts`:
- Around line 85-88: The thrown error in the unwrap() path for result.kind ===
"error" currently interpolates the raw API payload via
unwrapErrorDetail(result.error); change the thrown message to only include
descriptive metadata (kind/status/specStatus) and remove the raw payload from
the message, and if you need to preserve the payload attach it to the Error
object (e.g., error.payload = result.error or error.details = safeSanitizedCopy)
instead of including it in the message; update the code paths referencing this
behavior in treaty-types.ts and the unwrap function to rely on the new
non-sensitive message while preserving the original payload on the Error
instance if needed.
---
Nitpick comments:
In `@packages/examples/src/__tests__/petstore-treaty-fetch.test.ts`:
- Line 1: The test suite is patching global.fetch unnecessarily; remove the
global.fetch override and the related afterEach reset so each test uses the
per-test injected fetch via createClient(origin, { fetch }); specifically,
delete the global.fetch assignment and the afterEach cleanup in
packages/examples/src/__tests__/petstore-treaty-fetch.test.ts (and the similar
blocks at the other occurrences noted), and ensure tests construct clients with
createClient(origin, { fetch }) as intended so no global shared state remains.
- Around line 206-210: The test creates a compile-time-only sentinel `_client`
which triggers noUnusedLocals; replace the binding with an unassigned expression
so types still check but no dead local remains: remove `const _client =` and
instead call the `createClient("http://petstore.swagger.io/v1", { fetch:
mock<typeof fetch>() as unknown as typeof fetch })` expression without assigning
it (e.g., prefix with `void` or leave as a bare expression) so `createClient`
and the `mock<typeof fetch>()` cast are preserved for type-checking but no
unused local exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7095f595-dbc4-4f30-82ce-654b320851d6
⛔ Files ignored due to path filters (1)
packages/zenko/src/__tests__/__snapshots__/treaty-generator.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
docs/treaty-client.mdpackages/examples/generate.jspackages/examples/src/__tests__/petstore-treaty-fetch.test.tspackages/zenko/src/__tests__/treaty-generator.test.tspackages/zenko/src/__tests__/treaty-runtime.test.tspackages/zenko/src/treaty-types.ts
✅ Files skipped from review due to trivial changes (1)
- docs/treaty-client.md
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/zenko/src/treaty-types.ts`:
- Around line 82-105: The stray closing brace prematurely ends the
unwrap<T>(result: TreatyResult<T>) function causing the subsequent subtype
branches to be outside the function; remove that extra `}` so the function body
continues to handle the "unexpectedError" branches (checks on result.subtype
"transport", "parse", and default) and then close the function only once after
all branches are handled. Locate the unwrap function and its early returns for
result.kind ("success" and "error") and ensure the transport/parse/other error
throws are inside the same unwrap function before the single final closing
brace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69b16080-7099-454d-9c1a-9fd779b8d5ba
📒 Files selected for processing (1)
packages/zenko/src/treaty-types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Treaty client redesign plan: operationId-first clients with a single request object (
params,query,body,headers,init) and discriminated results (kind:success|http|transport|parse|unknown).What changed
errorStatusKeysmaps OpenAPI response keys toerrorsfield names so the runtime can pick the right Zod schema for HTTP error bodies. Emitted inoperationMetadataalongside existing status maps.TreatyResultis now a discriminated union; addedTreatyResultFor,TreatyOperationsClient,TreatyRequest,TreatyRouteTreeClient. Re-exportedunwrapandorThrow(alias) fromzenko/treaty.errorsalign; categorizes fetch failures, parse failures, and HTTP errors withspecStatus.operations, fulloperationMetadataJSON,treatyRoutes, andcreateClientthat returns the operation client plus$routesfor legacy Eden-style chaining (documented as lighter / not metadata-aware).kindnarrowing.Verification
bun checkbun zenko testpackages/examples:bun test(treaty tests run against generated*.gen.tsfrombun run codegenlocally; generated files are gitignored)Follow-ups (non-blocking)
statusliterals fromsuccessResponseswithout incorrect casts (now uses real HTTP status).TreatyResultForHTTP branches or align petstoreerrorResponseswith per-status OpenAPI entries for finer 404 vs default typing.$routescould gain metadata-aware parsing in a later iteration.Summary by CodeRabbit
New Features
Documentation
Tests
Chores