feat(c++/node): add close_outputs, NodeFailed and Reload event types#1410
feat(c++/node): add close_outputs, NodeFailed and Reload event types#1410PavelGuzenfeld wants to merge 3 commits into
Conversation
486b8cb to
f39fa15
Compare
…ain) Add try_next_event(), next_event_timeout(), events_is_empty(), drain_events(), drained_events_len(), drained_events_next() to the C++ node API. Introduce DoraEventType::Timeout for non-blocking and timed receive operations. Refactor DoraEvent from tuple struct to named fields to carry a timed_out flag that distinguishes timeout from stream closure.
Expose close_outputs() to selectively close node outputs from C++. Add NodeFailed and Reload variants to DoraEventType with event_as_node_failed() accessor for failure details.
f39fa15 to
6f28e29
Compare
|
Hi @PavelGuzenfeld — closing this in favor of #1847, which is the rescue of this PR. Second in the 4-PR rescue stack for your full series (#1409 → #1410 → #1413 → #1414); see #1846 for the first. Same pattern as the #1846 rescue: I extracted just the named feature (NodeFailed/Reload event types + close_outputs + event_as_node_failed), kept all your work intact, adapted the You're credited via Thanks for the work. |
Rescue of #1410 (PavelGuzenfeld). Second in the 4-PR rescue stack for the C++ API parity work; stacked on top of #1846 (event receive variants). New FFI surface =============== * Two new `DoraEventType` variants: - `NodeFailed` -- an upstream node has failed. Inputs that depended on the failed node will stop receiving data; the payload (via `event_as_node_failed`) names the failed node, the error message, and the affected input ids. - `Reload` -- hot-reload notification for an operator. C++ nodes can now react (e.g. flush caches) instead of treating it as `Unknown`. * `event_as_node_failed(event) -> DoraNodeFailed` -- extracts `{ affected_input_ids, error, source_node_id }` from a NodeFailed event. * `close_outputs(sender, output_ids)` -- selectively close one or more of this node's outputs without shutting the whole node down. Downstream subscribers see the corresponding `InputClosed` event. Implementation notes ==================== * `event_as_node_failed` destructures via `EventOrReason::Event(...)` rather than the original PR's `event.event` (which assumed the named-field DoraEvent shape from #1409 that we never adopted). * `close_outputs` returns `DoraResult` like the existing `send_output` family for ergonomic consistency on the C++ side. Closes #1410. Co-Authored-By: Pavel Guzenfeld <PavelGuzenfeld@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…(rescue of #1410) (#1847) * feat(c++/node): add close_outputs, NodeFailed and Reload event types Rescue of #1410 (PavelGuzenfeld). Second in the 4-PR rescue stack for the C++ API parity work; stacked on top of #1846 (event receive variants). New FFI surface =============== * Two new `DoraEventType` variants: - `NodeFailed` -- an upstream node has failed. Inputs that depended on the failed node will stop receiving data; the payload (via `event_as_node_failed`) names the failed node, the error message, and the affected input ids. - `Reload` -- hot-reload notification for an operator. C++ nodes can now react (e.g. flush caches) instead of treating it as `Unknown`. * `event_as_node_failed(event) -> DoraNodeFailed` -- extracts `{ affected_input_ids, error, source_node_id }` from a NodeFailed event. * `close_outputs(sender, output_ids)` -- selectively close one or more of this node's outputs without shutting the whole node down. Downstream subscribers see the corresponding `InputClosed` event. Implementation notes ==================== * `event_as_node_failed` destructures via `EventOrReason::Event(...)` rather than the original PR's `event.event` (which assumed the named-field DoraEvent shape from #1409 that we never adopted). * `close_outputs` returns `DoraResult` like the existing `send_output` family for ergonomic consistency on the C++ side. Closes #1410. Co-Authored-By: Pavel Guzenfeld <PavelGuzenfeld@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(c++/node): parse close_outputs ids via FromStr to avoid FFI panic Review finding on #1847 head 5b39f72: `close_outputs` converted caller-supplied `Vec<String>` to `Vec<DataId>` via `Into::into`, which calls `From<String> for DataId` -- documented as panicking on invalid characters at `libraries/message/src/id.rs:186` with an explicit `# Panics` notice. A typo in a C++ string literal would have aborted the whole node instead of returning `DoraResult.error` like the rest of the C++ API surface does. Same panic-across-cxx::bridge category as the `Instant::now() + Duration::from_millis(u64::MAX)` overflow on #1846 -- a public C++ API accepting unconstrained input must be panic-safe for the entire input domain. Fix: parse each id via `id.parse::<DataId>()` (`impl FromStr`, which is the safe path documented at `libraries/message/src/id.rs:167`) and on the first parse error return a `DoraResult` whose `error` names the offending id and the underlying validation error. The well-formed-id case is unchanged; only the previously- panicking error path now surfaces cleanly. Pattern bank update for me (second in this PR series): "public API accepts type T via FFI" + "T has a `From` impl that panics on invalid input" = "use `FromStr` / `TryFrom`, never the panicking `From`". The standard library's `String::from` is total, so it doesn't share this gotcha, but dora's id types deliberately panic in `From` to keep the happy path concise -- which is the right call for trusted Rust call sites and the wrong call for FFI boundaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(api-cxx): document close_outputs, NodeFailed/Reload, event_as_node_failed Review residual: docs/api-cxx.md was stale for the new C++ node API surface this PR adds (close_outputs, NodeFailed, Reload, DoraNodeFailed, event_as_node_failed). Also catches up on the #1846 additions that merged earlier in the stack (Empty, Timeout, try_next_event, next_event_timeout, events_is_empty, drain_events, DrainedEvents) since the DoraEventType enum and the downcast-helper section can't be accurate without them. Additions ========= * New "Non-blocking and timed receive" subsection under Events documenting try_next_event, next_event_timeout, events_is_empty, and the drain_events / DrainedEvents family. * DoraEvent downcast helpers gain event_as_node_failed. * DoraEventType enum now lists all 10 variants currently on main (was 6) with one-line descriptions, plus a paragraph explaining the Empty vs Timeout distinction. * New DoraNodeFailed struct section after DoraInput, with a short usage example. * New OutputSender::close_outputs subsection, noting that ids are validated and return DoraResult.error on invalid input (matching the FromStr parse fix landed earlier on this PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Pavel Guzenfeld <PavelGuzenfeld@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
close_outputs(sender, output_ids)to selectively close node outputs from C++DoraEventType::NodeFailedandDoraEventType::Reloadenum variantsevent_as_node_failed(event)accessor returningDoraNodeFailedstruct with affected input IDs, error message, and source node IDUsage (C++)