Skip to content

Bring the Rust crate's idempotency, redirects, Retry-After and two form writes into line with the other SDKs - #201

Merged
robzolkos merged 5 commits into
mainfrom
feature/rust-parity-fixes
Sep 15, 2026
Merged

robzolkos merged 5 commits into
mainfrom
feature/rust-parity-fixes

Conversation

@robzolkos

@robzolkos robzolkos commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Four defects in the Rust crate, found while reviewing the Kotlin SDK on #198, which already behaves as described here. A Go PR is being opened in parallel for the two defects Go shares (idempotency and the reminders).

Idempotency ignored the behavior model

rust/generator decided whether a route may be resent from the spec's explicit x-hey-idempotent.natural alone, and short of one from the verb. behavior-model.json marks ten PATCH operations "idempotent": true — UpdateHabit, UpdateCalendarTodo, BulkUpdateClearances, UpdateClearance, UpdateCollection, UpdateContact, UpdateContactClearance, UpdateContactNote, UpdateMyClearance, UpdateSticky — so those went out once where their own policy (max: 2 on 429 and 503) says to resend. The generator now reads it as the Kotlin and TypeScript generators do: the explicit override first (UpdateMessage is a PUT with natural: false and stays non-idempotent), then the model's readonly || idempotent, and the verb only when the model says neither. Regenerating flips exactly those ten routes in routes.rs and nothing else. AGENTS.md now states the rule.

A 301 or 302 rewrote every non-GET to GET

redirected() turned anything but a GET or HEAD into a bodiless GET on a 301 or 302, so a PUT, PATCH or DELETE answered that way was followed as a read of the target and could report a mutation done that never reached it. RFC 9110 permits the rewrite on those statuses for a POST alone; a 303 applies retrieval to any method; a 307 or 308 keeps everything. The hop is now built by that rule.

Retry-After was honoured on a 429 alone

The retry loop read Retry-After on a 429 and a 503 but only waited as asked on the 429; a 503 saying how long the outage would last was resent after the backoff. RFC 9110 defines the header on a 503 for exactly that, and the TypeScript and Kotlin clients honour it on any status that earns a resend. The wait is now taken as asked on any retryable status; a header that asks for nothing or cannot be read still leaves the backoff to decide.

Reminders under the wrong list, and an emptied membership that sent nothing

update_event_fields filed reminders under all_day_reminder_durations[] only when the update itself said all_day: Some(true), else under timed_reminder_durations[]. HEY reads the list matching the event's all-day state after the write and unschedules the reminders when that list is absent, so an update leaving all_day as None on an all-day event cleared its reminders. When all_day is None the durations now go out under both keys, as Kotlin sends them; the one HEY does not read is ignored.

Extenzions::update looped over members, so Some(vec![]) sent nothing and could not be told from None. HEY replaces the membership whenever the field is present, and a form carries no empty array, so an explicitly empty list now goes out as one blank extenzion[members][], as an empty attendee list already does on a calendar event.

Tests

  • Generator fixture: a PATCH the model calls idempotent, a PATCH it does not, and a PUT marked natural: false, checked on the emitted Route.
  • Retry policy: UpdateSticky sent twice on a 503 then 200; UpdateMessage sent once on a 503; a 503 with Retry-After: 2 then 200 waits the two seconds and sends twice.
  • Client unit tests: PUT + 302 stays a PUT with its body and content type; DELETE + 301 stays a DELETE; POST + 302 becomes a GET without body or content headers; DELETE + 303 becomes a GET; PATCH + 307 and POST + 308 keep method and body. The canned transport now records the body it was handed.
  • Calendar events: all_day: None puts every duration under both keys; Some(true) under the all-day key only; Some(false) under the timed key only.
  • Extenzions: Some(vec![]) sends one blank member; the existing None case still sends nothing.

GOWORK=off make rs-check rs-check-drift is green locally: fmt, clippy and tests with all features, clippy and tests with --no-default-features, the examples, cargo deny over both workspaces, the package dry-run and the generator drift check.


Summary by cubic

Fixes four Rust SDK behaviors that diverge from the Kotlin and TypeScript SDKs: idempotency, redirects, Retry-After, and two form field writes.

Retry behavior

  • The generator now reads idempotency from the behavior model before the verb, so the ten PATCH operations the model marks idempotent can be resent under their own policies; UpdateMessage keeps its explicit non-idempotent override.
  • A 301 or 302 now rewrites only a POST into a bodiless GET, while PUT, PATCH, and DELETE keep method and body; 303 and 307/308 behave as before.
  • Retry-After is now honored on any retryable status instead of only 429, so a 503 gets the wait it asks for; the README says so too.

Form writes

  • Event updates that leave all_day unset now send reminders under both the all-day and timed keys, so editing an all-day event no longer clears its reminders.
  • An explicitly empty extenzion membership now sends one blank member instead of nothing, so it clears the roster rather than leaving it unchanged.

Written for commit 4ca7b30. Summary will update on new commits.

Review in cubic

…k to the verb

The Rust generator decided whether a route may be resent from the spec's
explicit x-hey-idempotent.natural alone, and short of one from the verb, so
the ten PATCH operations behavior-model.json marks idempotent — UpdateHabit,
UpdateCalendarTodo, BulkUpdateClearances, UpdateClearance, UpdateCollection,
UpdateContact, UpdateContactClearance, UpdateContactNote, UpdateMyClearance
and UpdateSticky — were sent once where their own policy (max: 2 on 429 and
503) says to resend. The generator now reads it as the Kotlin and TypeScript
generators do: the explicit override first, so UpdateMessage stays a PUT that
is not resent, then the model's readonly or idempotent, and the verb only
when the model says neither. Regenerating flips exactly those ten routes.

A generator fixture puts a PATCH the model calls idempotent, one it does not
and a PUT marked natural: false through the emitter; two client tests send
UpdateSticky twice on a 503 then a 200, and UpdateMessage once on a 503.
…as a POST

The client turned any request but a GET or HEAD into a bodiless GET on a
301 or 302, so a PUT, PATCH or DELETE answered that way was followed as a
read of the target and could report a mutation done that never reached it.
RFC 9110 permits that rewrite on those statuses for a POST alone; a 303
applies retrieval to any method; a 307 or 308 keeps everything. The hop is
now built by that rule, and the transmit doc says so.

Unit tests send each write through a redirect of each status on the canned
transport, which now keeps the body it was handed, and check the method,
body and content headers of the second hop.
…9 alone

The retry loop read Retry-After on a 429 and a 503 but honoured it only on
the 429, so a 503 that said how long the outage would last was resent after
the backoff instead. RFC 9110 defines the header on a 503 for exactly that,
and the TypeScript and Kotlin clients honour it on any status that earns a
resend. The wait is now taken as asked on any retryable status, with a header
that asks for nothing or cannot be read still leaving the backoff to decide,
and the header is reported to the hooks on the same terms.

A test answers a 503 with Retry-After: 2 then a 200 and checks the second
send waited the two seconds.
…t alone, and an emptied extenzion membership as one blank member

An event update filed its reminders under all_day_reminder_durations only
when the update itself said all_day: true, and under timed_reminder_durations
otherwise. HEY reads the list matching the event's all-day state after the
write and unschedules the reminders when that list is absent, so an update
that left all_day as None on an existing all-day event cleared its reminders.
When all_day is None the durations now go out under both keys, as the Kotlin
SDK sends them; the one HEY does not read is ignored.

An extenzion update looped over members, so Some(vec![]) sent nothing and
could not be told from None. HEY replaces the membership whenever the field
is present, and a form carries no empty array, so an explicitly empty list
now goes out as one blank extenzion[members][] value, as an empty attendee
list already does on a calendar event.

Tests check the three all_day cases of an update carrying reminders, and an
empty membership against an absent one.
Copilot AI balanced review requested due to automatic review settings September 15, 2026 22:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The public Rust README still documents Retry-After as applying only to 429 responses.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Aligns Rust SDK retries, redirects, idempotency, and form updates with other SDKs.

Changes:

  • Corrects redirect and Retry-After behavior.
  • Generates behavior-model-aware idempotency.
  • Fixes calendar reminders and empty Extenzion memberships, with tests.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File summaries
File Description
AGENTS.md Documents Rust idempotency generation.
rust/generator/src/model.rs Incorporates behavior-model idempotency.
rust/generator/src/fixtures.rs Tests idempotency precedence.
rust/hey-sdk/src/generated/routes.rs Regenerates idempotent route metadata.
rust/hey-sdk/src/client.rs Fixes redirects and retry waits.
rust/hey-sdk/src/services/calendar_events.rs Sends reminders under appropriate keys.
rust/hey-sdk/src/services/extenzions.rs Encodes empty memberships explicitly.
rust/hey-sdk/tests/retry_policy.rs Covers retry behavior.
rust/hey-sdk/tests/services_calendar_events.rs Covers reminder forms.
rust/hey-sdk/tests/services_extenzions.rs Covers empty memberships.
Review details
  • Files reviewed: 9/10 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rust/hey-sdk/src/client.rs
…the policy resends on

The retry paragraph still said a Retry-After was honoured on a 429 alone,
which the client no longer limits it to: a 503 naming how long the outage
will last is waited out the same way. The sentence now says so, and names
the 503 alongside the 429.
@robzolkos
robzolkos requested a balanced review from Copilot September 15, 2026 23:02
@robzolkos
robzolkos merged commit 16f93a1 into main Sep 15, 2026
32 checks passed
@robzolkos
robzolkos deleted the feature/rust-parity-fixes branch September 15, 2026 23:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

The behavior changes are consistent with their documented contracts and have focused regression coverage.

Review details
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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.

3 participants