Skip to content

Return 4xx JSON errors for malformed HTTP API requests #32

Description

@Justinabox

Summary

The aiohttp server write endpoints let malformed JSON and validation exceptions bubble out of handlers, so authenticated bad requests return 500 Internal Server Error with server tracebacks instead of stable 4xx JSON responses.

This affects the external control surface for SMS/USSD automation: bad clients, probes, or malformed requests should not look like server faults or produce noisy tracebacks.

Evidence

Affected code:

  • server.py:98-104 (/sms/send) calls await request.json() directly, checks only missing to / body, then lets modem.sms.send(...) exceptions propagate.
  • server.py:111-117 (/sms/subscribe) calls await request.json() directly.
  • server.py:125-137 (/ussd/send) calls await request.json() directly and only catches (TimeoutError, RuntimeError) from the USSD service.
  • callstack/protocol/commands.py:8-12 correctly raises ValueError for invalid phone numbers, but the HTTP route does not translate that into a client error.

Malformed JSON repro against authenticated routes, with a fake modem so no hardware is needed:

PYTHONPATH=. uv run --no-project --with aiohttp --with pyserial-asyncio python /tmp/callstack_repro_invalid_json_api.py

Observed output:

/sms/send: status=500 body='500 Internal Server Error\n\nServer got itself in trouble'
/sms/subscribe: status=500 body='500 Internal Server Error\n\nServer got itself in trouble'
/ussd/send: status=500 body='500 Internal Server Error\n\nServer got itself in trouble'

Invalid phone-number repro through the real SMSService validation path:

PYTHONPATH=. uv run --no-project --with aiohttp --with pyserial-asyncio python /tmp/callstack_repro_http_invalid_phone.py

Observed output:

ValueError: Invalid phone number: 'bad"number' (only digits, +, *, # allowed)
invalid phone /sms/send: 500 500 Internal Server Error\n\nServer got itself in trouble

Baseline health from this scout run:

git diff --check
# exit 0

PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q
# first run: 1 transient failure in tests/test_sms_service.py::test_receive_cmti
# immediate focused rerun: passed
# immediate full rerun: 314 passed in 4.22s

Duplicate checks run before filing:

gh issue list --state all --search 'malformed JSON HTTP API 500 request.json in:title,body'
gh issue list --state all --search 'JSONDecodeError sms send ussd subscribe in:title,body'
gh issue list --state all --search 'invalid JSON 400 HTTP in:title,body'
# no matching issues

Expected behavior

  • Malformed JSON bodies return a JSON 400 Bad Request (or equivalent documented 4xx) without a server traceback.
  • Valid JSON with invalid field types/values returns a documented 4xx, e.g. invalid to returns 400/422 with a safe message.
  • Operational modem failures still map to an appropriate non-2xx response, but do not expose sensitive details.

Actual behavior

The handlers raise JSONDecodeError or ValueError, aiohttp logs a traceback, and clients receive generic 500 responses.

Suggested fix direction

  • Add a small request helper such as await _json_body(request) that catches json.JSONDecodeError, aiohttp.ContentTypeError, and oversized/empty-body cases and returns a consistent JSON 400.
  • Validate route payload shape and string fields before invoking modem services.
  • Translate ValueError / SMSSendError / SMSReadError / USSD validation failures into documented JSON errors without leaking secrets, SIM identifiers, or message bodies.
  • Add route-level tests with pytest-aiohttp and fake modem services; no real modem required.

Acceptance criteria

  • /sms/send, /sms/subscribe, and /ussd/send return JSON 400 for malformed JSON.
  • /sms/send returns JSON 400/422 for invalid phone numbers instead of 500.
  • Route tests assert that fake modem methods are not called when request validation fails.
  • Server logs do not include SMS bodies, SIM identifiers, API keys, or full private webhook URLs while reporting validation failures.
  • Existing auth/rate-limit behavior from tests/test_api_auth.py remains intact.

Verification gates

git diff --check
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_api_auth.py -q
PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions