feat: write safety, error classes, and the Features resource - #10
Merged
Conversation
Both fell through to the generic ApiError, so "this already exists" and "this payload is well formed but not allowed" arrived indistinguishable from an unrecognised status. The catalog push already depends on telling them apart -- it rescues ApiError and re-raises unless the status is 409 or 422 -- which is the shape a caller is forced into when the class does not carry the meaning. ConflictError is the one a caller often wants to treat as success. UnprocessableEntityError is the one that never becomes success by retrying.
Verified against the sandbox: POST /v1/subscriptions/{id}/update with the same
body and the same `Idempotency-Key` returns the same operation id both times,
while the same body with no key creates two operations. `X-Idempotency-Key` is
ignored. So the header is what makes a retried write safe, and nothing else
does -- the endpoint is not idempotent on its own.
The retry therefore engages only when a caller supplies a key, on 429 and 5xx,
bounded at two attempts. A conflict or a validation failure is the API's answer
and is raised at once. Faraday's own retry is left on the idempotent verbs where
it already was.
Every mutating method takes an optional idempotency_key. PriceBooks#add_products
passed its body as bare keywords, which the new keyword would have swallowed; it
now passes a hash. The transport moved to a Requests module -- the same methods,
extracted so the class stays inside its length budget with the retry added.
The example for #update_operation omitted payment_schedule, which the endpoint
requires -- so the documented body returns 400. Nothing caught it because the
spec stubs the request, and the stub matched the same wrong body the comment
described.
Recorded from the sandbox: payment_schedule is immediately / next_invoice /
custom, calculation_method is pro_rata / pay_in_full / do_not_charge, and the
operation types are update_count, update_prices, add_coupon and remove_coupon.
The response is the created operation, { "id" => "supd_..." }.
The catalog push reached all of this through `client.products.send(:request, …)`,
which works and should not be what a writer depends on. Every route and envelope
below was verified against the sandbox rather than inferred:
- Features are keyed by `code`, not an opaque id, so get/update/delete take a
code. /v1/features answers with the standard meta/data envelope, so list and
Collection work unchanged.
- Products#features answers with a **bare array** of feature_code/value, no
envelope -- nothing to paginate and no Collection to build.
- Archiving is a PUT for both products and features; POST answers 404 "Route not
found". For a feature it is also a prerequisite for deletion: DELETE on an
active one answers 400 "Cannot delete a feature that is not archived".
- link_feature carries `{ value: }` -- true for a boolean feature, the cap for a
numeric one -- and 400 means the value_type cannot hold it.
Verified end to end against the sandbox: create, get, update, link to a product,
unlink, archive, delete, with the product's linked-feature count restored to
exactly what it was.
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
1 participant
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.
Write safety for the endpoints a seat writer is about to depend on, plus the Features
routes the catalog push currently reaches through
send(:request, …).Every route, envelope and status below was verified against the sandbox, not inferred.
That mattered: one of the shapes this gem already documented returns 400.
The idempotency question, settled
POST /v1/subscriptions/{id}/update, same body, twice:Idempotency-KeyX-Idempotency-KeySo the standard header is what makes a retried write safe, and nothing else is: the endpoint
is not idempotent on its own.
update_countsets the count outright rather than incrementing,so the count survives a double apply — but two operation records do not.
The retry therefore engages only when a caller supplies a key, on 429 and 5xx, bounded at
two attempts. A 409 or a 422 is the API's answer and is raised at once. Faraday's own retry is
left exactly where it was, on the idempotent verbs.
Changes
ConflictError, 422 →UnprocessableEntityError. Both fell through to the genericApiError, so "this already exists" and "well formed but not allowed" were indistinguishablefrom an unrecognised status. The catalog push already depends on telling them apart by
rescuing
ApiErrorand re-raising on status.idempotency_key:on every mutating method, with the bounded retry described above. Thetransport moved to a
Requestsmodule — same methods, extracted soBaseResourcestaysinside its length budget with the retry added.
Featuresresource. Keyed bycode, not an opaque id, so get/update/delete take a code./v1/featuresuses the standard meta/data envelope, solistandCollectionwork unchanged.Products#archive,#features,#link_feature,#unlink_feature— the four routes thecatalog push reaches through
client.products.send(:request, …)today.Contracts discovered along the way
DELETEon an active one answers400
Cannot delete a feature that is not archived, so removal is archive-then-delete. HenceFeatures#archive.PUTfor both products and features;POSTanswers 404 "Route not found".GET /v1/products/{id}/featuresanswers with a bare array offeature_code/value— noenvelope, nothing to paginate, no
Collectionto build.update_operation's documented payload was wrong. It omittedpayment_schedule, whichthe endpoint requires, so the body in the comment and in the spec returns 400. Nothing caught
it because the spec stubs the request and the stub matched the same wrong body. The verified
shape, with the enum values, is now recorded on the method.
Verification
ApiError(so a 409 would be retried), never sending the header, dropping 409/422 from themapper,
archiveas POST on products and on features, andlink_featuresending no value.link it to a real product, unlink it, archive it, delete it — with the product's linked-feature
count restored to exactly the 39 it started with, and the probe feature returning 404 at the
end. Nothing left behind.
PriceBooks#add_productspassed its body as bare keywords, whichthe new keyword would have swallowed. Caught by the existing suite, now passes a hash.
Not here
No version bump and no CHANGELOG entry, following the convention of the previous feature PR
(#9, Customers) where both were left to a dedicated release commit.
The backend cannot use any of this until this merges and the monorepo's Gemfile pin moves —
it currently reads
github: 'factorialco/hyperline-cli', branch: 'main'.