Production-safe Rack 3 callable-body streaming (5.0): server-aware disconnect reaping, finite TTL, concurrency cap, SSE helpers#2172
Conversation
Rewrite stream and keep_open onto the Rack 3 callable-body contract and make streaming production-safe by default. A keep_open stream whose client vanished could previously park forever, leaking a fiber, a file descriptor, and on Falcon a connection slot until the process was restarted. Disconnect reaping is server-aware. Puma exposes a real socket, so a zero-byte MSG_PEEK detects a gone peer. Falcon HTTP/1 has no peekable socket, so a small SSE-comment write-probe is used, gated to text/event-stream so a non-SSE body is never corrupted. HTTP/2 reaps natively and uses no probe. A finite default TTL (set :stream_keep_open_ttl, 300 seconds plus jitter, per-stream ttl: override, nil to opt out) bounds any leak, and a per-process cap (set :stream_max_concurrent, 1000) sheds past the limit with 503 and Retry-After so a flood cannot starve the worker pool. Adds out.flush, out.sse and out.sse_comment with Last-Event-ID support, reason-carrying teardown callbacks, and on_stream_error so a mid-stream producer exception reaches error reporters instead of vanishing. A hand-set Content-Length is stripped on streaming responses, and a bare Rack 3 callable returned from a route now streams instead of dropping to an empty 200. Breaking, hence 5.0: the EventMachine and Thin async path (async.callback, async.close, throw :async) is removed; Sinatra::ExtendedRack is now an inert, deprecated pass-through that is no longer installed by default; and Stream.new has a new signature. sinatra-contrib's Streaming addon is rewritten on the callable body. The streaming examples (chat, stream.ru, and a new sse.rb) use the new API and are covered by an examples test so they cannot silently drift. Maintenance tripwire tests guard the TTL default sync, the Rack::Deflater limitation (rack/rack#2470), and the ExtendedRack removal at 6.0. out.sse keeps hot-path allocations minimal (it skips the scrub copy, the line split, and per-field interpolation when the input is already clean). The Falcon HTTP/1 write-probe is a workaround for socketry/async-http#224; once that lands the probe can be dropped. Refs sinatra#1892 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for pushing this forward. The overall shape makes sense to me: bounded A couple of addenda/corrections on the disconnect-detection wording/shape:
Pseudo-shape: last_write_at = now
# in the public write path:
stream.write(chunk)
last_write_at = now
# in the parked keep_open loop:
if now - last_write_at >= poll
stream.write(heartbeat)
last_write_at = now
endThis is mostly a precision/behavioral tuning point, not an objection to the approach. The important distinction is that |
Falcon only gained working Rack 3 callable-body streaming in ~0.54; the older releases that Ruby < 3.2 can resolve (0.46 on 3.0, 0.51 on 3.1) hand the callable body an IO-less stream that is never flushed, so the streamed response arrives empty and the integration suite fails. The existing skip guard intended to cover this but compared with strings: "3.0.7" <= "3.0.0" is false, so the patch releases CI actually runs were never skipped. Switch to Gem::Version so any Ruby below 3.2 is skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rack's in-process test harness (Rack::MockResponse, reached via rack-test) only learned to drive a *callable* streaming body in Rack 3.2, where MockResponse#body calls body.call(io). On Rack 3.0/3.1 it calls body.each instead and Rack::Lint then raises "Enumerable Body must respond to each" because our Stream is call-only, breaking the CI cells pinned to those Racks. Real servers (Puma, Falcon) drive callable bodies from Rack 3.0 on, so this is purely a harness limitation, not a runtime one - the integration suite already covers real servers. Add skip_unless_rack_buffers_callable_body and guard the handful of tests that buffer a streamed response through the mock harness, so they skip rather than assert against an error the runtime never produces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…idle The Falcon HTTP/1 disconnect probe emitted its synthetic heartbeat every poll interval unconditionally. But a stream that is actively sending events already exercises the transport on every out.write/out.sse, and a dead peer surfaces there as a DISCONNECT_ERROR - so the extra probe was redundant bytes injected into a perfectly healthy, busy SSE stream. Track the monotonic time of the last successful application write and only emit the heartbeat once the stream has been write-idle for a full poll. An idle stream still gets one heartbeat per poll (unchanged); a stream writing more often than once per poll now gets none. The MSG_PEEK path is untouched: it puts zero bytes on the wire, so there is nothing to suppress. Per review feedback from samuel-williams-shopify on sinatra#2172. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two precision fixes from samuel-williams-shopify's review of sinatra#2172: - "Falcon has no socket to peek" overstated it. Falcon HTTP/1 does retain the underlying transport deeper in protocol-rack/async-http; what is true is that the stream passed to the Rack callable body exposes no raw socket. Reaching for the transport would couple Sinatra to Falcon internals rather than the Rack streaming contract, which is why we don't - so reword to "exposes no raw socket" and note why we leave the transport alone. - MSG_PEEK is a closure detector, not a liveness test: it only reaps a disconnect the kernel already knows about (a received FIN/RST) and cannot tell a silently-failed or sleeping peer from a healthy one. Document that those cases are covered by the parking TTL ceiling, not by the peek. Comment-only; no behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The code comment and README cited async-http#224 as future work, implying the Falcon HTTP/1 write-probe is a temporary workaround to be dropped once async-http reaps idle HTTP/1 streams natively. The investigation on that issue concluded otherwise: a server cannot detect a disconnect on an idle HTTP/1 bidirectional body without either missing a FIN that sits behind buffered bytes (non-consuming peek) or stealing request-body bytes the body owns (consuming read). HTTP/2 only escapes this because its reader routes framed DATA by stream id. So the app-level heartbeat is the deliberate, portable mechanism, not a stopgap. Reframe both the probe-site comment and the README server matrix to say so, and keep the issue link for the background and repros rather than as a removal trigger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Streaming/SSE needs the server to drive a Rack 3 callable response body, and that support is not uniform across everything Sinatra runs on: - Puma streams across the whole supported range. Verified empirically with a real Puma server on Ruby 2.7, 3.0, 3.1 and 4.0, and on Rack 3.0, 3.1 and 3.2. - Falcon needs Falcon >= 0.54 (hence Ruby >= 3.2): older Falcon never flushes the callable-body stream so the response arrives empty, and Ruby < 3.2 can only resolve those older Falcons. This is Falcon's own Ruby floor, not a Sinatra constraint, so the README now tells Ruby < 3.2 users to stream with Puma rather than leaving the empty-response failure to be discovered at runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…con version The note claimed "needs Falcon >= 0.54". Only 0.51 (empty stream) and 0.54 (works) were actually tested; 0.52/0.53 were never bisected, so 0.54 as a hard minimum was asserted, not verified. State the rule that IS verified - streaming on Falcon requires Ruby >= 3.2 - and describe the Falcon versions (0.51 and earlier broken, 0.54+ works) rather than pinning an exact floor we did not prove. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @samuel-williams-shopify. All three addressed:
I also dropped the earlier "future work, remove the probe once async-http reaps idle HTTP/1 natively" framing (code + README): per the socketry/async-http#224 investigation, the heartbeat is the deliberate mechanism, not a stopgap. And I documented the per-server streaming floor after verifying SSE on real Puma across Ruby 2.7/3.0/3.1/4.0 and Rack 3.0 to 3.2. Falcon streaming needs Ruby >= 3.2 (older Ruby only resolves Falcon <= 0.51, which never flushes the callable body). |
|
Heads up, the red CI here is just flakiness, not the change. A few jobs timed out on apt installs, and Ruby 3.1 tripped on an i18n 1.15.0 bug (it calls There's no |
|
Only did a cursory look through the code, no detailed review, but I generally very much approve of this. Thanks for putting in the work! |
What
This makes Sinatra's streaming (
stream,stream :keep_open) production-safe by default and moves it onto the Rack 3 callable-body contract, with first-class Server-Sent Events support. It closes the long-standing gap (#1892) where akeep_openstream whose client vanished could park forever, leaking a fiber, a file descriptor, and on Falcon a connection slot, until the process was restarted.Because it also removes the dead EventMachine/Thin async path and changes
Stream.new's signature, this targets the 5.0 line.Why
The old behaviour assumed a cooperative client that always closes the connection cleanly. In the real world clients drop, proxies time out, and load balancers reset idle sockets without a FIN. Under Falcon those abandoned streams accumulated with no backstop. There was also no way to cap concurrent streams, no hook to observe a producer-side error (it was silently turned into a clean close), and no built-in SSE framing, so every app rolled its own and usually got
id/eventescaping wrong.Server-aware disconnect reaping
There is no portable way to learn that a streaming client has gone, so detection is per server and protocol:
MSG_PEEKdetects a half-closed peer without writing anything to the wire. This is the preferred path whenever the stream is a realBasicSocket.text/event-streamresponses, where the probe is an invisible:comment line, so a non-SSE body is never given injected bytes. For a non-SSEkeep_openstream on Falcon, passheartbeat:to opt in. The internal heartbeat fires only once the stream has been write-idle for apollinterval, so an actively-streaming endpoint never gets redundant probe bytes (its ownout.write/out.ssealready exercise the transport).The heartbeat is a Falcon-HTTP/1-only mechanism, and a deliberate one rather than a stopgap awaiting an upstream fix: async-http cannot detect a disconnect on an idle HTTP/1 bidirectional body without either missing a FIN that sits behind buffered bytes or stealing request-body bytes the body owns. That investigation is at socketry/async-http#224 (still open, link kept at the probe site for background, not as a removal trigger).
MSG_PEEKis likewise a closure detector, not a liveness test — silent failures are caught by the finitekeep_openTTL, not the peek.Streaming server/version support
Streaming relies on the server driving a Rack 3 callable response body, so support is per server, not uniform across everything Sinatra runs on:
This also fixes CI on the older matrix cells: Falcon integration tests are skipped on Ruby < 3.2 (the buggy
RUBY_VERSION <= "3.0.0"string compare is replaced withGem::Version), and the harness-driven streaming tests skip on Rack < 3.2, whereRack::MockResponsecannot drive a callable body (body.call(io)landed in Rack 3.2; real servers drive callable bodies from Rack 3.0 on).Defaults
set :stream_keep_open_ttl(default 300s, plus jitter so a fleet does not reconnect on the same tick). Tune per stream withstream(..., ttl:), orttl: nilto opt a stream out.set :stream_max_concurrent(default 1000) sheds past the limit with503and aRetry-Afterheader (set :stream_retry_after, default 5) instead of parking another worker, so a flood of idle connections cannot starve the pool. Set tonilto disable.Both defaults are generous enough that normal apps never trip them, while bounding the worst case.
API
out.flushflushes the server stream (no auto-flush, you decide when).out.sse(data, event:, id:, retry_after:)writes one spec-correct SSE event (multi-linedata, JSON-encoded non-strings,\0\r\nstripped fromid/event).out.sse_comment(text)writes an application keepalive comment, distinct from the internal disconnect probe.Base.on_stream_error { |e| ... }is invoked when a producer block raises mid-stream, so apps can report it (Sentry, OpenTelemetry); the exception still propagates.:complete,:disconnect,:error); existing zero-aritycallbackblocks keep working.Last-Event-IDheader (request.env['HTTP_LAST_EVENT_ID']).Breaking changes (5.0)
async.callback,async.close,throw :async,async?.Sinatra::ExtendedRackis now an inert pass-through, no longer installed by default; an app that still uses it manually gets a deprecation warning.Sinatra::Helpers::Stream.newhas a new signature (it now carries TTL, protocol, concurrency, and error-handler state).sinatra-contrib, examples, tests
sinatra-contrib'sStreamingaddon is rewritten on the callable body (itsputs/printf/<</map!IO emulation preserved), so the monorepo build stays green.chat.rb,stream.ru, and a newsse.rb) use the new API and are covered by a new examples test so they cannot silently drift.Rack::Deflaterlimitation (Rack::Deflater crashes (NoMethodError: each) on Rack 3 streaming/callable bodies rack/rack#2470, filed), and the ExtendedRack removal at 6.0.out.ssekeeps per-event allocation minimal.Verification
A few things I would value your read on
Sinatra::ExtendedRack: I kept it as an inert deprecated shim for one major (the Reloader precedent), but the async audience is effectively gone, so I am happy to just remove the constant in 5.0 if you prefer.Streaminbase.rbwhere it already lives; happy to extract it into its own file if you would rather.Closes #1892