Server logs an expected offline-runner resource 503 as ERROR + full traceback (follow-up to #1115)

[andrewreid]

Server logs an expected offline-runner resource 503 as ERROR + full traceback (follow-up to #1115)

Summary

When a session's runner is offline, every resource-proxy GET (environment,
filesystem, changes, terminals) correctly returns 503 RUNNER_UNAVAILABLE —
but the server also logs each one as ERROR [omnigent.server.app] Internal error: … with a full traceback. A runner being offline is a normal
operational state (host reboot, idle-reap, tunnel drop), not an internal error.
The result is a stack-trace per offline resource hit, burying genuine ERRORs.

This is a distinct, still-unaddressed facet of the (closed/completed) #1115.
That fix was client-side only — the useSessionRunnerOnline polling gate —
which cut the steady-state storm. Two things it did not touch remain:

1. Server log severity — never changed.
2. Open-time burst — the online-gate is tri-state and only blocks on
   === false; while undefined (before the first /health resolves) the
   resource fetches fire anyway, so each session open still emits a burst of
   503s before the client learns the runner is offline.

Mechanism (server)

• GET /sessions/{id}/resources/environments/{env} (get_session_environment)
  and /filesystem/{path} (list_environment_root) → _proxy_get_to_runner
  → _get_runner_client_for_resource_access → client_for_session_resources,
  which raises OmnigentError(RUNNER_UNAVAILABLE) when the pinned runner is not
  in the registry (omnigent/runner/routing.py:139).
• RUNNER_UNAVAILABLE (and RUNNER_CAPABILITY_MISMATCH) map to 503
  (omnigent/errors.py:78-79).
• The OmnigentError handler logs by status band:
  if exc.http_status >= 500: _logger.error("Internal error: …", exc_info=True)
  (omnigent/server/app.py:1555). 503 ≥ 500 ⇒ full traceback at ERROR. The
  line is unchanged since repo genesis, so Runner offline → resource-proxy GET returns raw 503 in a storm; UI keeps polling offline-runner sessions #1115's fix never reached it.
• _proxy_get_to_runner also carries a dead if runner_client is None: raise HTTPException(502) branch (omnigent/server/routes/sessions.py:17678)
  — the helper raises instead of ever returning None, so that graceful path
  is never taken.

Evidence

A single open of one offline-runner session produced ~27 filesystem + ~16
environment resource GETs, all 503, each preceded by
ERROR [omnigent.server.app] Internal error: runner … is offline … and a full
stack trace. DB-local endpoints (/items, /child_sessions) returned 200 in
the same window — confirming only the runner-proxied routes are affected.

Suggested fix

• In _handle_omnigent_error (omnigent/server/app.py:1543), treat the
  transient 503 codes (RUNNER_UNAVAILABLE, RUNNER_CAPABILITY_MISMATCH) as
  expected: log at INFO/WARN without exc_info. Keep ERROR + traceback for
  the genuine 500-class (INTERNAL_ERROR, HARNESS_PROTOCOL_VIOLATION). Status
  stays 503.
• Remove the dead runner_client is None branch in the proxy helpers, or make
  the helper return None on offline so that branch is live — pick one.
• Optional client follow-up: once a session's runner is known offline, also gate
  the environment-availability probe, and dampen the pre-/health open burst
  so an offline session doesn't fan out resource GETs before the gate resolves.

Impact

Low functional (responses are already correct 503s), but real operational noise:
genuine ERRORs are drowned by per-hit stack traces on any offline-runner session.

Found via server-log + codebase correlation.

[Aarya2004]

Hi! I'd like to address this issue if possible! I was able to reproduce it so I'd be happy to go ahead and make the fix!

I think the fix is to treat the transient runner codes as expected before the status-band check, keeping ERROR + traceback for the genuine 500-class codes and leaving the HTTP response untouched. Something like:

if exc.code in _TRANSIENT_RUNNER_ERROR_CODES:  # runner_unavailable, runner_capability_mismatch
    _logger.warning("Runner unavailable: %s", exc.message)
elif exc.http_status >= 500:
    _logger.error("Internal error: %s", exc.message, exc_info=True)

I'll also add a regression test that drives a resource GET against a session pinned to an offline runner through the real app factory and asserts the 503 is still returned but nothing is logged at ERROR / with a traceback.

One note on the suggested cleanup of the "dead" runner_client is None → 502 branch in _proxy_get_to_runner: as far as I can tell that branch isn't actually dead. _get_runner_client_for_resource_access only raises when a runner router is configured, when there's no router it falls back to get_runner_client(), which can legitimately return None (in-process/unit-test setups), and then the 502 branch is the live guard. So I'm planning to leave that branch as is, though let me know if I’m wrong!

For scope, I'm planning to keep this to the server-side logging fix but I can add the client-side follow-up (gating the environment-availability probe and damping the pre-/health open burst) in a separate PR or I can add it to the end of this PR, whichever works.

I'm new to this project so please let me know if you have any feedback, thanks!