Conversation
|
The new testing framework |
| } | ||
|
|
||
| public func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) { | ||
| guard event is IdleStateHandler.IdleStateEvent else { return } |
There was a problem hiding this comment.
usually the rules are that if you don't handle a user event, you should forward it (as opposed to dropping it). This might be the last handler in the pipeline so it might be fine...
There was a problem hiding this comment.
Yes, this is the last inbound handler in the pipeline.
There was a problem hiding this comment.
Added a change to propagate events that aren't IdleStateEvents
| } | ||
| } | ||
|
|
||
| public func errorCaught(context: ChannelHandlerContext, error: Error) { |
There was a problem hiding this comment.
here I would definitely recommend that on unknown error you either:
- forward (if this is not the "application handler", ie. the last and the most high-level handler)
- log & close the
Channel(in case this is the last handler which I often call the "application handler")
There was a problem hiding this comment.
Thanks @weissi I've addressed this comment.
|
@bridger Can you test your app with these changes please? Thanks! |
|
I tried my app with this branch and with Kitura/Kitura-NIO#217. Unfortunately, it looks like I still have stale connections left. Maybe it is related to #42 ? |
Motivation: On Kitura-WebSocket Kitura/Kitura-WebSocket#48 introduces an automatic connection cleanup feature. Using this, `WebSocketService`s can configure an optional timeout value. If a connection remains idle for half of the configured timeout value, the server sends a heartbeat ping to the client. In a following time period equal to half of the configured timeout value, if a pong is received on the connection, the latter is kept alive. If not, the connection is closed. In `Kitura-WebSocket-NIO`, we implement this using the `IdleStateHandler` which is configured with half of the configured timeout value. When we receive the first `IdleStateEvent`, we send a heartbeat ping to the client. If a pong is received before the next `IdleStateEvent`, the connection is kept alive. If the next `IdleStateEvent` is received before the pong, the connection is closed. This commit also includes a new client framework for testing. It is difficult to write the connection cleanup tests using the existing adhoc WebSocket client framework implemented in `KituraTest.swift` and `KituraTest+Frames.swift`. The new client framework is called `WebSocketClient`. It uses a callback model and is largely work-in-progress. We must move all of the existing tests to the new framework in due time.
Motivation:
On Kitura-WebSocket Kitura/Kitura-WebSocket#48
introduces an automatic connection cleanup feature. Using this,
WebSocketServices can configure an optional timeout value. If a connectionremains idle for half of the configured timeout value, the server sends a
heartbeat ping to the client. In a following time period, again equal to
half of the configured timeout value, if a pong is received on the connection,
the latter is kept alive. If not, the connection is closed.
In
Kitura-WebSocket-NIO, we implement this using theIdleStateHandlerwhichis configured with half of the configured timeout value. When we receive the
first
IdleStateEvent, we send a heartbeat ping to the client. If a pong isreceived before the next
IdleStateEvent, the connection is kept alive. If thenext
IdleStateEventis received before the pong, the connection is closed.This commit also includes a new client framework for testing. It is difficult
to write the connection cleanup tests using the existing adhoc WebSocket client
framework implemented in
KituraTest.swiftandKituraTest+Frames.swift. Thenew client framework is called
WebSocketClient. It uses a callback model andis largely work-in-progress. We must move all of the existing tests to the new
framework in due time.
Note: the
IdleStateHandleris inserted into the pipeline just before theWebSocketConnectionhandler only if we have a non-nil connection timeout value configured.