Skip to content

Commit

Permalink
rfc #23: Prevent Pony users from creating "silent failure" scenarios …
Browse files Browse the repository at this point in the history
…with their network code.

TCPConnectionNotify, UDPNotify and TCPListenNotify will all, by default, silently eat
connection failures.
  • Loading branch information
kcrimson committed Apr 24, 2017
1 parent 6631b63 commit e5bc69d
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 19 deletions.
3 changes: 3 additions & 0 deletions examples/echo/echo.pony
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,6 @@ class Server is TCPConnectionNotify

fun ref closed(conn: TCPConnection ref) =>
_out.print("server closed")

fun ref connect_failed(conn: TCPConnection ref) =>
_out.print("connect failed")
3 changes: 3 additions & 0 deletions examples/net/server.pony
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ class ServerSide is TCPConnectionNotify

fun ref closed(conn: TCPConnection ref) =>
_env.out.print("server closed")

fun ref connect_failed(conn: TCPConnection ref) =>
_env.out.print("connect failed")
22 changes: 18 additions & 4 deletions packages/net/_test.pony
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class _TestTCPExpectNotify is TCPConnectionNotify
_send(conn, "hi there")

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("client connect")
_h.fail_action("client connect failed")

fun ref connected(conn: TCPConnection ref) =>
_h.complete_action("client connect")
Expand Down Expand Up @@ -295,7 +295,7 @@ class _TestTCPWritevNotifyClient is TCPConnectionNotify
conn.writev(recover ["hello"; ", hello"] end)

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("client connect")
_h.fail_action("client connect failed")

class _TestTCPWritevNotifyServer is TCPConnectionNotify
let _h: TestHelper
Expand All @@ -318,6 +318,9 @@ class _TestTCPWritevNotifyServer is TCPConnectionNotify
end
true

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("sender connect failed")

class iso _TestTCPMute is UnitTest
"""
Test that the `mute` behavior stops us from reading incoming data. The
Expand Down Expand Up @@ -370,6 +373,10 @@ class _TestTCPMuteReceiveNotify is TCPConnectionNotify
_h.complete(false)
true

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("receiver connect failed")


class _TestTCPMuteSendNotify is TCPConnectionNotify
"""
Notifier that sends data back when it receives any. Used in conjunction with
Expand All @@ -388,7 +395,7 @@ class _TestTCPMuteSendNotify is TCPConnectionNotify
_h.complete_action("sender connected")

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("sender connected")
_h.fail_action("sender connect failed")

fun ref received(conn: TCPConnection ref, data: Array[U8] val,
times: USize): Bool
Expand Down Expand Up @@ -448,6 +455,10 @@ class _TestTCPUnmuteReceiveNotify is TCPConnectionNotify
_h.complete(true)
true

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("receiver connect failed")


class iso _TestTCPThrottle is UnitTest
"""
Test that when we experience backpressure when sending that the `throttled`
Expand Down Expand Up @@ -493,6 +504,9 @@ class _TestTCPThrottleReceiveNotify is TCPConnectionNotify
_h.complete_action("receiver asks for data")
_h.dispose_when_done(conn)

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("receiver connect failed")

class _TestTCPThrottleSendNotify is TCPConnectionNotify
"""
Notifier that sends data back when it receives any. Used in conjunction with
Expand All @@ -512,7 +526,7 @@ class _TestTCPThrottleSendNotify is TCPConnectionNotify
_h.complete_action("sender connected")

fun ref connect_failed(conn: TCPConnection ref) =>
_h.fail_action("sender connected")
_h.fail_action("sender connect failed")

fun ref received(conn: TCPConnection ref, data: Array[U8] val,
times: USize): Bool
Expand Down
6 changes: 6 additions & 0 deletions packages/net/http/_server_conn_handler.pony
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,9 @@ class _ServerConnHandler is TCPConnectionNotify
try
(_session as _ServerConnection).cancel(Payload.request())
end

fun ref connect_failed(conn: TCPConnection ref) =>
"""
The connect has failed. TODO: is it a case for server-side?
"""
None
21 changes: 15 additions & 6 deletions packages/net/tcp_connection.pony
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ actor TCPConnection
_out.print("GOT:" + String.from_array(consume data))
conn.close()
fun ref connect_failed(conn: TCPConnection ref) =>
None
actor Main
new create(env: Env) =>
try
Expand Down Expand Up @@ -84,6 +87,9 @@ actor TCPConnection
fun ref unthrottled(connection: TCPConnection ref) =>
_coordinator.unthrottled(this)
fun ref connect_failed(conn: TCPConnection ref) =>
None
actor Coordinator
var _senders: List[Any tag] = _senders.create()
Expand Down Expand Up @@ -116,18 +122,21 @@ actor TCPConnection
""
end
fun ref sentv(conn: TCPConnection ref, data: ByteSeqIter): ByteSeqIter =>
if not _throttled then
data
else
Array[String]
end
fun ref sentv(conn: TCPConnection ref, data: ByteSeqIter): ByteSeqIter =>
if not _throttled then
data
else
Array[String]
end
fun ref throttled(connection: TCPConnection ref) =>
_throttled = true
fun ref unthrottled(connection: TCPConnection ref) =>
_throttled = false
fun ref connect_failed(conn: TCPConnection ref) =>
None
```
In general, unless you have a very specific use case, we strongly advise that
Expand Down
11 changes: 9 additions & 2 deletions packages/net/tcp_connection_notify.pony
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,19 @@ interface TCPConnectionNotify
"""
None

fun ref connect_failed(conn: TCPConnection ref) =>
fun ref connect_failed(conn: TCPConnection ref)
"""
Called when we have failed to connect to all possible addresses for the
server. At this point, the connection will never be established.
It is expected to implement proper error handling. You need to opt in to
ignoring errors, which can be implemented like this:
```
fun ref connect_failed(conn: TCPConnection ref) =>
None
```
"""
None

fun ref auth_failed(conn: TCPConnection ref) =>
"""
Expand Down
11 changes: 9 additions & 2 deletions packages/net/tcp_listen_notify.pony
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,18 @@ interface TCPListenNotify
"""
None

fun ref not_listening(listen: TCPListener ref) =>
fun ref not_listening(listen: TCPListener ref)
"""
Called if it wasn't possible to bind the listener to an address.
It is expected to implement proper error handling. You need to opt in to
ignoring errors, which can be implemented like this:
```
fun ref not_listening(listen: TCPListener ref) =>
None
```
"""
None

fun ref closed(listen: TCPListener ref) =>
"""
Expand Down
9 changes: 6 additions & 3 deletions packages/net/tcp_listener.pony
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ actor TCPListener
fun ref received(conn: TCPConnection ref, data: Array[U8] iso): Bool =>
conn.write(String.from_array(consume data))
true
fun ref connect_failed(conn: TCPConnection ref) =>
None
class MyTCPListenNotify is TCPListenNotify
fun ref connected(listen: TCPListener ref): TCPConnectionNotify iso^ =>
Expand All @@ -38,7 +41,7 @@ actor TCPListener
var _max_size: USize

new create(auth: TCPListenerAuth, notify: TCPListenNotify iso,
host: String = "", service: String = "0", limit: USize = 0,
host: String = "", service: String = "0", limit: USize = 0,
init_size: USize = 64, max_size: USize = 16384)
=>
"""
Expand All @@ -54,7 +57,7 @@ actor TCPListener
_notify_listening()

new ip4(auth: TCPListenerAuth, notify: TCPListenNotify iso,
host: String = "", service: String = "0", limit: USize = 0,
host: String = "", service: String = "0", limit: USize = 0,
init_size: USize = 64, max_size: USize = 16384)
=>
"""
Expand All @@ -70,7 +73,7 @@ actor TCPListener
_notify_listening()

new ip6(auth: TCPListenerAuth, notify: TCPListenNotify iso,
host: String = "", service: String = "0", limit: USize = 0,
host: String = "", service: String = "0", limit: USize = 0,
init_size: USize = 64, max_size: USize = 16384)
=>
"""
Expand Down
11 changes: 9 additions & 2 deletions packages/net/udp_notify.pony
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,18 @@ interface UDPNotify
"""
None

fun ref not_listening(sock: UDPSocket ref) =>
fun ref not_listening(sock: UDPSocket ref)
"""
Called if it wasn't possible to bind the socket to an address.
It is expected to implement proper error handling. You need to opt in to
ignoring errors, which can be implemented like this:
```
fun ref not_listening(sock: UDPSocket ref) =>
None
```
"""
None

fun ref received(sock: UDPSocket ref, data: Array[U8] iso, from: NetAddress)
=>
Expand Down
5 changes: 5 additions & 0 deletions packages/net/udp_socket.pony
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ actor UDPSocket
=>
sock.write(consume data, from)
fun ref not_listening(sock: UDPSocket ref) =>
None
actor Main
new create(env: Env) =>
try
Expand All @@ -47,6 +50,8 @@ actor UDPSocket
=>
_out.print("GOT:" + String.from_array(consume data))
sock.dispose()
fun ref not_listening(sock: UDPSocket ref) =>
None
actor Main
new create(env: Env) =>
Expand Down

0 comments on commit e5bc69d

Please sign in to comment.