Skip to content

Capture node output and report unexpected node exits - #53

Merged
mavam merged 4 commits into
mainfrom
topic/node-fixture-diagnostics
Jul 15, 2026
Merged

Capture node output and report unexpected node exits#53
mavam merged 4 commits into
mainfrom
topic/node-fixture-diagnostics

Conversation

@lava

@lava lava commented Jul 14, 2026

Copy link
Copy Markdown
Member

The node fixture spawned tenzir-node with piped stdout/stderr that nothing ever drained: node output beyond the kernel pipe buffer stalled the node's logging, the captured output was lost when the process went away, and a node that crashed mid-suite went entirely unnoticed — dependent tests just failed with opaque connection errors.

A CI investigation of a 10/10-attempt failure of a node-backed suite test (job) ended inconclusive precisely because of this: the node's stderr, which would have shown whether the node crashed or wedged, was discarded.

With this change, the node fixture:

  • pumps the node's stdout and stderr into node-stdout.log / node-stderr.log next to the node's state directory, so the node can never block on a full pipe and its output survives for diagnosis,
  • exposes the log paths to dependent tests as TENZIR_NODE_STDOUT_LOG / TENZIR_NODE_STDERR_LOG, so tests can include node context in their own failure output, and
  • logs an error with the exit code and a stderr tail when the node exits while the fixture is still active, making a mid-suite node crash immediately visible in the harness output.

The pumps own and close their streams: closing a piped stream from another thread while a reader is blocked on it deadlocks on the stream's reader lock.

📚 Docs PR: tenzir/content#116

Benno Evers added 2 commits July 14, 2026 19:35
The node fixture spawned tenzir-node with piped stdout/stderr that
nothing ever drained: node output beyond the kernel pipe buffer stalled
the node's logging, the captured output was lost when the process went
away, and a node that crashed mid-suite went entirely unnoticed --
dependent tests just failed with opaque connection errors.

A CI investigation of a 10/10-attempt failure of a node-backed suite
test ended inconclusive precisely because of this: the node's stderr,
which would have shown whether the node crashed or wedged, was
discarded.

The fixture now pumps both streams into node-stdout.log and
node-stderr.log next to the node's state directory, exposes the paths
as TENZIR_NODE_STDOUT_LOG and TENZIR_NODE_STDERR_LOG to dependent
tests, and logs an error with the exit code and a stderr tail when the
node exits while the fixture is still active.
@lava
lava requested a review from mavam July 14, 2026 17:58

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2183327cd5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/tenzir_test/fixtures/node.py Outdated
Drain stderr during startup and coordinate watcher teardown.

An already-finished node can no longer be mistaken for an intentional shutdown.

Keep node logs when users pass --keep, and cover the lifecycle edge cases.

Assisted-by: GPT-5 (Codex)
@mavam
mavam force-pushed the topic/node-fixture-diagnostics branch from d943a9b to c321578 Compare July 15, 2026 07:37
@mavam
mavam merged commit d59fe42 into main Jul 15, 2026
5 checks passed
@mavam
mavam deleted the topic/node-fixture-diagnostics branch July 15, 2026 09:17
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