Azure / Azure/azure-sdk-for-rust
Cosmos FFI Follow-Up: Cancellation & diagnostics
- Dominant language
- Rust
- Stars
- 884
- Forks
- 365
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 112
Description
### Summary
Establish the completion diagnostics surface: either make cancellation propagate partial diagnostics or remove handle-based cancellation, and expose success-path diagnostics via a `cosmos_diagnostics_t`.
### Motivation
@FabianMeiswinkel (PR #4515 review, marked **blocking**, later approved as a follow-up):
> *"I don't think we should offer any cancellation mechanism preventing getting a (partial) diagnostics… Any cancellation / e2e timeout we allow should propagate partial diagnostics. rather not provide cancellation in first step than one without access to partial diagnostics."*
The completion's `diagnostics` field is currently reserved and always NULL, so even successful operations expose no diagnostics.
### Current state (verified)
- `src/submit.rs` races the driver future against `cancel_notify` in a `tokio::select!` and **drops** the future on cancel, discarding any partial `DiagnosticsContext`.
- The driver op entry points (`execute_singleton_operation`, `execute_plan`, `plan_operation`) take **no cancellation token**; diagnostics only escape via `DiagnosticsContextBuilder::complete()` on the Ok/Err return path.
- `CosmosCompletion.diagnostics: *mut c_void` (`src/completion.rs`) is *"Reserved for a future diagnostics handle; always NULL for now."*
- The handle-based cancellation surface: `cosmos_operation_handle_cancel`, the `Cancelled` outcome/state, `was_cancel_requested`, `OperationInner`'s `cancel_requested`/`cancel_notify`, `CosmosErrorCodeOperationCancelled = 4012`.
### Proposed approach
Pick a cancellation direction, then design the diagnostics surface once:
- **Option A (wrapper-only, smaller):** remove handle-based cancellation now (Fabian's endorsed fallback). Drop `cosmos_operation_handle_cancel`, the `Cancelled` outcome/state, `was_cancel_requested`, and the `tokio::select!` cancel arm (replace with `let out = work.await;`). Keep the end-to-end timeout (it resolves via `Err`, so it already carries diagnostics). Keep the `4012` slot reserved for ABI stability.
- **Option B (cross-crate, larger):** add a cancellation token to the driver op entry points + a contract that returns the partial `DiagnosticsContext` on cancel; the wrapper passes the token instead of dropping the future. File the driver change as its own `azure_data_cosmos_driver` issue.
Then, independent of A/B, expose a `cosmos_diagnostics_t` (regions contacted, retry timeline, per-attempt latency, RU) on completions — populated on success (and on cancel/timeout if Option B).
### Scope
- **In:** cancellation surface decision + implementation; `cosmos_diagnostics_t` shape, population, ownership/free contract; header regeneration; tests; spec/docs.
- **Out:** the driver-side token/diagnostics contract if Option B is chosen — split into a separate `azure_data_cosmos_driver` issue.
### Work breakdown
- [ ] Decide Option A vs Option B (record in `DATA_MOVEMENT_MODEL.md`).
- [ ] If A: remove the cancellation surface (see FFI-08 removal list); keep e2e timeout + reserved `4012`.
- [ ] If B: file + implement the driver cancellation-with-diagnostics contract; wire the wrapper to it.
- [ ] Design `cosmos_diagnostics_t` + its free contract; populate on success.
- [ ] Regenerate header; update C tests, `cancellation.c`, and docs.
### Acceptance criteria
- No cancellation API that silently discards diagnostics — either cancellation is gone or it always delivers partial diagnostics.
- Diagnostics reachable from a completion with a documented ownership/free contract.
- Full validation sweep + `cspell` clean; header regenerated.
### References
FFI-08, FFI-13 in `FFI_FOLLOWUPS.md` (FFI-08 lists the full Option-A removal surface); `src/submit.rs`, `src/completion.rs`.
Contributor guide
Assessment
This issue has not been assessed yet.