Skip to content

feat(c++/node): add close_outputs, NodeFailed and Reload event types#1410

Closed
PavelGuzenfeld wants to merge 3 commits into
dora-rs:mainfrom
PavelGuzenfeld:feature/cpp-close-outputs-event-types
Closed

feat(c++/node): add close_outputs, NodeFailed and Reload event types#1410
PavelGuzenfeld wants to merge 3 commits into
dora-rs:mainfrom
PavelGuzenfeld:feature/cpp-close-outputs-event-types

Conversation

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor

Summary

  • Add close_outputs(sender, output_ids) to selectively close node outputs from C++
  • Add DoraEventType::NodeFailed and DoraEventType::Reload enum variants
  • Add event_as_node_failed(event) accessor returning DoraNodeFailed struct with affected input IDs, error message, and source node ID

Stacked on #1409 (Phase 2) — merge that first, then this PR's diff is just the 1 new commit.

Usage (C++)

// Close specific outputs
rust::Vec<rust::String> ids;
ids.push_back("output_a");
auto result = close_outputs(dora_node.send_output, ids);

// Handle node failure events
if (ty == DoraEventType::NodeFailed) {
    auto failed = event_as_node_failed(std::move(event));
    std::cerr << "Node " << failed.source_node_id << " failed: " << failed.error;
}

…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.
@heyong4725
Copy link
Copy Markdown
Collaborator

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 event.event destructuring to the new tuple-struct DoraEvent(EventOrReason) shape that we landed in #1846 (the original PR used the named-field shape that we ended up not adopting — EventOrReason carries the no-event-available reason instead).

You're credited via Co-Authored-By on the commit. #1847 is currently stacked on #1846; once #1846 merges I'll rebase #1847 onto main.

Thanks for the work.

@heyong4725 heyong4725 closed this May 17, 2026
heyong4725 added a commit that referenced this pull request May 17, 2026
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>
heyong4725 added a commit that referenced this pull request May 17, 2026
…(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>
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.

2 participants