Skip to content

Commit

Permalink
throw DNSError instead of UVError in getaddrinfo()
Browse files Browse the repository at this point in the history
  • Loading branch information
@samoconnor
samoconnor committed Apr 14, 2016
1 parent a056ce2 commit 0a716ba
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion base/socket.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,13 +582,27 @@ function getaddrinfo(cb::Function, host::ASCIIString)
end
getaddrinfo(cb::Function, host::AbstractString) = getaddrinfo(cb,ascii(host))


type DNSError <: Exception
uverror::UVError
host::AbstractString
end

function show(io::IO, e::DNSError)
print(io, "DNSError: ", e.host, ", ")
show(io, e.uverror)
end

function getaddrinfo(host::ASCIIString)
c = Condition()
getaddrinfo(host) do IP
notify(c,IP)
end
ip = wait(c)
isa(ip,UVError) && throw(ip)
if isa(ip,UVError)
@assert ip.code in [UV_EAI_NONAME, UV_EAI_AGAIN]

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Apr 15, 2016

Collaborator

should throw the actual UVError if the code isn't one of these

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Apr 15, 2016

Author Owner

The goal is to make UVError invisible to the user.

A review of the possible errors per getaddrinfo(3) suggests that:

  • The following are impossible (or if they occur, there is a bug in Julia Base, or UVLib), so @assert is appropriate:
    • EAI_FAMILY and EAI_ADDRFAMILY because ai_family is hard coded to AF_UNSPEC
    • EAI_BADFLAGS because ai_flags is hard coded to AI_CANONNAME
    • EAI_SERVICE because service is hard coded to NULL
    • EAI_SOCKTYPE because ai_socktype is hard coded to SOCK_STREAM and ai_protocol to 0
  • EAI_FAIL and EAI_NODATA should be added to the list because they mean essentially "can't find that address".
  • EAI_SYSTEM and EAI_MEMORY should be converted to existing Julia types. e.g.
systemerror("$(e.prefix), $host", ip.code == EAI_SYSTEM)
EAI_MEMORY && throw(OutOfMemoryError())
@assert ip.code in [UV_EAI_NONAME, UV_EAI_AGAIN, EAI_FAIL, EAI_NODATA]
throw(DNSError(ip, host))

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Apr 15, 2016

Collaborator

The assert is going to provide less information to the user in case one of the "impossible" bugs-in-base/libuv situations occurs than throwing the UVError would. Throwing the UVError describes what happens in more detail.

This comment has been minimized.

Sign in to view

Copy link
@vtjnash

vtjnash Apr 15, 2016

occasionally windows throws some weird errors at us, since it doesn't enumerating the possible exceptions

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Apr 15, 2016

Author Owner

How does uvlib map the "weird" windows errors onto the UV_EAI_ constants? EAI_SYSTEM ?

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Apr 15, 2016

Collaborator

It doesn't map them and usually throws a C-level assertion. Which is a pain because then you have to go add a printf and recompile from source to actually figure out what code got thrown and add it to their switch-case (like in libuv/libuv@7b9bc28)

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Apr 15, 2016

Author Owner

MS now has a WSL layer that emulates Linux well enough to run ubuntu binaries.

Is there any fundamental reason that Julia couldn't just target WSL instead of Win32?

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Apr 15, 2016 •
edited
Loading

Collaborator

Windows 7 isn't going anywhere and is the most widely used (desktop) operating system version in the world by a factor of 3 or 4. WSL has tons of unimplemented syscalls, it's not actually usable yet, and there's zero interoperability between WSL and Win32 programs and libraries. Eventually WSL might be a decent way to run the Linux version of Julia for people who are able to use Windows 10 and have admin rights on their own machine. For the rest of the world, and anyone who needs to embed Julia in a Windows application, Win32 support can't be removed.
throw(DNSError(ip, host))
end
return ip::IPAddr
end
getaddrinfo(host::AbstractString) = getaddrinfo(ascii(host))
Expand Down

0 comments on commit 0a716ba

Please sign in to comment.