From 3b853e88e70c810c63f90375318a41fd3465b743 Mon Sep 17 00:00:00 2001
From: Bill Hlavacek <bill.hlavacek@gmail.com>
Date: Sun, 10 May 2026 16:02:25 -0600
Subject: [PATCH] Capture stdout/stderr in run_command's timeout path so
 BNG2.pl errors reach callers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`run_command(suppress=True, timeout=set)` was redirecting stdout/stderr
to `DEVNULL`, so when `BNGCLI` raised `BNGRunError` on a failing BNG2.pl
invocation, the actual diagnostic message (e.g. `Missing end parentheses
... at and(...)`) never reached the user.

Always capture instead. With a timeout set, `subprocess.run` buffers all
output anyway, so the `suppress` distinction was already a no-op for the
user during the run — what it did do was discard the bytes needed to
build a useful error message after the fact.

After this, the `_run.log` for a failing BNG2.pl call shows the real
ABORT message, which makes round-trip / regenerated-BNGL bugs trivial
to localize.
---
 bionetgen/core/utils/utils.py | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/bionetgen/core/utils/utils.py b/bionetgen/core/utils/utils.py
index 7d19fd23..29b33f21 100644
--- a/bionetgen/core/utils/utils.py
+++ b/bionetgen/core/utils/utils.py
@@ -655,20 +655,18 @@ def run_command(command, suppress=True, timeout=None, cwd=None):
     be killed.
     """
     if timeout is not None:
-        if suppress:
-            # I am unsure how to do both timeout and the live polling of stdo
-            rc = subprocess.run(
-                command,
-                timeout=timeout,
-                stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
-                cwd=cwd,
-            )
-            return rc.returncode, rc
-        else:
-            # I am unsure how to do both timeout and the live polling of stdo
-            rc = subprocess.run(command, timeout=timeout, capture_output=True, cwd=cwd)
-            return rc.returncode, rc
+        # Always capture stdout/stderr — this lets callers (notably BNGCLI)
+        # surface BNG2.pl's error tail in BNGRunError when the command
+        # fails. With timeout set, subprocess.run buffers all output
+        # anyway, so suppress=True vs False makes no behavioral difference
+        # for the user during the run.
+        rc = subprocess.run(
+            command,
+            timeout=timeout,
+            capture_output=True,
+            cwd=cwd,
+        )
+        return rc.returncode, rc
     else:
         if suppress:
             process = subprocess.Popen(