Bound SQL Browser lookup and fall back to configured port - #187
Bound SQL Browser lookup and fall back to configured port#187goodcomm74 wants to merge 4 commits into
Conversation
mjaric
left a comment
mjaric
left a comment
There was a problem hiding this comment.
Hi @goodcomm74 ,
Thank you for your contribution. The PR already looks great and with just few more adjustments it will be perfect. Could you please look into my comments and do this micro changes. Once done I will be more than happy to merge this into master branch and make release.
Thank you!
| end | ||
|
|
||
| defp instance_timeout(opts) do | ||
| case Keyword.get(opts, :instance_timeout, @default_timeout) do |
There was a problem hiding this comment.
:instance_timeout is not documented, could you please add it to README.md in Configuration section. Mention default value too.
|
|
||
| defp browser_port(opts) do | ||
| opts | ||
| |> Keyword.get(:instance_browser_port, @default_browser_port) |
There was a problem hiding this comment.
instance_browser_port sa as for instance_timeout, please document it in README.md
|
|
||
| @type t :: %__MODULE__{ | ||
| sock: nil | sock, | ||
| usock: nil | pid, |
There was a problem hiding this comment.
All usages of usock are removed from code, please remove it from type definition
| } | ||
|
|
||
| defstruct sock: nil, | ||
| usock: nil, |
There was a problem hiding this comment.
All usages of usock are removed from code, please remove it from structure definition.
| end | ||
|
|
||
| defp find_instance([], _instance) do | ||
| error("SQL Server Browser returned a malformed response") |
There was a problem hiding this comment.
| error("SQL Server Browser returned a malformed response") | |
| error("No instances advertised") |
Probably better message
| defp normalize_port(port, label) when is_binary(port) do | ||
| case Integer.parse(String.trim(port)) do | ||
| {port, ""} when port in 1..65_535 -> {:ok, port} | ||
| _other -> error("#{label} is invalid: #{inspect(port)}") |
There was a problem hiding this comment.
normalize_port/2 now rejects values like "15433x" that the old code silently prefix-parsed. Nice fix, just add a line to the docs and CHANGELOG.md so it doesn't surprise anyone.
2 participants
Summary
Why
gen_udp.recv/2has no timeout, so a lost SQL Browser response can leave a DBConnection worker stuck indefinitely and prevent reconnect backoff from running. A configured fixed port is useful as a recovery path when UDP discovery is unavailable.Tests
The fallback remains opt-in: without
:port, discovery errors are returned as before.