Conversation
@propgate/sdk: a method for every route except signup, which is a mailbox
flow the CLI owns.
Every call returns `{ data, error, meta }` and none of them throw, so the
failure branch is type-checked instead of an `unknown` from a `catch`. `meta`
stays beside the data because that is where `nextCursor`, `created` and
`resolver` live. `listAll` walks a cursor to the end at the page size the
server clamps to.
Retries cover connection failures, timeouts, 429s and 5xx, and never repeat a
POST that may already have been applied — `POST /v1/api-keys` mints a key every
time it is called. A `Retry-After` past five seconds comes back as
`error.retryAfterSeconds` rather than stalling inside an await nobody can see.
Three things keep the coverage claim honest: sdk-coverage.spec.ts reads the
app's own router and fails when a route has no method reaching it,
sdk.e2e.spec.ts drives the client against createApp() over a socket with real
DNS and Postgres, and readme.spec.ts finds the methods by reflection so a
method added without a paragraph fails.
…en the caller does Three ways an exception could escape a client that promises never to throw: a body that stops arriving after the headers, a `timeoutMs` that `AbortSignal.timeout` refuses, and a backoff that grew unbounded with `maxRetries`. The first two are now errors like any other — `connection_error` and a new `invalid_option` naming the option and the value. The ceiling that already capped `Retry-After` now covers the exponential backoff too, so `maxRetries: 10` stops after six attempts rather than waiting two minutes inside one await. A backoff also no longer outlives the signal: aborting mid-wait returns `aborted` at once instead of sleeping to the end and spending one more fetch on a signal that is already aborted.
Owner
Author
|
Review round, for the morning read. Greptile raised two, both real, both fixed in 8b0d612 and their threads resolved:
Reviewing the same loop turned up two more it did not flag:
And 872589f adds Each fix ships a regression spec. 14/14 checks green. |
Merged
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
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.
@propgate/sdk— a typed client with a method for every route the API serves, except signup.What it covers
checks.run, the wholedomainslifecycle (create / list / listAll / get / update / check / timeline / remove),profiles,webhooks(includingrotateSecretand the delivery ledger),apiKeys,members,health. Twenty-one routes.Signup is deliberately absent: it is a mailbox flow — a six-digit code out, a key back — and by the time you are holding a server-side SDK you already have a key.
@propgate/clicovers it, andNOT_IN_SDKin the coverage spec is where that decision is written down.The DX decisions worth arguing with
{ data, error, meta }, the same envelope the API writes. Acatchbindsunknownand the compiler never mentions the case you forgot; returning the failure makes the branch type-checked.PropgateErroris still anError, sothrow result.errorworks for anyone who prefers that.metastays beside the data, because that is wherenextCursor,created,resolver,superseded,alreadyRevokedandpreviousSecretExpiresAtlive, and it is typed per call rather than as a bag.POSTthat may already have been applied, becausePOST /v1/api-keysmints a key every time it is called. A 429 is the one exception: the server refused before doing anything.Retry-Afterlonger than 5s is not waited out.POST /v1/domains/:id/checksanswersRetry-After: 47, and honouring that inside the call is a 47-second stall the caller never asked for — twice, at the defaultmaxRetries. It comes back aserror.retryAfterSecondsinstead. Worst case for one call is stated in the code: 65 seconds at the defaults, and only if every attempt times out.PROPGATE_API_KEY, rather than spending a round trip to be told 401.checks.runandhealthneed no key at all.fetchand nothing else.@propgate/dnsis a dependency for its types — the diagnosis taxonomy is a public contract and a hand-copied union is one that drifts.Tests
packages/sdk— 62 specs over request construction, the retry policy, envelope handling, pagination and the README.apps/api/src/sdk-coverage.spec.ts— readsapp.routesand fails when a route exists that no SDK method reaches, in both directions. Ungated, no containers. Verified it fails by deleting one call.apps/api/src/e2e/sdk.e2e.spec.ts— the client againstcreateApp()over a socket, with the real fixture DNS tier and Postgres. Eight specs covering the full lifecycle, the paging cursor, the delivery ledger, key rotation and error mapping.packages/sdk/src/readme.spec.ts— finds the methods by reflection, so a method added without a paragraph fails.pnpm lint,pnpm check,pnpm testand the gatedapps/apisuite (332 specs, all three tiers) all pass.Not in this PR
The docs site still shows
curland CLI snippets only. Adding an SDK tab touches ~20_snippets.tsfiles and the tab component, which is its own change rather than a rider on this one.Greptile Summary
The PR adds and publishes
@propgate/sdk, a typed, dependency-light Node client covering the API’s non-signup routes.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (2): Last reviewed commit: "ci(sdk): build the published package, so..." | Re-trigger Greptile