Chronos strict exception tracking (#652)

* Enable chronos strict exception tracking ( -d:chronosStrictException )

.pinned

@@ -2,7 +2,7 @@ asynctest;https://github.com/markspanbroek/asynctest@#3882ed64ed3159578f796bc5ae
 bearssl;https://github.com/status-im/nim-bearssl@#ba80e2a0d7ae8aab666cee013e38ff8d33a3e5e7
 chronicles;https://github.com/status-im/nim-chronicles@#2a2681b60289aaf7895b7056f22616081eb1a882
 chronos;https://github.com/status-im/nim-chronos@#7dc58d42b6905a7fd7531875fa76060f8f744e4e
-dnsclient;https://github.com/ba0f3/dnsclient.nim@#536cc6b7933e5f86590bb27083c0ffeab31255f9
+dnsclient;https://github.com/ba0f3/dnsclient.nim@#fbb76f8af8a33ab818184a7d4406d9fee20993be
 faststreams;https://github.com/status-im/nim-faststreams@#c653d05f277dca0f374732c5b9b80f2368faea33
 httputils;https://github.com/status-im/nim-http-utils@#507bfb7dcb6244d76ce2567df7bf3756cbe88775
 json_serialization;https://github.com/status-im/nim-json-serialization@#010aa238cf6afddf1fbe4cbcd27ab3be3f443841
@@ -14,4 +14,4 @@ stew;https://github.com/status-im/nim-stew@#2f9c61f485e1de6d7e163294008276c455d3
 testutils;https://github.com/status-im/nim-testutils@#aa6e5216f4b4ab5aa971cdcdd70e1ec1203cedf2
 unittest2;https://github.com/status-im/nim-unittest2@#4e2893eacb916c7678fdc4935ff7420f13bf3a9c
 websock;https://github.com/status-im/nim-websock@#c2aae352f7fad7a8d333327c37e966969d3ee542
-zlib;https://github.com/status-im/nim-zlib@#d4e716d071eba1b5e0ffdf7949d983959e2b95dd
+zlib;https://github.com/status-im/nim-zlib@#d4e716d071eba1b5e0ffdf7949d983959e2b95dd

examples/directchat.nim

@@ -162,7 +162,10 @@ proc main() {.async.} =
     stdinReader = fromPipe(rfd)
 
   var thread: Thread[AsyncFD]
-  thread.createThread(readInput, wfd)
+  try:
+    thread.createThread(readInput, wfd)
+  except Exception as exc:
+    quit("Failed to create thread: " & exc.msg)
 
   var localAddress = MultiAddress.init(DefaultAddr).tryGet()
 

libp2p.nimble

@@ -9,7 +9,7 @@ skipDirs      = @["tests", "examples", "Nim", "tools", "scripts", "docs"]
 
 requires "nim >= 1.2.0",
          "nimcrypto >= 0.4.1",
-         "https://github.com/ba0f3/dnsclient.nim == 0.1.0",
+         "dnsclient >= 0.1.2",
          "bearssl >= 0.1.4",
          "chronicles >= 0.10.2",
          "chronos >= 3.0.6",
@@ -18,11 +18,19 @@ requires "nim >= 1.2.0",
          "stew#head",
          "websock"
 
+const nimflags =
+  "--verbosity:0 --hints:off " &
+  "--warning[CaseTransition]:off --warning[ObservableStores]:off " &
+  "--warning[LockLevel]:off " &
+  "-d:chronosStrictException " &
+  "--styleCheck:usages --styleCheck:hint "
+
 proc runTest(filename: string, verify: bool = true, sign: bool = true,
              moreoptions: string = "") =
   let env_nimflags = getEnv("NIMFLAGS")
-  var excstr = "nim c --opt:speed -d:debug -d:libp2p_agents_metrics -d:libp2p_protobuf_metrics -d:libp2p_network_protocols_metrics --verbosity:0 --hints:off --styleCheck:usages --styleCheck:hint " & env_nimflags
-  excstr.add(" --warning[CaseTransition]:off --warning[ObservableStores]:off --warning[LockLevel]:off")
+  var excstr = "nim c --opt:speed -d:debug -d:libp2p_agents_metrics -d:libp2p_protobuf_metrics -d:libp2p_network_protocols_metrics "
+  excstr.add(" " & env_nimflags & " ")
+  excstr.add(" " & nimflags & " ")
   excstr.add(" -d:libp2p_pubsub_sign=" & $sign)
   excstr.add(" -d:libp2p_pubsub_verify=" & $verify)
   excstr.add(" " & moreoptions & " ")
@@ -34,8 +42,8 @@ proc runTest(filename: string, verify: bool = true, sign: bool = true,
   rmFile "tests/" & filename.toExe
 
 proc buildSample(filename: string, run = false) =
-  var excstr = "nim c --opt:speed --threads:on -d:debug --verbosity:0 --hints:off "
-  excstr.add(" --warning[CaseTransition]:off --warning[ObservableStores]:off --warning[LockLevel]:off")
+  var excstr = "nim c --opt:speed --threads:on -d:debug "
+  excstr.add(" " & nimflags & " ")
   excstr.add(" examples/" & filename)
   exec excstr
   if run:
@@ -44,7 +52,7 @@ proc buildSample(filename: string, run = false) =
 
 proc buildTutorial(filename: string) =
   discard gorge "cat " & filename & " | nim c -r --hints:off tools/markdown_runner.nim | " &
-    " nim --warning[CaseTransition]:off --warning[ObservableStores]:off --warning[LockLevel]:off c -"
+    " nim " & nimflags & " c -"
 
 task testnative, "Runs libp2p native tests":
   runTest("testnative")

libp2p/connmanager.nim

@@ -65,7 +65,7 @@ type
         discard
 
   PeerEventHandler* =
-    proc(peerId: PeerId, event: PeerEvent): Future[void] {.gcsafe.}
+    proc(peerId: PeerId, event: PeerEvent): Future[void] {.gcsafe, raises: [Defect].}
 
   MuxerHolder = object
     muxer: Muxer

libp2p/daemon/daemonapi.nim

@@ -150,10 +150,10 @@ type
     key*: PublicKey
 
   P2PStreamCallback* = proc(api: DaemonAPI,
-                            stream: P2PStream): Future[void] {.gcsafe.}
+                            stream: P2PStream): Future[void] {.gcsafe, raises: [Defect, CatchableError].}
   P2PPubSubCallback* = proc(api: DaemonAPI,
                             ticket: PubsubTicket,
-                            message: PubSubMessage): Future[bool] {.gcsafe.}
+                            message: PubSubMessage): Future[bool] {.gcsafe, raises: [Defect, CatchableError].}
 
   DaemonError* = object of LPError
   DaemonRemoteError* = object of DaemonError
@@ -755,7 +755,13 @@ proc newDaemonApi*(flags: set[P2PDaemonFlags] = {},
 
   # Starting daemon process
   # echo "Starting ", cmd, " ", args.join(" ")
-  api.process = startProcess(cmd, "", args, env, {poParentStreams})
+  api.process = 
+    try:
+      startProcess(cmd, "", args, env, {poParentStreams})
+    except CatchableError as exc:
+      raise exc
+    except Exception as exc:
+      raiseAssert exc.msg
   # Waiting until daemon will not be bound to control socket.
   while true:
     if not api.process.running():
@@ -900,7 +906,7 @@ proc openStream*(api: DaemonAPI, peer: PeerId,
         stream.flags.incl(Outbound)
         stream.transp = transp
         result = stream
-  except Exception as exc:
+  except CatchableError as exc:
     await api.closeConnection(transp)
     raise exc
 
@@ -936,7 +942,7 @@ proc addHandler*(api: DaemonAPI, protocols: seq[string],
                                                                protocols))
     pb.withMessage() do:
       api.servers.add(P2PServer(server: server, address: maddress))
-  except Exception as exc:
+  except CatchableError as exc:
     for item in protocols:
       api.handlers.del(item)
     server.stop()
@@ -1301,7 +1307,7 @@ proc pubsubSubscribe*(api: DaemonAPI, topic: string,
       ticket.transp = transp
       asyncSpawn pubsubLoop(api, ticket)
       result = ticket
-  except Exception as exc:
+  except CatchableError as exc:
     await api.closeConnection(transp)
     raise exc
 

libp2p/errors.nim

@@ -49,7 +49,7 @@ proc allFuturesThrowing*[T](args: varargs[Future[T]]): Future[void] =
   for fut in args:
     futs &= fut
   proc call() {.async.} =
-    var first: ref Exception = nil
+    var first: ref CatchableError = nil
     futs = await allFinished(futs)
     for fut in futs:
       if fut.failed:

libp2p/nameresolving/dnsresolver.nim

@@ -78,7 +78,12 @@ proc getDnsResponse(
       dataStream = newStringStream()
     dataStream.writeData(addr rawResponse[0], rawResponse.len)
     dataStream.setPosition(0)
+    # parseResponse can has a raises: [Exception, ..] because of
+    # https://github.com/nim-lang/Nim/commit/035134de429b5d99c5607c5fae912762bebb6008
+    # it can't actually raise though
     return parseResponse(dataStream)
+  except CatchableError as exc: raise exc
+  except Exception as exc: raiseAssert exc.msg
   finally:
     await sock.closeWait()
 

libp2p/protocols/ping.nim

@@ -7,6 +7,8 @@
 ## This file may not be copied, modified, or distributed except according to
 ## those terms.
 
+{.push raises: [Defect].}
+
 import chronos, chronicles, bearssl
 import ../protobuf/minprotobuf,
        ../peerinfo,

libp2p/transports/tcptransport.nim

@@ -225,7 +225,7 @@ method accept*(self: TcpTransport): Future[Connection] {.async, gcsafe.} =
     debug "Server was closed", exc = exc.msg
     raise newTransportClosedError(exc)
   except CancelledError as exc:
-    raise
+    raise exc
   except CatchableError as exc:
     debug "Unexpected error accepting connection", exc = exc.msg
     raise exc

tests/commontransport.nim

@@ -11,7 +11,7 @@ import ../libp2p/[stream/connection,
 
 import ./helpers
 
-type TransportProvider* = proc(): Transport {.gcsafe.}
+type TransportProvider* = proc(): Transport {.gcsafe, raises: [Defect].}
 
 proc commonTransportTest*(name: string, prov: TransportProvider, ma: string) =
   suite name & " common tests":

tests/helpers.nim

@@ -13,6 +13,8 @@ import ../libp2p/protocols/secure/secure
 import ./asyncunit
 export asyncunit
 
+{.push raises: [Defect].}
+
 const
   StreamTransportTrackerName = "stream.transport"
   StreamServerTrackerName = "stream.server"
@@ -51,7 +53,9 @@ template checkTrackers*() =
       checkpoint tracker.dump()
       fail()
   # Also test the GC is not fooling with us
-  GC_fullCollect()
+  try:
+    GC_fullCollect()
+  except: discard
 
 type RngWrap = object
   rng: ref BrHmacDrbgContext

tests/testmultistream.nim

@@ -10,6 +10,9 @@ import ../libp2p/errors,
        ../libp2p/protocols/protocol,
        ../libp2p/upgrademngrs/upgrade
 
+
+{.push raises: [Defect].}
+
 import ./helpers
 
 when defined(nimHasUsed): {.used.}
@@ -53,7 +56,7 @@ method readOnce*(s: TestSelectStream,
 
 method write*(s: TestSelectStream, msg: seq[byte]) {.async, gcsafe.} = discard
 
-method close(s: TestSelectStream) {.async, gcsafe.} =
+method close(s: TestSelectStream) {.async, gcsafe, raises: [Defect].} =
   s.isClosed = true
   s.isEof = true
 
@@ -63,7 +66,7 @@ proc newTestSelectStream(): TestSelectStream =
 
 ## Mock stream for handles `ls` test
 type
-  LsHandler = proc(procs: seq[byte]): Future[void] {.gcsafe.}
+  LsHandler = proc(procs: seq[byte]): Future[void] {.gcsafe, raises: [Defect].}
 
   TestLsStream = ref object of Connection
     step*: int
@@ -115,7 +118,7 @@ proc newTestLsStream(ls: LsHandler): TestLsStream {.gcsafe.} =
 
 ## Mock stream for handles `na` test
 type
-  NaHandler = proc(procs: string): Future[void] {.gcsafe.}
+  NaHandler = proc(procs: string): Future[void] {.gcsafe, raises: [Defect].}
 
   TestNaStream = ref object of Connection
     step*: int
@@ -195,14 +198,14 @@ suite "Multistream select":
   asyncTest "test handle `ls`":
     let ms = MultistreamSelect.new()
 
-    proc testLsHandler(proto: seq[byte]) {.async, gcsafe.} # forward declaration
-    let conn = Connection(newTestLsStream(testLsHandler))
+    var conn: Connection = nil
     let done = newFuture[void]()
     proc testLsHandler(proto: seq[byte]) {.async, gcsafe.} =
       var strProto: string = string.fromBytes(proto)
       check strProto == "\x26/test/proto1/1.0.0\n/test/proto2/1.0.0\n"
       await conn.close()
       done.complete()
+    conn = Connection(newTestLsStream(testLsHandler))
 
     proc testHandler(conn: Connection, proto: string): Future[void]
       {.async, gcsafe.} = discard
@@ -216,13 +219,12 @@ suite "Multistream select":
   asyncTest "test handle `na`":
     let ms = MultistreamSelect.new()
 
-    proc testNaHandler(msg: string): Future[void] {.async, gcsafe.}
-    let conn = newTestNaStream(testNaHandler)
-
+    var conn: Connection = nil
     proc testNaHandler(msg: string): Future[void] {.async, gcsafe.} =
       echo msg
       check msg == Na
       await conn.close()
+    conn = newTestNaStream(testNaHandler)
 
     var protocol: LPProtocol = new LPProtocol
     proc testHandler(conn: Connection,

tests/testnoise.nim

@@ -40,7 +40,7 @@ const
 type
   TestProto = ref object of LPProtocol
 
-method init(p: TestProto) {.gcsafe.} =
+method init(p: TestProto) {.gcsafe, raises: [Defect].} =
   proc handle(conn: Connection, proto: string) {.async, gcsafe.} =
     let msg = string.fromBytes(await conn.readLp(1024))
     check "Hello!" == msg

tests/testwstransport.nim

@@ -13,7 +13,7 @@ import ../libp2p/[stream/connection,
 import ./helpers, ./commontransport
 
 const
-  SecureKey* = """
+  SecureKey = """
 -----BEGIN PRIVATE KEY-----
 MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAP0yH7F7FtGunC91
 IPkU+u8B4gdxiwYW0J3PrixtB1Xz3e4dfjwQqhIJlG6BxQ4myCxmSPjxP/eOOYp+
@@ -32,7 +32,7 @@ NABr5ec1FxuJa/8=
 -----END PRIVATE KEY-----
 """
 
-  SecureCert* = """
+  SecureCert = """
 -----BEGIN CERTIFICATE-----
 MIICjDCCAfWgAwIBAgIURjeiJmkNbBVktqXvnXh44DKx364wDQYJKoZIhvcNAQEL
 BQAwVzELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
@@ -62,13 +62,16 @@ suite "WebSocket transport":
 
   commonTransportTest(
     "WebSocket Secure",
-    proc (): Transport =
-      WsTransport.new(
-        Upgrade(),
-        TLSPrivateKey.init(SecureKey),
-        TLSCertificate.init(SecureCert),
-        {TLSFlags.NoVerifyHost, TLSFlags.NoVerifyServerName}),
-      "/ip4/0.0.0.0/tcp/0/wss")
+    (proc (): Transport {.gcsafe.} =
+      try:
+        return WsTransport.new(
+          Upgrade(),
+          TLSPrivateKey.init(SecureKey),
+          TLSCertificate.init(SecureCert),
+          {TLSFlags.NoVerifyHost, TLSFlags.NoVerifyServerName})
+      except Exception: check(false)
+    ),
+    "/ip4/0.0.0.0/tcp/0/wss")
 
   asyncTest "Hostname verification":
     let ma = @[MultiAddress.init("/ip4/0.0.0.0/tcp/0/wss").tryGet()]