Skip to content

fix(oneshot): don't swallow stderr on failure; enable faulthandler - #29024

Closed
fabiosiqueira wants to merge 1 commit into
NousResearch:mainfrom
fabiosiqueira:fix/oneshot-stderr-diagnostics
Closed

fix(oneshot): don't swallow stderr on failure; enable faulthandler#29024
fabiosiqueira wants to merge 1 commit into
NousResearch:mainfrom
fabiosiqueira:fix/oneshot-stderr-diagnostics

Conversation

@fabiosiqueira

Copy link
Copy Markdown
Contributor

Summary

hermes_cli/oneshot.py:run_oneshot redirects both stdout and stderr to /dev/null for the entire _run_agent call tree. This keeps the success path quiet (the original goal) but discards every diagnostic when something goes wrong:

  • C-level crash inside a native extension → process dies with SIGSEGV, exit 139, zero output (no Python traceback, no faulthandler trace, no last-log-line).
  • Unhandled Python exception inside the agent → traceback printed to the redirected sys.stderr, never reaches the operator (the exception object propagates, but anything the agent wrote to stderr along the way is gone).
  • Empty-response return (provider returned empty content, retries exhausted with no usable text, etc.) → exit 0 with empty stdout, no clue what happened.

This was concretely felt while debugging #29021 — the only symptom of a pydantic-core thread segfault was hermes -z "ping" returning empty stdout. Adding a single print was the only way to learn the process had been SIGSEGV'd at all.

What this PR changes

run_oneshot keeps the silent-on-success behavior but stops discarding diagnostics on failure:

  1. faulthandler.enable(file=sys.stderr, all_threads=True) is wired before the redirect block. contextlib.redirect_stderr only swaps sys.stderr (Python-level); faulthandler holds the underlying fd, so a C-level SIGSEGV/SIGABRT stack still reaches the operator's terminal.

    Caveat: Rust PyO3 panics that call libc abort() cannot be caught by faulthandler — those still die mute. This catches everything else: numpy aborts, ctypes mis-calls, double-frees, openssl issues, etc.

  2. Redirected stderr → SpooledTemporaryFile instead of /dev/null. On any exception escaping _run_agent, the tail of that buffer is written to the real stderr before the exception propagates. On an empty-response return, the tail is also surfaced with an explicit hermes -z: empty response; stderr tail follows: header.

The happy path is unchanged: stdout = final response, stderr = empty.

Test plan

Validated end-to-end in the Hermes venv (Linux x86_64 musl, Python 3.11.15):

  • Happy path: hermes -z "responda apenas 'pong'" --model openrouter/owl-alpha --provider openrouter → stdout=pong, stderr=0 bytes, exit 0. Identical to current behavior.
  • Python exception path: monkey-patched _run_agent to write to sys.stderr and then raise. With this PR, the stderr write reaches the operator (instead of being swallowed) and the exception propagates as before.
  • SIGSEGV path (pydantic-core 2.41.5 + Responses API + thread): still dies hard with no output — Rust abort() is uncatchable by faulthandler. Acknowledged in commit message; the dep bump in fix(deps): bump pydantic to 2.13.4 to avoid pydantic-core thread segfault #29021 is what fixes that particular crash, not this PR.

Platforms tested

  • Linux 6.8.0 x86_64, musllinux (Alpine container), Python 3.11.15

Related

Companion to #29021 (pydantic bump). #29021 fixes the specific crash; this PR fixes the diagnosability gap that hid the crash and turned a 30-second triage into a multi-hour investigation.

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels May 20, 2026
`run_oneshot` was redirecting both stdout and stderr to /dev/null for
the entire call tree to keep the success path silent. The cost is that
any failure inside the agent — a SIGSEGV in a native extension, an
unhandled Python exception, or a silent empty-response — left the
operator with zero signal.

This change keeps the silent-on-success behavior but stops discarding
the diagnostic on failure:

1. `faulthandler.enable(file=sys.stderr, all_threads=True)` is wired
   before the redirect block. `contextlib.redirect_stderr` only swaps
   `sys.stderr` (Python-level); faulthandler holds the underlying fd,
   so a C-level crash trace still reaches the operator's terminal.
   (Note: Rust PyO3 panics that call libc `abort()` cannot be caught
   by faulthandler — those still die mute. This catches everything
   else: numpy aborts, ctypes mis-calls, double-frees, etc.)

2. Redirected stderr now goes to a `SpooledTemporaryFile` instead of
   /dev/null. On any exception escaping `_run_agent`, the tail of that
   buffer is written to the real stderr before the exception
   propagates. On an empty-response return (the agent ran but produced
   nothing — e.g. provider returned empty content, rate limited and
   exhausted retries, etc.) the tail is also surfaced with an explicit
   header so the operator can diagnose.

The happy path is unchanged: stdout = final response, stderr = empty.
@fabiosiqueira
fabiosiqueira force-pushed the fix/oneshot-stderr-diagnostics branch from 3c8d33f to c73296f Compare May 21, 2026 12:32
fabiosiqueira added a commit to fabiosiqueira/hermes-engine that referenced this pull request Jun 6, 2026
…30623 supersedes

PR NousResearch#29024 is superseded by upstream NousResearch#30623, which solves the silent-failure
core its own way. Returning hermes_cli/oneshot.py to the origin/main version
so local/all-fixes carries no orphan diff for it (the branch should only hold
commits with an open upstream PR routing them back to origin/main).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fabiosiqueira

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #30623, which lands a fix for the same silent-failure problem (agent exceptions/empty responses no longer exit 0 quietly). The remaining delta here — faulthandler for native crashes and surfacing the captured stderr tail rather than just str(exc) — is marginal now that the core is covered upstream, and #30623's author already settled on their approach for this path, so I'd rather not carry a competing PR. Dropping it from our fork too. Thanks.

— 🤖 Claude Opus 4.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants