SSE Notification Service

/healthz reports ok even when Redis is down

GET /healthz exists so an orchestrator -- Kubernetes, a load balancer, an uptime monitor -- can tell whether this instance can actually do its job. Every feature this service offers goes through Redis, so if /healthz reports "ok" while Redis is completely unreachable, nothing routes traffic away from a fully broken instance -- every stream and publish call it receives will fail while its health check keeps insisting everything is fine. This is exactly the failure mode a health check exists to catch, inverted into false reassurance, and it stays invisible until the one day Redis actually goes down.

SSE events are missing their terminating blank line

The Server-Sent Events spec only dispatches an event to the client's EventSource.onmessage once a blank line terminates it. If that blank line is missing, a strict SSE parser keeps buffering fields, waiting for the terminator, and only dispatches once something else eventually supplies one -- so every notification arrives one event late, and if a user's session ends after their last notification, that final notification may never be dispatched to the browser at all. For a notifications feature, the last thing that happened being the one thing the user never finds out about is close to the worst possible failure mode.

Bearer token parsing drops the user id's first character

Every module downstream of get_current_user_id -- the stream generator, the publisher, the Redis key helpers -- treats whatever string this function returns as the identity for that connection. If a client authenticates with Authorization: Bearer alice and this function hands back a mangled id instead of "alice", that connection is durably, silently subscribed to a different user's notification stream than the one everyone else means when they say "alice." Two different ways of supplying the same identity -- the Authorization header vs. the ?user_id= query param the browser test page uses -- would authenticate the same human as two different users who never see each other's notifications, with no error and no log line pointing at why.

Add per-event-type notification metrics

GET /metrics exposes one unlabeled counter for every notification ever published, regardless of what kind it was. A single number can tell ops that traffic went up; it can't tell them which feature is generating that traffic -- which matters the moment more than one part of the product publishes through this same endpoint. app/metrics.py already defines a per-event-type counter for exactly this purpose, but nothing increments it, so capacity planning and anomaly detection have no signal to work with.

Duplicate delivery on an exact Last-Event-ID match

event_stream() subscribes to pub/sub before reading the replay backlog specifically to close a race: a notification published in the gap between a client reconnecting with Last-Event-ID and the backlog read actually returning shows up on both paths -- once via replay, once via the live subscription that was already listening. The check that's supposed to drop that duplicate is wrong at exactly the boundary that matters: the case where the live copy's id equals the last id already delivered, which is precisely what happens during the race the whole mechanism exists to handle. A user reconnecting during exactly the wrong half-second sees one notification twice, undermining the exactly-once guarantee this feature is built around.

Every connection emits a spurious first heartbeat

redis-py's PubSub.subscribe() deliberately leaves its own subscription-confirmation message sitting in the connection's read queue rather than consuming it. That confirmation is still sitting there the first time event_stream()'s main loop reads a message, and the loop currently can't distinguish it from genuinely nothing having happened, so it emits a heartbeat immediately instead of waiting out a real interval. This inflates the heartbeat counter by exactly one for every single connection regardless of how idle it actually was, quietly corrupting any alerting built on heartbeat rate, and it makes any monitor or test that expects the first thing it reads to be real content see a bogus heartbeat instead -- a source of flaky, hard-to-explain failures in exactly the tooling meant to catch real problems.

Add a broadcast endpoint for notifying multiple users

POST /notifications publishes to exactly one user_id per call. There is no endpoint for notifying a list of users or everyone at once -- anything that wants to announce something to more than one user, such as a maintenance-window notice or a shared-document update, has to loop over POST /notifications itself, once per recipient, from the caller's side. That's N round trips for something that's conceptually one event, and it pushes retry and partial-failure handling onto every caller instead of handling it once, here.

Reconnect replay reads the wrong Redis key

The whole point of this service is that a network blip must not silently drop notifications -- a guarantee that depends entirely on the backlog replay branch reading a user's stream of past notifications when a client reconnects with Last-Event-ID. It's currently reading from the pub/sub channel key instead, a key nothing ever writes stream entries to. Redis happily returns an empty result for a range query against a key that was never used as a stream, with no error at all -- so every reconnect-with-replay silently returns nothing, for every user, all the time. A user disconnected for even one second who missed a notification during that second never sees it, with no error, log line, or metric anywhere pointing at why.

Disconnect not checked during backlog replay

The main live loop in event_stream() checks whether the client has disconnected on every iteration, so a client that goes away while idle is noticed promptly. The backlog replay loop -- which can iterate over up to the configured backlog length (1000 by default) of entries for a user reconnecting after a long time away -- has no such check at all. If a client disconnects partway through a large replay, event_stream() keeps formatting and yielding every remaining backlog entry to a connection that's already gone, for however long that takes, before it ever gets a chance to notice -- wasted server work under exactly the load pattern (a long-absent user with a full backlog) that matters most.

Live delivery crashes on a publisher/subscriber protocol mismatch

publish_event() writes the same logical notification to two different places with two different shapes: a Redis Stream entry that the replay path reads, and a pub/sub JSON message that the live path reads. Those two shapes have drifted apart, so every notification delivered while a subscriber is live-connected raises an unhandled error inside that subscriber's stream generator, aborting their connection mid-flight. This breaks the primary, common-case delivery path for all live traffic, and it's dangerously easy to miss in manual testing because reconnection replay works perfectly fine -- someone testing only Last-Event-ID replay would see it work great, right up until an actual live user has any notification published to them and their stream dies with no client-visible explanation.

Pub/sub connection leaks when subscribe() fails

Every event_stream() call creates a pubsub object that holds a connection from the underlying Redis connection pool. If subscribing raises -- a transient Redis blip, a connection reset, anything -- while a client happens to be connecting at that exact moment, the pubsub object and the connection it holds are never closed, and each such failure permanently removes one connection from the pool. This is invisible under normal conditions, but under sustained connection churn with intermittent Redis instability -- exactly the conditions a real production Redis experiences during a failover -- it accumulates until the pool is exhausted and every new stream request, even healthy ones, starts failing or hanging, with a full restart the only way to recover.

Add a per-user cap on concurrent connections

GET /notifications/stream has no limit on how many concurrent connections a single user_id can hold open at once. app/config.py already defines max_connections_per_user (default 5) with a comment explaining its purpose, but nothing enforces it -- a client with a retry-loop bug that opens a new connection on every failed attempt without closing the old one, or a malicious script, can accumulate an arbitrarily large number of open connections for one user_id, each holding its own Redis pub/sub subscription and its own slot in whatever connection limits the deployment has.

Graceful shutdown doesn't signal open streams to close

When this instance is asked to shut down -- a rolling deploy, docker compose down, a Kubernetes pod eviction -- every currently-open notification stream connection just keeps running exactly as before, right up until the ASGI server forcibly tears the connection down out from under it, which either happens abruptly after a grace period or not at all within a reasonable window. Every rolling deploy leaves connected clients hanging instead of reconnecting promptly against whichever instance takes over next.