Skip to content

fix(python-sdk): log silent sidecar cleanup failures#1279

Open
realfishsam wants to merge 1 commit into
mainfrom
fix/empty-catch-logging-1256-1260
Open

fix(python-sdk): log silent sidecar cleanup failures#1279
realfishsam wants to merge 1 commit into
mainfrom
fix/empty-catch-logging-1256-1260

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Summary

  • Replace silent pass handlers in the Python SDK sidecar manager with debug logging.
  • Add debug logging when API error body parsing falls back to the original exception string.

Fixes #1256
Fixes #1257
Fixes #1258
Fixes #1259
Fixes #1260

Test Plan

  • python3 -m py_compile sdks/python/pmxt/server_manager.py sdks/python/pmxt/client.py
  • git diff --check

Notes

  • This is a focused Python SDK observability fix; no generated client or API surface changes are intended.

@realfishsam

Copy link
Copy Markdown
Contributor Author

Focused validation passed locally:

  • python3 -m py_compile sdks/python/pmxt/server_manager.py sdks/python/pmxt/client.py
  • git diff --check

The current red generated-sync checks are unrelated repo-wide drift: regenerators want broad hosted-client/API reference/COMPLIANCE changes (for example removing hosted-mode dispatch sections from generated clients and rewriting Hunch/API reference docs). I am intentionally keeping this PR scoped to the Python SDK empty-catch diagnostics fix rather than folding that unrelated generated churn into it.

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: PASS (NOT VERIFIED)

What This Does

Adds debug logging to several previously silent Python SDK exception paths around sidecar version comparison, orphan cleanup, old-server termination, and API error-body parsing. This should improve diagnostics without changing normal SDK responses.

Blast Radius

Python SDK only: sdks/python/pmxt/client.py and sdks/python/pmxt/server_manager.py. No core sidecar routes, exchange normalizers, OpenAPI schema, or TypeScript SDK behavior changes.

Consumer Verification

Before (base branch):
The changed exception handlers swallowed failures with bare pass, e.g. malformed API error bodies in _extract_api_error() and sidecar cleanup/version-check failures in ServerManager produced no log trail.

After (PR branch):
Static/AST verification shows the touched handlers now call logger.debug(..., exc_info=exc), and the edited Python files compile:

python3 -m py_compile sdks/python/pmxt/client.py sdks/python/pmxt/server_manager.py
# PASS

Full runtime import/consumer verification was blocked because this checkout does not include/install the generated pmxt_internal Python package:

ModuleNotFoundError: No module named 'pmxt_internal'

Test Results

  • Build: PASS (npm run build --workspace=pmxt-core)
  • Unit tests: NOT VERIFIED for Python runtime behavior (generated pmxt_internal unavailable in this review checkout); syntax compile passed for touched Python files.
  • Server starts: N/A (Python logging-only SDK change)
  • E2E smoke: NOT VERIFIED (same generated Python package blocker)

Findings

No blocking findings.

Non-blocking note: there are still unrelated silent pass handlers in sidecar cleanup-adjacent code paths, e.g. sdks/python/pmxt/server_manager.py:331 (SIGKILL escalation failure) and sdks/python/pmxt/server_manager.py:401 (stale lock unlink failure). If the intent is to eliminate all silent sidecar cleanup failures, those should get debug logging too, but the handlers explicitly touched by this PR are improved.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: OK — no new Any/schema surface added beyond existing Python exception handling.
  • Auth safety: OK — logs include exception metadata only; I did not see credentials added to log messages.

Semver Impact

patch -- diagnostic/logging-only Python SDK fix.

Risk

I could not run an installed Python SDK consumer path because generated pmxt_internal is missing from this checkout, so this is a syntax/static pass rather than a runtime-verified pass.

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