lpeterse / haskell-socket

A Haskell binding to the POSIX sockets API
MIT License
47 stars 10 forks source link

Exception: Add EOPNOTSUPP. #9

Closed nh2 closed 9 years ago

nh2 commented 9 years ago

This happens e.g. when you try to accept on a socket that's not a Stream socket.

I haven't tested whether it works on Windows, please do that for me :)

lpeterse commented 9 years ago

I also added a test and validated it on Windows. The new version on Hackage is 0.5.3.0. Thanks for the pull request.

nh2 commented 9 years ago

Cool. I wonder if we could do something so that the exceptions make clear from which function call they originate. For example, if you accept on a non-Stream socket, you currently just get the exception bubbled up that says eOperationNotSupported. It would be more useful if it could say e.g. accept: eOperationNotSupported. Then you know where the problem is, as opposed to only knowing that there is one.

lpeterse commented 9 years ago

@nh2: This came to my mind, too. For some time I even thought it'd be a good idea to have a separate exception type for each operation. But then I decided to not overengineer it all. The library is intended to be a thin layer around the sockets API and the exceptions are currently only newtype'd errnos.

My conclusion was: If you're interested in where an exception was thrown you should catch or at least catch-transform-rethrow it right there.

What did you have in mind? How would you annotate the exception with their origin?

nh2 commented 9 years ago

What did you have in mind? How would you annotate the exception with their origin?

I had something very simple in mind actually: Catch the exception in each function where it can happen (e.g. connect, accept), and turn e.g. eOperationNotSupported into accept: eOperationNotSupported.

By the way, wouldn't it be better to use base's error handling functionalities? See e.g. https://hackage.haskell.org/package/base-4.8.1.0/docs/Foreign-C-Error.html#g:3

lpeterse commented 9 years ago

It already has cross-platform error numbers

This assumption is wrong (at least on Windows). I'll give proof in a minute, but I just wanted to let you know before you start investing time into this.

Still, I'm glad that you take the time to review my library :-)

lpeterse commented 9 years ago
C:\Users\Lars>ghci
GHCi, version 7.8.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> import Foreign.C.Error
Prelude Foreign.C.Error> let Errno i = ePROTONOSUPPORT
Prelude Foreign.C.Error> i
-1
Prelude Foreign.C.Error> let Errno i = eISCONN
Prelude Foreign.C.Error> i
-1
Prelude Foreign.C.Error> let Errno i = eINPROGRESS
Prelude Foreign.C.Error> i
-1
Prelude Foreign.C.Error> let Errno i = eCONNREFUSED
Prelude Foreign.C.Error> i
-1
nh2 commented 9 years ago

This assumption is wrong (at least on Windows).

Ah, do you know why this is? I thought this was supposed to work.

Independently, it seems that getErrno and errnoToIOError can still be useful, can't they?

lpeterse commented 9 years ago
int hs_get_last_socket_error(void) {
  return WSAGetLastError();
};

The problem is that at least on Windows you don't collect socket exceptions via errno. We could of course use wire c_get_last_socket_error to use getErrno on Linux etc., but I'll need to have a closer look first.

errnoToIOError uses the wrong errno definitions internally, so it can't be used.

lpeterse commented 9 years ago

Mhh, maybe the issue with the wrong errnos should be solved in base once and for all.

nh2 commented 9 years ago

errnoToIOError uses the wrong errno definitions internally, so it can't be used.

Ah right, I see. What's your opinion on copying (unfortunately it's not exported) the foreign import ccall unsafe "string.h" strerror :: Errno -> IO (Ptr CChar) and using it to get a full error message?

nh2 commented 9 years ago

Mhh, maybe the issue with the wrong errnos should be solved in base once and for all.

That'd be my favourite. Cross-platform networking is hard enough, if base just has -1 for some of those on Windows (and the docs don't even seem to mention that, or did I overlook it?), that's really making life harder than it needs to be.

lpeterse commented 9 years ago

I'm okay with this if we get it to work and behave equally on all platforms :-)

lpeterse commented 9 years ago

Here is the Windows equivalent: https://stackoverflow.com/questions/3400922/how-do-i-retrieve-an-error-string-from-wsagetlasterror