My `claude -p` Wrapper Catches Timeouts. A Non-Zero Exit Isn't a Timeout, So It Just Crashes.

Two days ago I published a post about an ERROR: convention I'd built into this repo's AI-calling code: every failure path from _claude() — the helper both my MCP server and my commit-message script use to shell out to claude -p — was supposed to return a string starting with ERROR:, so callers could do result.startswith("ERROR:") instead of trusting the output as a real commit message. That post fixed the one place the convention wasn't actually followed: the timeout branch returned a bare string with no prefix.

I closed that post treating the convention as done. It wasn't. There's a third failure path in the exact same function, and it doesn't return a string at all — it raises.

the code as it stood

server.py's _claude(), after Monday's fix:

def _claude(prompt: str, system: str = None) -> str:
    full = (system + "\n\n" + prompt) if system else prompt
    try:
        raw = subprocess.check_output(["claude", "-p", full], text=True, timeout=20).strip()
    except subprocess.TimeoutExpired:
        return "ERROR: claude -p timed out after 20s"
    return "\n".join(
        l for l in raw.splitlines()
        if not _STRIP_RE.search(l)
    ).strip()

One except clause. subprocess.check_output doesn't just time out, though — it also raises subprocess.CalledProcessError if the child process exits non-zero, and FileNotFoundError if the binary isn't on PATH at all. Neither of those is a TimeoutExpired. Neither is caught. Both propagate straight out of _claude(), out of the generate_commit_message MCP tool, and up to whatever's holding the tool call.

I'd been treating "the convention has two failure paths, make sure they agree" as the whole problem. It has three, and the third one isn't a disagreement about formatting — it's the total absence of any string at all.

proving it, not just reading it

I didn't want to take my own code's word for what subprocess.check_output does under a real claude -p failure, so I built one. A one-line stand-in binary on PATH ahead of the real claude:

#!/bin/sh
echo "simulated failure: rate limited" >&2
exit 1

Then I called the actual staged-diff path through git_commit.py (same subprocess call, same except subprocess.TimeoutExpired gap) with a real staged file:

$ python3 git_commit.py
simulated failure: rate limited
Traceback (most recent call last):
  File "git_commit.py", line 36, in <module>
    raw = subprocess.check_output(
          ...
subprocess.CalledProcessError: Command '['claude', '-p', ...]' returned non-zero exit status 1.

Full traceback, exit code 1, no clean message anywhere in it. I did the same thing to server.py's _claude() directly (stubbing the mcp import so I could load the module without the real package installed) and got the identical uncaught exception. Then I emptied PATH of anything named claude entirely and hit FileNotFoundError the same way — same gap, different trigger.

The commit-hook version of this is quietly saved by an accident, not a design decision: hooks/prepare-commit-msg runs git_commit.py with 2>/dev/null and only writes to the commit-message file if the captured stdout is non-empty:

MSG="$(python "$SCRIPT" 2>/dev/null)" && [ -n "$MSG" ] && printf '%s\n' "$MSG" > "$1"

The traceback goes to stderr, which gets thrown away; stdout is empty; the hook falls back to git's own default template. That's the right outcome, but it's the right outcome for the wrong reason — it works because the hook happens to discard stderr, not because git_commit.py handles the failure. Run the script directly, outside the hook, and you get a raw Python traceback where a one-line "claude failed, here's why" message should be. The MCP tool has no equivalent stderr-swallowing wrapper at all — an uncaught exception there is just an uncaught exception.

the fix

Same shape as the timeout branch, extended to cover the other two ways subprocess.check_output fails:

def _claude(prompt: str, system: str = None) -> str:
    full = (system + "\n\n" + prompt) if system else prompt
    try:
        raw = subprocess.check_output(
            ["claude", "-p", full], text=True, timeout=20, stderr=subprocess.PIPE
        ).strip()
    except subprocess.TimeoutExpired:
        return "ERROR: claude -p timed out after 20s"
    except subprocess.CalledProcessError as e:
        return f"ERROR: claude -p exited {e.returncode}: {(e.stderr or '').strip()[:200]}"
    except FileNotFoundError:
        return "ERROR: claude CLI not found on PATH"
    return "\n".join(
        l for l in raw.splitlines()
        if not _STRIP_RE.search(l)
    ).strip()

I added stderr=subprocess.PIPE so the CalledProcessError branch has something useful to report instead of just a return code — without it, e.stderr is None and the message loses exactly the "rate limited" detail that would tell a caller whether to retry or give up. git_commit.py got the same two except clauses, printing to stderr and exiting 1 instead of returning a string, matching its existing timeout branch.

Reran both repro cases against the fixed code:

$ python3 git_commit.py            # fake claude exits 1
claude -p exited 1: simulated failure: rate limited

$ python3 -c "import server; print(server._claude('hello'))"   # claude missing from PATH
ERROR: claude CLI not found on PATH

No tracebacks. Clean, ERROR:-prefixed strings a caller can actually check, on both new paths, matching the one that already worked.

the actual lesson

"Fixed the convention" and "audited every place the convention applies" are not the same claim, and I'd conflated them. The Monday post found two return statements that disagreed on formatting and made them agree — a genuinely different bug from a third code path that was never a return statement in the first place. Both live in the same six-line try/except, which is exactly why it was easy to see one and miss the other: except subprocess.TimeoutExpired reads like it's handling "the ways this subprocess call can fail," when it's actually handling one specific way out of at least three. Grepping for "does every except clause return a matching string" wouldn't have found this — the bug is in the exception types that have no except clause at all, and those don't show up by reading the ones you already wrote.