Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
(shape-checked so an arbitrary value is never echoed into the form), and
`/favicon.ico` is an open route (308 → `/static/favicon.svg`) so the probe
never bounces through the auth redirect at all.
- **Clean Ctrl-C shutdown.** `start --serve`'s Ctrl-C/SIGTERM handling now
completes the supervisor teardown instead of the process dying mid-shutdown
(uvicorn 0.49's own signal capture re-raised the captured signal after
`serve()` returned, killing the process before the daemon's `finally` ran);
the dashboard's `/api/events` SSE stream also no longer spams an ERROR-level
cancellation traceback on every shutdown while a client holds it open.

### Deprecated

Expand Down
48 changes: 48 additions & 0 deletions doc/dev/03-decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,54 @@ rejected approaches as tombstones.

---

### 2026-07-05 Clean Ctrl-C shutdown: the daemon owns its signals; SSE cancellation ends the stream [accepted]
- **Choice**: two independent fixes, both in the shutdown path uvicorn/starlette
own by default. (1) `_run_daemon`'s `--serve` branch overrides the built
`uvicorn.Server` instance's `capture_signals` with a no-op context manager
(there is no public `install_signal_handlers` config flag on uvicorn 0.49) and
installs its own `SIGINT`/`SIGTERM` handlers on the running loop around
`await server.serve()`: first signal sets `server.should_exit = True`
(graceful — uvicorn's own main loop polls this regardless of who sets it),
second sets `server.force_exit = True`; the handlers are removed once
`serve()` returns. (2) both `/api/events` SSE generators in
`interfaces/api/app.py` catch `asyncio.CancelledError` around their streaming
loop and end the generator cleanly instead of letting it propagate (the merged
generator also explicitly cancels any outstanding per-iteration getter task on
the way out); additionally, a `logging.Filter` on the `uvicorn.error` logger
drops uvicorn's "Exception in ASGI application" record specifically when the
underlying exception is a plain `asyncio.CancelledError` — necessary because
the noisy traceback is *not* raised by our own generator code: it originates
in Starlette's `StreamingResponse.__call__` (an `anyio` task group racing
"stream the body" against "listen for disconnect", the code path uvicorn 0.49
is stuck on since it declares ASGI `spec_version: "2.3"`, below the `2.4`
Starlette checks for its newer, single-await implementation), so no amount of
fixing our own generator's cancellation handling prevents uvicorn from logging
it.
- **Why**: uvicorn 0.49's `Server.serve()` unconditionally wraps itself in
`capture_signals()`, which restores the pre-`serve()` signal disposition and
**re-raises** the captured signal with that (default) disposition right after
`serve()` returns — killing the process *before* `_run_daemon`'s own `finally`
(scheduler shutdown + `supervisor.shutdown()`) could run, so a paper store's
pending writes were never drained and units weren't stopped cleanly on Ctrl-C.
Separately, operators reading a multi-screen `CancelledError`/`anyio.WouldBlock`
traceback on every shutdown while a dashboard tab was open mistook a routine,
timeout-bounded force-cancel (`_SHUTDOWN_GRACE_SECONDS = 3`, kept as-is; its
own "Cancel N running task(s)..." log line stays — it *is* useful signal) for
a crash.
- **Rejected alternatives**: passing `install_signal_handlers=False` to
`uvicorn.Config(...)` (the shape assumed going in) — that flag does not exist
in uvicorn 0.49's actual `Config`/`Server` API (confirmed against the
installed version; it raises `TypeError` at construction), so the
`capture_signals` override is the version-correct equivalent. Restructuring
`_install_hardening`'s two `@app.middleware("http")` handlers into raw ASGI
middleware (removing `BaseHTTPMiddleware`'s task/stream indirection entirely,
which independently reproduces the same traceback even with *no* custom
middleware registered, per direct testing against a bare FastAPI app) — would
also work and is more "correct" in spirit, but is a much larger, riskier
change for a shutdown-logging concern; the targeted logging filter achieves
the same operator-facing outcome (no ERROR-level traceback spam) without
touching request/response plumbing every endpoint relies on.

### 2026-07-04 Display-only rounding, exact value on hover (PR #160) [accepted]
- **Choice**: a single dependency-free `static/format.js` (the `tbFmt` namespace)
owns every display transform — grouped/rounded money by quote currency
Expand Down
113 changes: 113 additions & 0 deletions trading_bot/interfaces/api/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,82 @@ def _finite_or_none(value: float | None) -> float | None:
}


class _SuppressGracefulShutdownCancellation(logging.Filter):
"""Drop uvicorn's "Exception in ASGI application" record for an expected
graceful-shutdown cancellation — not a real application bug.

Why this lives at the logging layer, not in a route
-----------------------------------------------------
Every ``/api/events`` route below returns a plain
:class:`~starlette.responses.StreamingResponse`. uvicorn 0.49 declares ASGI
``spec_version: "2.3"`` for HTTP (see
``uvicorn/protocols/http/httptools_impl.py``), which is *below* the
``(2, 4)`` threshold Starlette's own ``StreamingResponse.__call__`` checks
before picking its newer, single-await implementation — so under this
uvicorn version Starlette *always* falls back to its older code path: an
``anyio`` task group racing "stream the body" against "listen for
disconnect" (``starlette/responses.py``). When
``timeout_graceful_shutdown`` (:data:`_SHUTDOWN_GRACE_SECONDS`) elapses on a
still-open SSE connection, uvicorn force-cancels the per-connection task —
logging "Cancel N running task(s), timeout graceful shutdown exceeded" (a
legitimate, expected line, left alone). That cancellation lands *inside
Starlette's own task group*, which re-raises it unconverted all the way
back up to uvicorn's ``run_asgi``, which logs **any** exception escaping the
ASGI app — even a deliberate cancellation it just issued itself — as an
ERROR-level, multi-frame traceback ("Exception in ASGI application").
Operators read that as a crash on every shutdown while a client holds
``/api/events`` open, even though the SSE generators in this module (see
their own ``except asyncio.CancelledError`` handling) already end cleanly
the moment cancellation reaches *their* frame — the noisy traceback
originates in Starlette's own plumbing, which this module does not control,
so suppressing it has to happen at the logging layer instead. This filter
drops *only* that exact, expected shape — an :class:`asyncio.CancelledError`
reaching uvicorn's "Exception in ASGI application" log call — so a genuine
application exception (any other exception type, or a ``CancelledError``
logged under a different message) is still logged normally.

Matching on exception *type* alone (not also uvicorn's own cancel message,
e.g. "Task cancelled, timeout graceful shutdown exceeded") is deliberate:
with **two** stacked ``@app.middleware("http")`` handlers here (see
:func:`_install_hardening`), each ``BaseHTTPMiddleware`` layer's own nested
``anyio`` task group can re-signal the cancellation through its *own*
cancel scope on the way back out, which — per ``anyio``'s cancel-scope
bookkeeping — can substitute a **fresh, message-less**
``CancelledError()`` for the one uvicorn originally raised. The message is
the only thing lost; every step in the chain is still a plain
cancellation. Restricting the match to uvicorn's own "Exception in ASGI
application" message keeps this from ever catching an unrelated
``uvicorn.error`` record.
"""

_EXPECTED_MESSAGE_PREFIX = "Exception in ASGI application"

def filter(self, record: logging.LogRecord) -> bool:
"""Return ``False`` (drop) only for the expected shutdown cancellation."""
if not record.exc_info:
return True
exc = record.exc_info[1]
is_expected = isinstance(
exc, asyncio.CancelledError
) and record.getMessage().startswith(self._EXPECTED_MESSAGE_PREFIX)
return not is_expected


def _suppress_graceful_shutdown_cancellation_logs() -> None:
"""Install :class:`_SuppressGracefulShutdownCancellation` on ``uvicorn.error`` once.

Idempotent (checked by filter *type*), so calling this from every
:func:`create_app` / :func:`create_dashboard_app` build — including in
tests, which build many apps per process — never stacks up duplicate
filter instances.
"""
target = logging.getLogger("uvicorn.error")
if not any(
isinstance(f, _SuppressGracefulShutdownCancellation) for f in target.filters
):
target.addFilter(_SuppressGracefulShutdownCancellation())


def _install_hardening(app: FastAPI) -> None:
"""Add the shared body-size cap + security-header middleware to ``app``.

Expand All @@ -456,11 +532,17 @@ def _install_hardening(app: FastAPI) -> None:
:data:`_SECURITY_HEADERS` (nosniff / anti-clickjacking / no-referrer), and
every authed JSON body (``/api/*``) additionally gets ``Cache-Control:
no-store`` so the live book is never cached by a browser or intermediary.
* **Quiet shutdown.** Installs
:class:`_SuppressGracefulShutdownCancellation` on the ``uvicorn.error``
logger so a force-cancelled ``/api/events`` connection past
:data:`_SHUTDOWN_GRACE_SECONDS` doesn't spam an ERROR-level traceback for
what is an expected, controlled shutdown (see that class's docstring).

Middleware runs outermost-first in registration order; the body cap is added
last here so it runs **first** (it can reject before the header middleware even
builds a response).
"""
_suppress_graceful_shutdown_cancellation_logs()

@app.middleware("http")
async def _security_headers(request: Request, call_next: Any) -> Any:
Expand Down Expand Up @@ -658,6 +740,17 @@ async def _generator() -> Any:
yield f"data: {json.dumps(frame, default=_default)}\n\n"
except asyncio.TimeoutError:
yield ": heartbeat\n\n"
except asyncio.CancelledError:
# Server shutdown (the graceful-shutdown timeout force-cancels this
# task while it is suspended in the wait above) or the ASGI server
# tearing the connection down: either way the stream is over, which
# is the *correct* time for this generator to end — not an error.
# Left uncaught, this propagates as a bare CancelledError that
# uvicorn/starlette log as a scary ERROR-level traceback on every
# shutdown while a client holds this endpoint open; ending the
# generator cleanly here is the intended response to "server is
# going away", not a swallowed real failure.
pass
finally:
bus.remove_queue(queue)

Expand Down Expand Up @@ -1925,6 +2018,10 @@ async def events(request: Request) -> StreamingResponse:

async def _generator() -> Any:
seen: set[str] = set()
# Tracks the current iteration's per-queue getter tasks so a
# cancellation between iterations (see the CancelledError handler
# below) can still cancel whichever ones are outstanding.
getters: list[asyncio.Task[Any]] = []
try:
yield ": connected\n\n"
if not queues:
Expand All @@ -1950,6 +2047,7 @@ async def _generator() -> Any:
)
for task in pending:
task.cancel()
getters = []
if not done:
yield ": heartbeat\n\n"
continue
Expand All @@ -1966,6 +2064,21 @@ async def _generator() -> Any:
"ts": _epoch_ms(),
}
yield f"data: {json.dumps(frame, default=_default)}\n\n"
except asyncio.CancelledError:
# Server shutdown (the graceful-shutdown timeout force-cancels this
# task while a client holds the merged stream open) or the ASGI
# server tearing the connection down: the stream is over, which is
# the correct time for this generator to end, not an error. Any
# getter task still outstanding at the point of cancellation (e.g.
# cancelled mid-`asyncio.wait`, before the per-iteration cleanup
# above ran) is cancelled here too, so no bare queue.get() task is
# left dangling. Left uncaught, this propagates as a bare
# CancelledError that uvicorn/starlette log as a scary ERROR-level
# traceback on every shutdown while a client holds this endpoint
# open.
for task in getters:
if not task.done():
task.cancel()
finally:
for bus, queue in zip(buses, queues, strict=True):
bus.remove_queue(queue)
Expand Down
Loading
Loading