Add support for passing IO objects to Websockets.open()#1251
Conversation
|
One failure on 1.6, and it looks like the 32bit windows CI is hanging. |
JamesWrigley
force-pushed
the
websockets
branch
2 times, most recently
from
4d2b83c to
2fef03e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1251 +/- ##
==========================================
- Coverage 87.31% 87.16% -0.16%
==========================================
Files 29 29
Lines 11289 11326 +37
==========================================
+ Hits 9857 9872 +15
- Misses 1432 1454 +22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This is handy if the caller already has the required stream/socket open.
|
Now rewritten to work on v2 🤞 I think the design is ok (went through a couple rounds with Claude) though I'm certainly no expert. |
| """ | ||
| function open( | ||
| io::IO; | ||
| suppress_close_error::Bool=false, |
There was a problem hiding this comment.
this keyword isn't getting passed along to the _open_client_websocket_io call; I think I'd prefer a one-line definition here to simplify
| """ | ||
| Adapter that presents an arbitrary caller-provided `IO` for `WebSockets.open(io)`, | ||
| bypassing the connection pool entirely. | ||
|
|
||
| The key difference it papers over is read semantics: the HTTP/1 handshake parser | ||
| and the WebSocket read loop use `readbytes!(...; all=false)` and treat a `0` | ||
| return as EOF, relying on Reseau's "block until at least one byte or EOF" | ||
| behavior. A stdlib `TCPSocket` with `all=false` instead returns `0` whenever no | ||
| bytes are buffered *yet*, so we reimplement the blocking contract here on top of | ||
| `eof`/`bytesavailable`. | ||
| """ | ||
| struct _RawIOConn{T<:IO} <: IO | ||
| io::T | ||
| end | ||
|
|
||
| # Block until data is available or EOF, then return whatever is buffered (up to | ||
| # `nb`) — never a spurious `0` while the stream is still open. | ||
| function Base.readbytes!(c::_RawIOConn, b::AbstractVector{UInt8}, nb::Integer=length(b); all::Bool=false)::Int | ||
| eof(c.io) && return 0 | ||
| n = min(bytesavailable(c.io), Int(nb)) | ||
| return readbytes!(c.io, b, n) | ||
| end | ||
|
|
||
| Base.read(c::_RawIOConn, ::Type{UInt8}) = read(c.io, UInt8) | ||
| Base.write(c::_RawIOConn, x::UInt8) = write(c.io, x) | ||
| Base.unsafe_write(c::_RawIOConn, p::Ptr{UInt8}, n::UInt) = unsafe_write(c.io, p, n) | ||
| Base.eof(c::_RawIOConn) = eof(c.io) | ||
|
|
There was a problem hiding this comment.
I think this could all be a bit simpler than having a dedicated struct. I think you only really need 2 methods of a new HTTP-specific function called _blocking_readybtes! or something. For Reseau conn types, you'd pass through to readbytes!(; all=false) and for all other IO, you'd have the readbytes! implementation here.
I don't think I realized this subtle different in how Reseau.Conns work, but it seems simpler to have ~6 lines for _blocking_readybtes! vs. this whole struct + passthrough.
There was a problem hiding this comment.
Ah yeah, that is much simpler. Refactored in a21f95e.
2 participants
This is handy if the caller already has the required stream/socket open.
Written with help from Claude 🤖