feat: take the socket options callers actually pass - #49
Merged
Conversation
The list was what a socket happens to be settable to. That is how `inet` came to
be refused, and the fix for it left the same shape behind: `{ip, _}`,
`{linger, _}`, `{priority, _}` and friends were still refused, waiting for the
first caller who passes one.
Sorted by what the option decides rather than by what it looks like.
Read before connecting, because that is the only moment they mean anything:
`inet`, `inet6`, and now `{ip, _}` and `{port, _}`, which choose the local end
to bind - how a multi homed host picks the interface the server sees.
Set on the socket: the four that already were, and now `{priority, _}`,
`{reuseaddr, _}`, `{tos, _}`, `{bind_to_device, _}` and `{linger, _}`, which
takes `m:gen_tcp`'s `{OnOff, Seconds}` and gives the socket module the struct
the OS wants.
Accepted and ignored, and said out loud: `{delay_send, _}`, `{high_watermark,
_}`, `{low_watermark, _}`, `{exit_on_close, _}` and `{show_econnreset, _}`
configure a port's send queue and a port's reporting. There is no port here.
Anything else is still refused rather than dropped. The doc now says that a
refusal for an option the driver takes is a gap in this list rather than a
decision.
Taking `[gen_tcp:connect_option()]` rather than `[term()]` is what let the
option values be typed at all, which is how `{ip, _}` turned out to be wider
than an address: `any`, `loopback` and `{local, Path}` are all legal and now
handled.
🟡 Code Coverage — 88.6%1780 of 2008 lines covered. ✅ ELP LintNo diagnostics. |
Windows found both of these, which is what it is there for.
`priority` and `tos` are Linux's. Setting them on Windows answers
`{invalid, {socket_option, _}}`, and refusing the connection over that would
mean a config that works on one machine breaks on another - the same shape as
refusing `inet`, one layer along. The driver does not fail a connection over an
option the OS has never heard of, and neither does this now.
The refusal case assumed the driver answers `{error, _}` for an option it will
not take. On Windows it raises instead. Either is a refusal; what the case is
about is the transport that has to decide for itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The accept-list was built from what a socket can be set to. That is how
inetcame to be refused and an outage came within one merge of production - and fixing that one option left the same shape behind.{ip, _},{linger, _},{priority, _},{tos, _},{reuseaddr, _}and{bind_to_device, _}were all still refused, waiting for the first caller to pass one.Now sorted by what an option decides.
Read before connecting, the only moment they mean anything:
inet,inet6, and now{ip, _}and{port, _}, which choose the local end to bind - how a multi homed host picks which interface the server sees.Set on the socket: the four that already were, plus
{priority, _},{reuseaddr, _},{tos, _},{bind_to_device, _}and{linger, _}(translating gen_tcp's{OnOff, Seconds}into the struct the socket module wants).Accepted and ignored, and documented as such:
{delay_send, _},{high_watermark, _},{low_watermark, _},{exit_on_close, _},{show_econnreset, _}. These configure a port's send queue and a port's error reporting, and there is no port here.Anything else is still refused rather than silently dropped, and the guide now says plainly that hitting that with an option the driver takes is a gap in the list rather than a decision.
Tests
every_socket_option_the_list_claims_is_taken- all twenty in one connection, both transports.a_local_address_is_bound_not_ignored- binds TEST-NET-1, which no machine can have, and requireseaddrnotavail. Asking the server which address it sees proves nothing through a NAT, and a container is one.Both fail without the change. 1398 eunit, 184 ct, and the full static pipeline clean.
Type note
Taking
[gen_tcp:connect_option()]instead of[term()]is what made the option values typed at all, and eqWAlizer immediately found that{ip, _}is wider than an address:any,loopbackand{local, Path}are all legal. All three are handled now. That would have been a crash on a caller doing something perfectly ordinary.