fix: GET /v1/user_info 500 on missing broker_credentials table (issue #9)
trex-core reported GET /v1/user_info returning 500 for valid service/daemon tokens (whoami → daemon_id), degrading the daemon and holding new trades. The reported hypothesis (service-token None deref) was a misdiagnosis. Root cause: app/api/v1_user_info.py: credentials = repo.list_broker_credentials(...) psycopg.errors.UndefinedTable: relation "broker_credentials" does not exist The broker_credentials table was missing and the handler only caught AttributeError, so the DB error propagated to a bare 500. Two fixes: 1. Handler now degrades gracefully when broker-credential lookup fails — returns empty brokers and still serves daemon_id/user_id/permission, which do not depend on broker credentials. This is what trex-core's whoami needs. 2. ensure_schema() now also CREATE TABLE broker_credentials (same shape as migration 20260502_0014). ensure_schema-bootstrapped DBs were missing it, since previously only the alembic path created it. Added tests/test_v1.py regression: broker-credential lookup raising a DB error must yield HTTP 200 with daemon_id, not 500. Verified against the real DB (was 500 → now 200 + daemon_id). Full suite 377 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -59,6 +59,13 @@ def get_user_info(
|
||||
except AttributeError:
|
||||
# repo does not implement list_broker_credentials yet (mock or older impl)
|
||||
pass
|
||||
except Exception as exc: # pragma: no cover - defensive
|
||||
# Broker credentials are supplementary. A failure here (e.g. missing
|
||||
# broker_credentials table, DB error) must not crash /v1/user_info —
|
||||
# the critical fields (daemon_id, user_id, permission) do not depend
|
||||
# on broker credentials. Degrade gracefully with an empty broker list.
|
||||
logger.warning("broker credential lookup failed; returning empty brokers: %s", exc)
|
||||
brokers = []
|
||||
|
||||
finally:
|
||||
if callable(getattr(type(repo), "close", None)):
|
||||
|
||||
@@ -406,6 +406,19 @@ class PostgresPortfolioRepository:
|
||||
ON daemons (owner_user_id)
|
||||
"""
|
||||
)
|
||||
cur.execute(
|
||||
"""
|
||||
CREATE TABLE IF NOT EXISTS broker_credentials (
|
||||
user_id TEXT NOT NULL,
|
||||
broker TEXT NOT NULL,
|
||||
account_ref TEXT NOT NULL,
|
||||
encrypted_payload TEXT NOT NULL,
|
||||
credential_version INTEGER NOT NULL DEFAULT 1,
|
||||
last_rotated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
|
||||
PRIMARY KEY (user_id, broker)
|
||||
)
|
||||
"""
|
||||
)
|
||||
cur.execute("SELECT EXISTS (SELECT 1 FROM pg_extension WHERE extname = 'timescaledb')")
|
||||
if cur.fetchone()[0]:
|
||||
cur.execute(
|
||||
|
||||
@@ -804,6 +804,37 @@ def test_v1_user_info_returns_user(monkeypatch) -> None:
|
||||
assert body["disabled_reason"] is None
|
||||
|
||||
|
||||
def test_v1_user_info_degrades_when_broker_credentials_lookup_fails(monkeypatch) -> None:
|
||||
"""issue #9 regression: broker_credentials lookup failure (missing table / DB error)
|
||||
must NOT crash /v1/user_info with 500. daemon_id/user_id/permission do not depend on
|
||||
broker credentials — degrade gracefully with empty brokers."""
|
||||
monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true")
|
||||
|
||||
principal = TokenPrincipal(
|
||||
token_id="t1", user_id="user-1", permission="read",
|
||||
token_type="service", runtime_id="rt-1", daemon_id="daemon-9",
|
||||
)
|
||||
user = User(id="user-1", username="alice", permission="read", is_active=True)
|
||||
|
||||
mock_repo = MagicMock()
|
||||
mock_repo.get_token_principal.return_value = principal
|
||||
mock_repo.get_user.return_value = user
|
||||
mock_repo.list_broker_credentials.side_effect = RuntimeError(
|
||||
'relation "broker_credentials" does not exist'
|
||||
)
|
||||
|
||||
with patch("app.core.auth.get_portfolio_repository_from_settings", return_value=mock_repo), \
|
||||
patch("app.api.v1_user_info.get_portfolio_repository_from_settings", return_value=mock_repo):
|
||||
client = _make_app()
|
||||
resp = client.get("/v1/user_info", headers=_service_token_header())
|
||||
|
||||
assert resp.status_code == 200
|
||||
body = resp.json()
|
||||
assert body["daemon_id"] == "daemon-9"
|
||||
assert body["user_id"] == "user-1"
|
||||
assert body["brokers"] == []
|
||||
|
||||
|
||||
def test_v1_user_info_inactive_user_has_disabled_reason(monkeypatch) -> None:
|
||||
monkeypatch.setenv("PORTFOLIOHUB_DB_ENABLED", "true")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user