Admin Routstr provider balance check returns 500 on upstream ConnectTimeout

[shroominic]

Summary

GET /admin/api/upstream-providers/{provider_id}/balance can raise an unhandled httpx.ConnectTimeout when the upstream provider is another Routstr node and that node is slow or unreachable.

Instead of returning a controlled upstream error, the request falls through to the general exception handler and becomes a 500.

Affected versions

• Reproduced in v0.3.0
• Still present on branch v0.4.0 as of 2026-03-13

Reproduction

1. Configure an upstream provider with provider_type == "routstr"
2. Point base_url at a Routstr node that is slow, unreachable, or timing out on TCP connect
3. Call GET /admin/api/upstream-providers/{provider_id}/balance
4. Observe httpx.ConnectTimeout and a 500 response

Current behavior

The admin route special-cases Routstr providers and calls the upstream node directly:

• routstr/core/admin.py lines 1025-1046 on current v0.4.0
• raw httpx.AsyncClient().get(f"{clean_url}/v1/balance/info", ...)
• no try/except for httpx.TimeoutException / httpx.RequestError

This produces logs like:

• error_type: ConnectTimeout
• path: /admin/api/upstream-providers/4/balance

Expected behavior

The admin balance endpoint should not crash on upstream timeout.

It should either:

• return a controlled 502/504 with a useful error message, or
• return { "ok": false, "balance_data": null } consistently for unreachable upstreams

but it should not become an unhandled 500.

Additional context

There is already a RoutstrUpstreamProvider.get_balance() implementation in routstr/upstream/routstr.py that wraps the request and returns None on failure. The admin route currently bypasses that abstraction and duplicates the HTTP call.

There was a related v0.4.0 change in commit 4643aef ("no auth for routstr node to check and topup balance"), but that only makes the auth header optional. It does not address timeout handling.

There are also related timeout issues/fixes elsewhere in the repo:

• 504 Gateway Timeout error when using api.nonkycai.com node #283 general upstream timeout report
• Routstr node deducting max cost when returning 443 timeout bug #314 / make sure to refund #315 fixed payment reversion on timeout for proxied upstream requests

Those do not fix this admin balance path.

Possible fix direction

1. Remove the Routstr-specific raw HTTP branch from get_provider_balance
2. Instantiate the provider via _instantiate_provider(provider) for all provider types
3. Let RoutstrUpstreamProvider.get_balance() own the Routstr balance fetch behavior
4. Normalize failures into a controlled API response or explicit HTTPException
5. Add regression tests for timeout and non-200 upstream responses

Secondary observation

The error log for this path may show request_id: no-request-id even though general_exception_handler() passes request_id in extra. RequestIdFilter appears to overwrite record.request_id from the context var fallback in routstr/core/logging.py. That makes debugging harder, but it is separate from the 500 timeout bug.

[shroominic]

Proposed fix plan:

1. Patch the admin endpoint directly in routstr/core/admin.py

   • Keep the existing Routstr-specific branch for now.
   • Add an explicit timeout to the outbound httpx call.
   • Catch httpx.TimeoutException and map it to a controlled HTTPException (504 is the cleanest fit).
   • Catch other httpx.RequestError cases and map them to 502.
   • Log provider id + base URL + upstream URL in the handled error path.

2. Do not switch this endpoint to RoutstrUpstreamProvider.get_balance() in the same patch

   • That provider method is not behaviorally equivalent yet:
     • it still always sends Authorization
     • its balance unit handling does not match the admin route
   • Reusing it immediately would mix a bug fix with a behavior change.

3. Add regression coverage

   • Add an integration test for /admin/api/upstream-providers/{id}/balance where the outbound Routstr call raises httpx.ConnectTimeout.
   • Assert the endpoint returns a controlled upstream error instead of an unhandled 500.
   • Add a second test for a generic httpx.RequestError.
   • Keep the assertions tight enough to lock in the status code choice.

4. Optional follow-up cleanup in a separate PR

   • Normalize Routstr balance units (balance vs balance_msats) across routstr/core/admin.py, routstr/upstream/routstr.py, and routstr/upstream/auto_topup.py.
   • After that, collapse the duplicated admin Routstr balance logic into the provider abstraction.

5. Separate follow-up for logging

   • RequestIdFilter appears to overwrite record.request_id even when it is provided via extra, which is why these logs can show no-request-id.
   • Useful to fix, but it should stay out of the timeout patch.

Acceptance criteria:

• A timed-out Routstr upstream balance check no longer produces an unhandled 500.
• The admin endpoint returns a predictable upstream error response.
• Regression tests fail before the patch and pass after it.