Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Report target host/port with connection exception #529

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Report target host/port with connection exception #529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Network/Socket/SockAddr.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import qualified Network.Socket.Name as G
import qualified Network.Socket.Syscall as G
import Network.Socket.Flag
import Network.Socket.Imports
import Network.Socket.Info ()
#if !defined(mingw32_HOST_OS)
import Network.Socket.Posix.Cmsg
#else
Expand Down
10 changes: 5 additions & 5 deletions Network/Socket/Syscall.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,14 @@ bind s sa = withSocketAddress sa $ \p_sa siz -> void $ withFdSocket s $ \fd -> d
-- Connecting a socket

-- | Connect to a remote socket at address.
connect :: SocketAddress sa => Socket -> sa -> IO ()
connect :: (Show sa, SocketAddress sa) => Socket -> sa -> IO ()
connect s sa = withSocketsDo $ withSocketAddress sa $ \p_sa sz ->
connectLoop s p_sa (fromIntegral sz)
connectLoop (show sa) s p_sa (fromIntegral sz)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

showed socket addr is lazy right? so no problem with that,
though I think SockAddr Show instance is doing too much and might fail due IO and hide original error.
I would fall back to generated Show and move current Show implementation to a new dedicated method of SocketAddress class. I don't know how much legacy would be affected by such refactoring.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kazu-yamamoto

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to introduce the Strict pragma someday and don't want to relay on the lazy evaluation. So, what about just passing sa to connectLoop and connectLoop calls show by itself?


connectLoop :: SocketAddress sa => Socket -> Ptr sa -> CInt -> IO ()
connectLoop s p_sa sz = withFdSocket s $ \fd -> loop fd
connectLoop :: SocketAddress sa => String -> Socket -> Ptr sa -> CInt -> IO ()
connectLoop show_sa s p_sa sz = withFdSocket s $ \fd -> loop fd
where
errLoc = "Network.Socket.connect: " ++ show s
errLoc = "Network.Socket.connect: " ++ show s ++ " " ++ show_sa
loop fd = do
r <- c_connect fd p_sa sz
when (r == -1) $ do
Expand Down
13 changes: 11 additions & 2 deletions tests/Network/SocketSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}

module Network.SocketSpec (main, spec) where

import Control.Concurrent (threadDelay, forkIO)
import Control.Concurrent.MVar (readMVar)
import Control.Exception (SomeException)
import Control.Monad
import Data.Maybe (fromJust)
import Data.List (nub)
import Data.List (isInfixOf, nub)
import Network.Socket
import Network.Socket.ByteString
import Network.Test.Common
Expand All @@ -32,9 +34,16 @@ spec = do
sock <- socket (addrFamily addr) (addrSocketType addr) (addrProtocol addr)
connect sock (addrAddress addr)
return sock
assumedFreePort = 8003 :: Int

it "fails to connect and throws an IOException" $ do
connect' (8003 :: Int) `shouldThrow` anyIOException
connect' assumedFreePort `shouldThrow` anyIOException

it "fails to connect exception contains target port" $ do
connect' assumedFreePort `shouldThrow` \(e :: SomeException) -> show assumedFreePort `isInfixOf` show e

it "fails to connect exception contains target host" $ do
connect' assumedFreePort `shouldThrow` \(e :: SomeException) -> serverAddr `isInfixOf` show e

it "successfully connects to a socket with no exception" $ do
withPort $ \portVar -> test (tcp serverAddr return portVar)
Expand Down