Split socket/SSLSocket nonblock methods into exception: overloads#2963
Split socket/SSLSocket nonblock methods into exception: overloads#2963zonuexe wants to merge 3 commits into
exception: overloads#2963Conversation
The `exception:` keyword of the non-blocking socket methods selects the return value: the normal result when `exception: true` (the default), or a `:wait_readable` / `:wait_writable` symbol when `exception: false`. Express this with `true`/`false` literal overloads instead of a single `?exception: boolish` parameter with a union return type.
…erloads `SSLSocket#sysread_nonblock` and `#syswrite_nonblock` were typed as `(*untyped) -> untyped`. Like the other non-blocking methods, their `exception:` keyword selects the return value: the normal result when `exception: true` (the default), or a `:wait_readable` / `:wait_writable` symbol (or `nil`) when `exception: false`. Express this with `true`/`false` literal overloads.
exception: overloadsexception: overloads
| def syswrite_nonblock: (_ToS string, ?exception: true) -> Integer | ||
| | (_ToS string, exception: false) -> (Integer | :wait_readable | :wait_writable | nil) |
There was a problem hiding this comment.
The _ToS interface applies to all objects except BasicObject (e.g., nil, Hash, etc.). However, the actual implementation uses StringValue, and objects like nil should be excluded. I think string is appropriate.
And, Based on the implementation, syswrite_nonblock raises an error even when exception: false is specified. This is asymmetric to sysread_nonblock.
It seems there is no possibility of it returning nil.
| # | ||
| def recv_nonblock: (Integer maxlen, ?Integer flags, ?String buf, ?exception: boolish) -> (String | :wait_readable) | ||
| def recv_nonblock: (Integer maxlen, ?Integer flags, ?String buf, ?exception: true) -> String | ||
| | (Integer maxlen, ?Integer flags, ?String buf, exception: false) -> (String | :wait_readable) |
There was a problem hiding this comment.
Since the documentation states that Socket#recv_nonblock returns nil, there is a possibility that nil may be returned; the same may be true for recvmsg_nonblock and Socket#recvfrom_nonblock.
Per review feedback:
- `SSLSocket#syswrite_nonblock`:
- Use `string` (= `String | _ToStr`) instead of `_ToS` for the
argument: the C implementation uses `StringValue`, which converts
via `to_str`, so `nil`, `Hash`, etc. are not accepted.
- Remove `nil` from the `exception: false` return: unlike
`sysread_nonblock`, write has no EOF-equivalent path that returns
`nil`; it raises on real errors and only converts wait-state into
`:wait_readable` / `:wait_writable`.
- `BasicSocket#recv_nonblock`, `BasicSocket#recvmsg_nonblock`,
`Socket#recvfrom_nonblock`, `UDPSocket#recvfrom_nonblock`: include
`nil` in both `exception: true` and `exception: false` overloads.
The rdoc states that these methods return `nil` when the underlying
`recvfrom(2)` returns 0 (closed connection or empty datagram),
regardless of `exception:`.
| # * Socket#recvfrom | ||
| # | ||
| def recvfrom_nonblock: (Integer maxlen, ?Integer flags, ?untyped outbuf, ?exception: boolish) -> ([ String, Addrinfo ] | :wait_readable) | ||
| def recvfrom_nonblock: (Integer maxlen, ?Integer flags, ?untyped outbuf, ?exception: true) -> [ String, Addrinfo ]? |
There was a problem hiding this comment.
Claude Code told me that nil won't return from this method. Is it correct?
| # | ||
| def sysread_nonblock: (*untyped) -> untyped | ||
| def sysread_nonblock: (Integer length, ?String buffer, ?exception: true) -> String | ||
| | (Integer length, ?String buffer, exception: false) -> (String | :wait_readable | :wait_writable | nil) |
There was a problem hiding this comment.
| | (Integer length, ?String buffer, exception: false) -> (String | :wait_readable | :wait_writable | nil) | |
| | (Integer length, ?String buffer, exception: boolish) -> (String | :wait_readable | :wait_writable | nil) |
exception: keyword parameter must be wider than false, because the precise type information may be lost: exception = rand() > 0.5 #: bool.
I found exception: accepts any object (..., exception: "true"), but it's really complicated to support these cases. So, the assumption, programmers will write the exception: true, would make sense.
Summary
The non-blocking socket methods (and OpenSSL
SSLSocketequivalents) take anexception:keyword that selects the return value: the normal result whenexception: true(the default), or a:wait_readable/:wait_writablesymbol whenexception: false. The signatures expressed this with a single?exception: boolishparameter and a union return type (or plainuntyped), so the symbol leaked into the result type even whenexception:was not passed.This splits each method into
true/falseliteral overloads so the return type reflects the argument — the same treatment applied toStringScanner#scan_full/#search_fullin #2959.BasicSocket#recv_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_recvfrom_nonblockBasicSocket#recvmsg_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_bsock_recvmsg_nonblockBasicSocket#sendmsg_nonblockexception:intotrue/falseoverloads (false→… | :wait_writable)rsock_bsock_sendmsg_nonblockOpenSSL::SSL::SSLSocket#sysread_nonblock(*untyped) -> untypedwithtrue/falseoverloads (false→… | :wait_readable | :wait_writable | nil)ossl_ssl_read_nonblockOpenSSL::SSL::SSLSocket#syswrite_nonblock(*untyped) -> untypedwithtrue/falseoverloads (false→… | :wait_readable | :wait_writable | nil)ossl_ssl_write_nonblockSocket#accept_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_accept_nonblockSocket#connect_nonblockexception:intotrue/falseoverloads (false→… | :wait_writable)sock_connect_nonblockSocket#recvfrom_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_recvfrom_nonblockTCPServer#accept_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_accept_nonblockUDPSocket#recvfrom_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_recvfrom_nonblockUNIXServer#accept_nonblockexception:intotrue/falseoverloads (false→… | :wait_readable)rsock_s_accept_nonblockOut of scope
read_nonblock,write_nonblock,connect_nonblock, andaccept_nonblockmethods already usetrue/falseoverloads — no change needed.read_nonblock/write_nonblockare not redefined understdlib/socket; they inherit the coreIOsignatures, which already have the overloads.PTY.check(pid, raise)is a related parameter-driven return type (raise: truecan only returnnil), but it belongs to a different method family rather than non-blocking I/O; left for a separate change.Test plan
bundle exec rbs -r socket validatebundle exec rbs -r openssl validatebundle exec rubocop stdlib/socket/0/ stdlib/openssl/0/openssl.rbs