Skip to content

Commit b9bcc88

Browse files
cootcoot
committed
connection-manager: propagate disconnection exceptions
Most importantly propagate that the connection was torn down because we reached inbound hard limit. We also modify `HandleError`: it is no longer parametrised by `muxMode` to avoid `Typeable muxMode` instance propagating through the code base which could suggest we're doing type reflection on it.
1 parent b496fb4 commit b9bcc88

File tree

8 files changed

+156
-88
lines changed

8 files changed

+156
-88
lines changed

‎ouroboros-network-framework/demo/connection-manager.hs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,6 @@ bidirectionalExperiment
567567
Mux.InitiatorResponderMode peerAddr
568568
UnversionedProtocolData ByteString IO () ())
569569
(HandleError
570-
Mux.InitiatorResponderMode
571570
UnversionedProtocol))
572571
connect n cm | n <= 1 =
573572
acquireOutboundConnection cm InitiatorAndResponderDiffusionMode remoteAddr

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionHandler.hs‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -155,29 +155,35 @@ type HandleWithMinimalCtx muxMode peerAddr versionData bytes m a b =
155155
(ResponderContext peerAddr)
156156
versionData bytes m a b
157157

158-
data HandleError (muxMode :: Mx.Mode) versionNumber where
158+
data HandleError versionNumber where
159159
HandleHandshakeClientError
160-
:: HasInitiator muxMode ~ True
161-
=> !(HandshakeException versionNumber)
162-
-> HandleError muxMode versionNumber
160+
:: !(HandshakeException versionNumber)
161+
-> HandleError versionNumber
163162

164163
HandleHandshakeServerError
165-
:: HasResponder muxMode ~ True
166-
=> !(HandshakeException versionNumber)
167-
-> HandleError muxMode versionNumber
164+
:: !(HandshakeException versionNumber)
165+
-> HandleError versionNumber
168166

169167
HandleError
170168
:: !SomeException
171-
-> HandleError muxMode versionNumber
169+
-> HandleError versionNumber
172170

173171
instance Show versionNumber
174-
=> Show (HandleError muxMode versionNumber) where
172+
=> Show (HandleError versionNumber) where
175173
show (HandleHandshakeServerError err) = "HandleHandshakeServerError " ++ show err
176174
show (HandleHandshakeClientError err) = "HandleHandshakeClientError " ++ show err
177175
show (HandleError err) = "HandleError " ++ show err
178176

177+
instance ( Typeable versionNumber
178+
, Show versionNumber
179+
)
180+
=> Exception (HandleError versionNumber) where
181+
displayException (HandleHandshakeClientError err) = "handshake client error: " ++ displayException err
182+
displayException (HandleHandshakeServerError err) = "handshake server error: " ++ displayException err
183+
displayException (HandleError err) = show err
179184

180-
classifyHandleError :: HandleError muxMode versionNumber
185+
186+
classifyHandleError :: HandleError versionNumber
181187
-> HandleErrorType
182188
classifyHandleError (HandleHandshakeClientError (HandshakeProtocolLimit _)) =
183189
HandshakeProtocolViolation
@@ -202,7 +208,7 @@ type MuxConnectionHandler muxMode socket initiatorCtx responderCtx peerAddr vers
202208
socket
203209
peerAddr
204210
(Handle muxMode initiatorCtx responderCtx versionData bytes m a b)
205-
(HandleError muxMode versionNumber)
211+
(HandleError versionNumber)
206212
versionNumber
207213
versionData
208214
m
@@ -212,15 +218,15 @@ type MuxConnectionHandler muxMode socket initiatorCtx responderCtx peerAddr vers
212218
type MuxConnectionManager muxMode socket initiatorCtx responderCtx peerAddr versionData versionNumber bytes m a b =
213219
ConnectionManager muxMode socket peerAddr
214220
(Handle muxMode initiatorCtx responderCtx versionData bytes m a b)
215-
(HandleError muxMode versionNumber)
221+
(HandleError versionNumber)
216222
m
217223

218224
-- | Type alias for 'ConnectionManager' which is using expanded context.
219225
--
220226
type ConnectionManagerWithExpandedCtx muxMode socket peerAddr versionData versionNumber bytes m a b =
221227
ConnectionManager muxMode socket peerAddr
222228
(HandleWithExpandedCtx muxMode peerAddr versionData bytes m a b)
223-
(HandleError muxMode versionNumber)
229+
(HandleError versionNumber)
224230
m
225231

226232
-- | To be used as `makeConnectionHandler` field of 'ConnectionManagerArguments'.
@@ -298,15 +304,14 @@ makeConnectionHandler muxTracers forkPolicy
298304
throwIO (ExceptionInHandler remoteAddress err)
299305

300306
outboundConnectionHandler
301-
:: HasInitiator muxMode ~ True
302-
=> InResponderMode muxMode ( StrictTVar m (StrictMaybe ResponderCounters)
307+
:: InResponderMode muxMode ( StrictTVar m (StrictMaybe ResponderCounters)
303308
-> Tracer m (WithBearer (ConnectionId peerAddr) Trace)
304309
, versionData -> DataFlow)
305310
-> ConnectionHandlerFn (ConnectionHandlerTrace versionNumber versionData)
306311
socket
307312
peerAddr
308313
(Handle muxMode initiatorCtx responderCtx versionData ByteString m a b)
309-
(HandleError muxMode versionNumber)
314+
(HandleError versionNumber)
310315
versionNumber
311316
versionData
312317
m
@@ -382,14 +387,13 @@ makeConnectionHandler muxTracers forkPolicy
382387

383388

384389
inboundConnectionHandler
385-
:: HasResponder muxMode ~ True
386-
=> ( StrictTVar m (StrictMaybe ResponderCounters)
390+
:: ( StrictTVar m (StrictMaybe ResponderCounters)
387391
-> Tracer m (WithBearer (ConnectionId peerAddr) Trace))
388392
-> ConnectionHandlerFn (ConnectionHandlerTrace versionNumber versionData)
389393
socket
390394
peerAddr
391395
(Handle muxMode initiatorCtx responderCtx versionData ByteString m a b)
392-
(HandleError muxMode versionNumber)
396+
(HandleError versionNumber)
393397
versionNumber
394398
versionData
395399
m

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Core.hs‎

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -947,28 +947,32 @@ with args@Arguments {
947947
, toState = Known connState
948948
})
949949
return ( State.insert connId connVar state
950-
, Just (connVar, connThread, reader)
950+
, Right (connVar, connThread, reader)
951951
)
952952
else
953953
return ( state
954-
, Nothing
954+
, Left ReachedInboundConnectionHardLimit
955955
)
956956

957957
case r of
958-
Nothing ->
959-
return (Disconnected connId Nothing)
958+
Left reason ->
959+
-- we were unable to include the connection due to hard inbound
960+
-- connection limit
961+
return (Disconnected connId reason)
960962

961-
Just (mutableConnState@MutableConnState { connVar, connStateId }
962-
, connThread, reader) -> do
963+
Right ( mutableConnState@MutableConnState { connVar, connStateId }
964+
, connThread, reader) -> do
963965
traceCounters stateVar
964966

965967
res <- atomically $ readPromise reader
966968
case res of
967969
Left handleError -> do
968-
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState $ Just handleError
970+
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState
971+
(ConnectionHandlerError handleError)
969972

970973
Right HandshakeConnectionQuery -> do
971-
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState Nothing
974+
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState
975+
ConnectionDisconnectedByHandshakeQuery
972976

973977
Right (HandshakeConnectionResult handle (_version, versionData)) -> do
974978
let dataFlow = connectionDataFlow versionData
@@ -1025,10 +1029,10 @@ with args@Arguments {
10251029
throwSTM (withCallStack (ImpossibleState (remoteAddress connId)))
10261030

10271031
TerminatingState _ _ err ->
1028-
return (Left err, Nothing, Inbound)
1032+
return (Left (mkDisconnectionException ConnectionInTerminatingState err), Nothing, Inbound)
10291033

10301034
TerminatedState err ->
1031-
return (Left err, Nothing, Inbound)
1035+
return (Left (mkDisconnectionException ConnectionInTerminatedState err), Nothing, Inbound)
10321036

10331037
traverse_ (traceWith trTracer . TransitionTrace connStateId) mbTransition
10341038
traceCounters stateVar
@@ -1068,23 +1072,26 @@ with args@Arguments {
10681072
-> StrictTMVar
10691073
m (ConnectionManagerState peerAddr handle handleError version m)
10701074
-> MutableConnState peerAddr handle handleError version m
1071-
-> Maybe handleError
1075+
-> DisconnectionException handleError
10721076
-> m (Connected peerAddr handle handleError)
1073-
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState handleErrorM = do
1077+
terminateInboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState connectionError = do
10741078
transitions <- atomically $ do
10751079
connState <- readTVar connVar
10761080

10771081
let connState' =
1078-
case classifyHandleError <$> handleErrorM of
1079-
Just HandshakeFailure ->
1082+
case classifyHandleError <$> connectionError of
1083+
ConnectionHandlerError HandshakeFailure ->
10801084
TerminatingState connId connThread
1081-
handleErrorM
1082-
Just HandshakeProtocolViolation ->
1083-
TerminatedState handleErrorM
1085+
(case connectionError of
1086+
ConnectionHandlerError err -> Just err
1087+
_ -> Nothing)
1088+
ConnectionHandlerError HandshakeProtocolViolation ->
1089+
TerminatedState (case connectionError of
1090+
ConnectionHandlerError err -> Just err
1091+
_ -> Nothing)
10841092
-- On inbound query, connection is terminating.
1085-
Nothing ->
1086-
TerminatingState connId connThread
1087-
handleErrorM
1093+
_ ->
1094+
TerminatingState connId connThread Nothing
10881095
transition = mkTransition connState connState'
10891096
absConnState = State.abstractState (Known connState)
10901097
shouldTrace = absConnState /= TerminatedSt
@@ -1148,7 +1155,7 @@ with args@Arguments {
11481155
traverse_ (traceWith trTracer . TransitionTrace connStateId) transitions
11491156
traceCounters stateVar
11501157

1151-
return (Disconnected connId handleErrorM)
1158+
return (Disconnected connId connectionError)
11521159

11531160
-- We need 'mask' in order to guarantee that the traces are logged if an
11541161
-- async exception lands between the successful STM action and the logging
@@ -1619,10 +1626,12 @@ with args@Arguments {
16191626
res <- atomically (readPromise reader)
16201627
case res of
16211628
Left handleError -> do
1622-
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState $ Just handleError
1629+
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState
1630+
(ConnectionHandlerError handleError)
16231631

16241632
Right HandshakeConnectionQuery -> do
1625-
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState Nothing
1633+
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState
1634+
ConnectionDisconnectedByHandshakeQuery
16261635

16271636
Right (HandshakeConnectionResult handle (_version, versionData)) -> do
16281637
let dataFlow = connectionDataFlow versionData
@@ -1685,7 +1694,7 @@ with args@Arguments {
16851694
return (Right $ mkTransition connState connState')
16861695

16871696
TerminatedState err ->
1688-
return $ Left err
1697+
return $ Left $ mkDisconnectionException ConnectionInTerminatedState err
16891698
_ ->
16901699
let st = State.abstractState (Known connState) in
16911700
throwSTM (withCallStack (ForbiddenOperation peerAddr st))
@@ -1773,12 +1782,12 @@ with args@Arguments {
17731782

17741783
TerminatingState _connId _connThread handleError ->
17751784
return ( Right (TrTerminatingConnection provenance connId)
1776-
, Disconnected connId handleError
1785+
, Disconnected connId (mkDisconnectionException ConnectionInTerminatingState handleError)
17771786
)
17781787
TerminatedState handleError ->
17791788
return ( Right (TrTerminatedConnection provenance
17801789
(remoteAddress connId))
1781-
, Disconnected connId handleError
1790+
, Disconnected connId (mkDisconnectionException ConnectionInTerminatedState handleError)
17821791
)
17831792

17841793
case etr of
@@ -1799,27 +1808,31 @@ with args@Arguments {
17991808
-> Async m ()
18001809
-> StrictTMVar m (ConnectionManagerState peerAddr handle handleError version m)
18011810
-> MutableConnState peerAddr handle handleError version m
1802-
-> Maybe handleError
1811+
-> DisconnectionException handleError
18031812
-> m (Connected peerAddr handle handleError)
1804-
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState handleErrorM = do
1813+
terminateOutboundWithErrorOrQuery connId connStateId connVar connThread stateVar mutableConnState connectionError = do
18051814
transitions <- atomically $ do
18061815
connState <- readTVar connVar
18071816

18081817
let connState' =
1809-
case classifyHandleError <$> handleErrorM of
1810-
Just HandshakeFailure ->
1818+
case classifyHandleError <$> connectionError of
1819+
ConnectionHandlerError HandshakeFailure ->
18111820
TerminatingState connId connThread
1812-
handleErrorM
1813-
Just HandshakeProtocolViolation ->
1814-
TerminatedState handleErrorM
1821+
(case connectionError of
1822+
ConnectionHandlerError err -> Just err
1823+
_ -> Nothing)
1824+
ConnectionHandlerError HandshakeProtocolViolation ->
1825+
TerminatedState (case connectionError of
1826+
ConnectionHandlerError err -> Just err
1827+
_ -> Nothing)
18151828
-- On outbound query, connection is terminated.
1816-
Nothing ->
1817-
TerminatedState handleErrorM
1829+
_ ->
1830+
TerminatedState Nothing
18181831
transition = mkTransition connState connState'
18191832
absConnState = State.abstractState (Known connState)
18201833
shouldTransition = absConnState /= TerminatedSt
18211834

1822-
-- 'handleError' might be either a handshake negotiation
1835+
-- 'connectionError' might be either a handshake negotiation
18231836
-- a protocol failure (an IO exception, a timeout or
18241837
-- codec failure). In the first case we should not reset
18251838
-- the connection as this is not a protocol error.
@@ -1870,7 +1883,7 @@ with args@Arguments {
18701883
traverse_ (traceWith trTracer . TransitionTrace connStateId) transitions
18711884
traceCounters stateVar
18721885

1873-
return (Disconnected connId handleErrorM)
1886+
return (Disconnected connId connectionError)
18741887

18751888

18761889
releaseOutboundConnectionImpl

‎ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Types.hs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ module Ouroboros.Network.ConnectionManager.Types
130130
, InboundConnectionManager (..)
131131
-- * Exceptions
132132
, ConnectionManagerError (..)
133+
, DisconnectionException (..)
134+
, mkDisconnectionException
133135
, SomeConnectionManagerError (..)
134136
, AbstractState (..)
135137
-- * Counters
@@ -493,7 +495,7 @@ data Connected peerAddr handle handleError =
493495
-- update from the handshake instead, but this would introduce a race
494496
-- between inbound \/ outbound threads.
495497
--
496-
| Disconnected !(ConnectionId peerAddr) !(Maybe handleError)
498+
| Disconnected !(ConnectionId peerAddr) !(DisconnectionException handleError)
497499

498500

499501
type AcquireOutboundConnection peerAddr handle handleError m
@@ -743,7 +745,7 @@ data ConnectionManagerError peerAddr
743745
-- was expected.
744746
--
745747
| UnknownPeer !peerAddr !CallStack
746-
deriving (Show)
748+
deriving Show
747749

748750

749751
instance ( Show peerAddr
@@ -833,6 +835,48 @@ connectionManagerErrorFromException x = do
833835
SomeConnectionManagerError a <- fromException x
834836
cast a
835837

838+
839+
-- | Disconnection contextual information returned by `Disconnected`.
840+
--
841+
data DisconnectionException handlerError =
842+
-- | We tried to accept or acquire a connection but the connection
843+
-- manager moved it to `TerminatingState`.
844+
ConnectionInTerminatingState
845+
846+
-- | We tried to accept or acquire a connection but the connection
847+
-- manager moved it to `TerminatedState`.
848+
| ConnectionInTerminatedState
849+
850+
-- | The connection was refused because we reached inbound connection hard
851+
-- limit.
852+
| ReachedInboundConnectionHardLimit
853+
854+
-- | `ConnectionDisconnectedByHandshakeQuery` is returned when the remote
855+
-- side is doing a handshake query. It's not an exception when handshake is
856+
-- terminated after a query, it should be handled in a special way.
857+
| ConnectionDisconnectedByHandshakeQuery
858+
859+
-- | Connection handler raised an exception.
860+
| ConnectionHandlerError handlerError
861+
deriving (Show, Functor)
862+
863+
864+
instance Exception handleError
865+
=> Exception (DisconnectionException handleError) where
866+
displayException ConnectionInTerminatingState = "disconnected: connection in terminating state"
867+
displayException ConnectionInTerminatedState = "disconnected: connection in terminated state"
868+
displayException ReachedInboundConnectionHardLimit = "disconnected: inbound connection hard limit reached"
869+
displayException ConnectionDisconnectedByHandshakeQuery = "disconnected: handshake query"
870+
displayException (ConnectionHandlerError handlerError) = "disconnected: " ++ displayException handlerError
871+
872+
mkDisconnectionException
873+
:: DisconnectionException handlerError
874+
-> Maybe handlerError
875+
-> DisconnectionException handlerError
876+
mkDisconnectionException _ (Just handlerError) = ConnectionHandlerError handlerError
877+
mkDisconnectionException err Nothing = err
878+
879+
836880
--
837881
-- Tracing
838882
--

0 commit comments

Comments
 (0)