From f3dee6865cae08032e2976ff31e977a3371f3e6d Mon Sep 17 00:00:00 2001 From: Tanguy Date: Wed, 5 Jan 2022 16:27:33 +0100 Subject: [PATCH] Chronos strict exception tracking (#652) * Enable chronos strict exception tracking ( -d:chronosStrictException ) --- .pinned | 4 ++-- examples/directchat.nim | 5 ++++- libp2p.nimble | 20 ++++++++++++++------ libp2p/connmanager.nim | 2 +- libp2p/daemon/daemonapi.nim | 18 ++++++++++++------ libp2p/errors.nim | 2 +- libp2p/nameresolving/dnsresolver.nim | 5 +++++ libp2p/protocols/ping.nim | 2 ++ libp2p/transports/tcptransport.nim | 2 +- tests/commontransport.nim | 2 +- tests/helpers.nim | 6 +++++- tests/testmultistream.nim | 18 ++++++++++-------- tests/testnoise.nim | 2 +- tests/testwstransport.nim | 21 ++++++++++++--------- 14 files changed, 71 insertions(+), 38 deletions(-) diff --git a/.pinned b/.pinned index 43951a310..a2aea2339 100644 --- a/.pinned +++ b/.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 \ No newline at end of file +zlib;https://github.com/status-im/nim-zlib@#d4e716d071eba1b5e0ffdf7949d983959e2b95dd diff --git a/examples/directchat.nim b/examples/directchat.nim index d473a9fb1..121f03142 100644 --- a/examples/directchat.nim +++ b/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() diff --git a/libp2p.nimble b/libp2p.nimble index 33be693a0..22cfce873 100644 --- a/libp2p.nimble +++ b/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") diff --git a/libp2p/connmanager.nim b/libp2p/connmanager.nim index 3f670cd40..3d769e5ee 100644 --- a/libp2p/connmanager.nim +++ b/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 diff --git a/libp2p/daemon/daemonapi.nim b/libp2p/daemon/daemonapi.nim index ff9a502aa..c082a6070 100644 --- a/libp2p/daemon/daemonapi.nim +++ b/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 diff --git a/libp2p/errors.nim b/libp2p/errors.nim index 83d41162a..3b6f3ed40 100644 --- a/libp2p/errors.nim +++ b/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: diff --git a/libp2p/nameresolving/dnsresolver.nim b/libp2p/nameresolving/dnsresolver.nim index 1bb6660cb..d224afc40 100644 --- a/libp2p/nameresolving/dnsresolver.nim +++ b/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() diff --git a/libp2p/protocols/ping.nim b/libp2p/protocols/ping.nim index 6e5e6bc78..c07b2a0d0 100644 --- a/libp2p/protocols/ping.nim +++ b/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, diff --git a/libp2p/transports/tcptransport.nim b/libp2p/transports/tcptransport.nim index 1c2fd3d40..4238c08bc 100644 --- a/libp2p/transports/tcptransport.nim +++ b/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 diff --git a/tests/commontransport.nim b/tests/commontransport.nim index 92f817ada..76aa2bbb8 100644 --- a/tests/commontransport.nim +++ b/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": diff --git a/tests/helpers.nim b/tests/helpers.nim index c336291b3..a607df68b 100644 --- a/tests/helpers.nim +++ b/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 diff --git a/tests/testmultistream.nim b/tests/testmultistream.nim index 986b837ac..5690ec0fc 100644 --- a/tests/testmultistream.nim +++ b/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, diff --git a/tests/testnoise.nim b/tests/testnoise.nim index 293dec997..8415d6f17 100644 --- a/tests/testnoise.nim +++ b/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 diff --git a/tests/testwstransport.nim b/tests/testwstransport.nim index bf8b9ce87..0e46b009e 100644 --- a/tests/testwstransport.nim +++ b/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()]