More raises cleanup (#575)

* use toException to map errors

* don't initialize address twice

* better error messages

* allow LPError to escape

* allow LPError to escape

libp2p/builders.nim

@@ -50,7 +50,7 @@ proc new*(T: type[SwitchBuilder]): T =
 
   let address = MultiAddress
   .init("/ip4/127.0.0.1/tcp/0")
-  .expect("address should initialize to default")
+  .expect("Should initialize to default")
 
   SwitchBuilder(
     privKey: none(PrivateKey),
@@ -123,13 +123,15 @@ proc withAgentVersion*(b: SwitchBuilder, agentVersion: string): SwitchBuilder =
   b.agentVersion = agentVersion
   b
 
-proc build*(b: SwitchBuilder): Switch =
+proc build*(b: SwitchBuilder): Switch
+  {.raises: [Defect, LPError].} =
+
   if b.rng == nil: # newRng could fail
     raise newException(Defect, "Cannot initialize RNG")
 
   let pkRes = PrivateKey.random(b.rng[])
   let
-    seckey = b.privKey.get(otherwise = pkRes.expect("Should supply a valid RNG"))
+    seckey = b.privKey.get(otherwise = pkRes.expect("Expected default Private Key"))
 
   var
     secureManagerInstances: seq[Secure]
@@ -183,7 +185,7 @@ proc build*(b: SwitchBuilder): Switch =
   return switch
 
 proc newStandardSwitch*(privKey = none(PrivateKey),
-                        address = MultiAddress.init("/ip4/127.0.0.1/tcp/0").tryGet(),
+                        address = MultiAddress.init("/ip4/127.0.0.1/tcp/0"),
                         secureManagers: openarray[SecureProtocol] = [
                             SecureProtocol.Noise,
                           ],
@@ -194,13 +196,14 @@ proc newStandardSwitch*(privKey = none(PrivateKey),
                         maxConnections = MaxConnections,
                         maxIn = -1,
                         maxOut = -1,
-                        maxConnsPerPeer = MaxConnectionsPerPeer): Switch =
+                        maxConnsPerPeer = MaxConnectionsPerPeer): Switch
+                        {.raises: [Defect, LPError].} =
   if SecureProtocol.Secio in secureManagers:
       quit("Secio is deprecated!") # use of secio is unsafe
 
   var b = SwitchBuilder
     .new()
-    .withAddress(address)
+    .withAddress(address.expect("Should have been initialized with default"))
     .withRng(rng)
     .withMaxConnections(maxConnections)
     .withMaxIn(maxIn)

libp2p/daemon/daemonapi.nim

@@ -11,12 +11,13 @@
 
 ## This module implementes API for `go-libp2p-daemon`.
 import std/[os, osproc, strutils, tables, strtabs]
-import chronos, chronicles
+import pkg/[chronos, chronicles]
 import ../varint, ../multiaddress, ../multicodec, ../cid, ../peerid
 import ../wire, ../multihash, ../protobuf/minprotobuf
 import ../crypto/crypto
 
-export peerid, multiaddress, multicodec, multihash, cid, crypto, wire
+export
+  peerid, multiaddress, multicodec, multihash, cid, crypto, wire
 
 when not defined(windows):
   import posix
@@ -154,6 +155,9 @@ type
                             ticket: PubsubTicket,
                             message: PubSubMessage): Future[bool] {.gcsafe.}
 
+  # TODO: would be nice to be able to map other errors to
+  # this types with `Result.toException`, but it doesn't work
+  # in this module
   DaemonRemoteError* = object of CatchableError
   DaemonLocalError* = object of CatchableError
 
@@ -833,15 +837,19 @@ proc getPeerInfo(pb: var ProtoBuffer): PeerInfo
   result.addresses = newSeq[MultiAddress]()
   if pb.getValue(1, result.peer) == -1:
     raise newException(DaemonLocalError, "Missing required field `peer`!")
+
   var address = newSeq[byte]()
   while pb.getBytes(2, address) != -1:
     if len(address) != 0:
       var copyaddr = address
       let addrRes = MultiAddress.init(copyaddr)
+
+      # TODO: for some reason `toException` doesn't
+      # work for this module
       if addrRes.isErr:
         raise newException(DaemonLocalError, addrRes.error)
 
-      result.addresses.add(MultiAddress.init(copyaddr).get())
+      result.addresses.add(addrRes.get())
       address.setLen(0)
 
 proc identity*(api: DaemonAPI): Future[PeerInfo] {.async.} =

libp2p/peerinfo.nim

@@ -9,11 +9,11 @@
 
 {.push raises: [Defect].}
 
-import options, sequtils, hashes
+import std/[options, sequtils, hashes]
-import chronos, chronicles
+import pkg/[chronos, chronicles, stew/results]
-import peerid, multiaddress, crypto/crypto
+import peerid, multiaddress, crypto/crypto, errors
 
-export peerid, multiaddress, crypto
+export peerid, multiaddress, crypto, errors, results
 
 ## A peer can be constructed in one of tree ways:
 ## 1) A local peer with a private key
@@ -28,6 +28,8 @@ type
     HasPrivate,
     HasPublic
 
+  PeerInfoError* = LPError
+
   PeerInfo* = ref object of RootObj
     peerId*: PeerID
     addrs*: seq[MultiAddress]
@@ -51,6 +53,12 @@ func shortLog*(p: PeerInfo): auto =
   )
 chronicles.formatIt(PeerInfo): shortLog(it)
 
+func toException*(e: string): ref PeerInfoError =
+  (ref PeerInfoError)(msg: e)
+
+func toException*(e: cstring): ref PeerInfoError =
+  (ref PeerInfoError)(msg: $e)
+
 template postInit(peerinfo: PeerInfo,
                   addrs: openarray[MultiAddress],
                   protocols: openarray[string]) =
@@ -65,10 +73,12 @@ proc init*(
   addrs: openarray[MultiAddress] = [],
   protocols: openarray[string] = [],
   protoVersion: string = "",
-  agentVersion: string = ""): PeerInfo =
+  agentVersion: string = ""): PeerInfo
+  {.raises: [Defect, PeerInfoError].} =
+
   let peerInfo = PeerInfo(
     keyType: HasPrivate,
-    peerId: PeerID.init(key).expect("Unable to create peer id from key"),
+    peerId: PeerID.init(key).tryGet(),
     privateKey: key,
     protoVersion: protoVersion,
     agentVersion: agentVersion)
@@ -98,11 +108,12 @@ proc init*(
   addrs: openarray[MultiAddress] = [],
   protocols: openarray[string] = [],
   protoVersion: string = "",
-  agentVersion: string = ""): PeerInfo =
+  agentVersion: string = ""): PeerInfo
+  {.raises: [Defect, PeerInfoError].} =
 
   let peerInfo = PeerInfo(
     keyType: HasPublic,
-    peerId: PeerID.init(peerId).expect("Unable to create peer id from string"),
+    peerId: PeerID.init(peerId).tryGet(),
     protoVersion: protoVersion,
     agentVersion: agentVersion)
 
@@ -115,11 +126,12 @@ proc init*(
   addrs: openarray[MultiAddress] = [],
   protocols: openarray[string] = [],
   protoVersion: string = "",
-  agentVersion: string = ""): PeerInfo =
+  agentVersion: string = ""): PeerInfo
+  {.raises: [Defect, PeerInfoError].} =
 
   let peerInfo = PeerInfo(
     keyType: HasPublic,
-    peerId: PeerID.init(key).expect("Unable to create peer id from public key"),
+    peerId: PeerID.init(key).tryGet(),
     key: some(key),
     protoVersion: protoVersion,
     agentVersion: agentVersion)

libp2p/protocols/secure/noise.nim

@@ -189,7 +189,7 @@ proc mixKey(ss: var SymmetricState, ikm: ChaChaPolyKey) =
   ss.cs = CipherState(k: temp_keys[1])
   trace "mixKey", key = ss.cs.k.shortLog
 
-proc mixHash(ss: var SymmetricState; data: openArray[byte]) =
+proc mixHash(ss: var SymmetricState, data: openArray[byte]) =
   var ctx: sha256
   ctx.init()
   ctx.update(ss.h.data)
@@ -198,7 +198,7 @@ proc mixHash(ss: var SymmetricState; data: openArray[byte]) =
   trace "mixHash", hash = ss.h.data.shortLog
 
 # We might use this for other handshake patterns/tokens
-proc mixKeyAndHash(ss: var SymmetricState; ikm: openArray[byte]) {.used.} =
+proc mixKeyAndHash(ss: var SymmetricState, ikm: openArray[byte]) {.used.} =
   var
     temp_keys: array[3, ChaChaPolyKey]
   sha256.hkdf(ss.ck, ikm, [], temp_keys)
@@ -597,14 +597,16 @@ method init*(p: Noise) {.gcsafe.} =
 
 proc newNoise*(
     rng: ref BrHmacDrbgContext,
-    privateKey: PrivateKey;
+    privateKey: PrivateKey,
-    outgoing: bool = true;
+    outgoing: bool = true,
     commonPrologue: seq[byte] = @[]): Noise =
 
-  let pkBytes = privateKey.getKey()
+  let pkBytes = privateKey
-  .expect("Expected valid private key")
+  .getKey()
+  .expect("Expected valid Private Key")
   .getBytes()
-  .expect("Couldn't get key bytes")
+  .expect("Couldn't get Private Key bytes")
+
   var noise = Noise(
     rng: rng,
     outgoing: outgoing,

libp2p/protocols/secure/secio.nim

@@ -259,7 +259,7 @@ proc newSecioConn(conn: Connection,
                   secrets: Secret,
                   order: int,
                   remotePubKey: PublicKey): SecioConn
-                  {.raises: [Defect, SecioError].} =
+                  {.raises: [Defect, LPError].} =
   ## Create new secure stream/lpstream, using specified hash algorithm ``hash``,
   ## cipher algorithm ``cipher``, stretched keys ``secrets`` and order
   ## ``order``.