fix enum parsing, work around potential nil dereference (#150)

* fix enum parsing, work around potential `nil` dereference

When the bizarre error handling in the http client fails, it may happen
that a nil is returned (maybe due to Nim bugs) - do it manually for now

json_rpc/client.nim

@@ -160,7 +160,10 @@ proc createRpcFromSig*(clientType, rpcDecl: NimNode): NimNode =
   else:
     # native json expected so no work
     callBody.add quote do:
-      `procRes` = `rpcResult`
+      `procRes` = if `rpcResult` == nil:
+        newJNull()
+      else:
+        `rpcResult`
 
   when defined(nimDumpRpcs):
     echo pathStr, ":\n", result.repr

json_rpc/clients/httpclient.nim

@@ -69,22 +69,19 @@ method call*(client: RpcHttpClient, name: string,
   var req: HttpClientRequestRef
   var res: HttpClientResponseRef
 
-  defer:
-    # BEWARE!
-    # Using multiple defer statements in this function or multiple
-    # try/except blocks within a single defer statement doesn't
-    # produce the desired run-time code, so we use slightly bizzare
-    # code to ensure the exceptions safety of this function:
-    try:
-      var closeFutures = newSeq[Future[void]]()
-      if req != nil: closeFutures.add req.closeWait()
-      if res != nil: closeFutures.add res.closeWait()
-      if closeFutures.len > 0: await allFutures(closeFutures)
-    except CatchableError as err:
-      # TODO
-      # `close` functions shouldn't raise in general, but we first
-      # need to ensure this through exception tracking in Chronos
-      debug "Error closing JSON-RPC HTTP resuest/response", err = err.msg
+  template closeRefs() =
+    # We can't trust try/finally in async/await in all nim versions, so we
+    # do it manually instead
+    if req != nil:
+      try:
+        await req.closeWait()
+      except CatchableError as exc: # shouldn't happen
+        debug "Error closing JSON-RPC HTTP resuest/response", err = exc.msg
+    if res != nil:
+      try:
+        await res.closeWait()
+      except CatchableError as exc: # shouldn't happen
+        debug "Error closing JSON-RPC HTTP resuest/response", err = exc.msg
 
   req = HttpClientRequestRef.post(client.httpSession,
                                   client.httpAddress.get,
@@ -94,11 +91,14 @@ method call*(client: RpcHttpClient, name: string,
     try:
       await req.send()
     except CancelledError as e:
+      closeRefs()
       raise e
     except CatchableError as e:
-      raise (ref RpcPostError)(msg: "Failed to send POST Request with JSON-RPC.", parent: e)
+      closeRefs()
+      raise (ref RpcPostError)(msg: "Failed to send POST Request with JSON-RPC: " & e.msg, parent: e)
 
   if res.status < 200 or res.status >= 300: # res.status is not 2xx (success)
+    closeRefs()
     raise newException(ErrorResponse, "POST Response: " & $res.status)
 
   debug "Message sent to RPC server",
@@ -109,9 +109,11 @@ method call*(client: RpcHttpClient, name: string,
     try:
       await res.getBodyBytes(client.maxBodySize)
     except CancelledError as e:
+      closeRefs()
       raise e
-    except CatchableError as exc:
-      raise (ref FailedHttpResponse)(msg: "Failed to read POST Response for JSON-RPC.", parent: exc)
+    except CatchableError as e:
+      closeRefs()
+      raise (ref FailedHttpResponse)(msg: "Failed to read POST Response for JSON-RPC: " & e.msg, parent: e)
 
   let resText = string.fromBytes(resBytes)
   trace "Response", text = resText
@@ -125,9 +127,15 @@ method call*(client: RpcHttpClient, name: string,
   try:
     # Might raise for all kinds of reasons
     client.processMessage(resText)
-  finally:
+  except CatchableError as e:
     # Need to clean up in case the answer was invalid
     client.awaiting.del(id)
+    closeRefs()
+    raise e
+
+  client.awaiting.del(id)
+
+  closeRefs()
 
   # processMessage should have completed this future - if it didn't, `read` will
   # raise, which is reasonable

json_rpc/jsonmarshal.nim

@@ -1,6 +1,6 @@
 import
   std/[macros, json, options, typetraits],
-  stew/byteutils
+  stew/[byteutils, objects]
 
 export json, options
 
@@ -33,7 +33,11 @@ proc fromJson*[T](n: JsonNode, argName: string, result: var Option[T])
 # This can't be forward declared: https://github.com/nim-lang/Nim/issues/7868
 proc fromJson*[T: enum](n: JsonNode, argName: string, result: var T) =
   n.kind.expect(JInt, argName)
-  result = n.getBiggestInt().T
+
+  let v = n.getBiggestInt()
+  if not checkedEnumAssign(result, v):
+    raise (ref ValueError)(
+      msg: "Unknown enum ordinal for " & name(T) & ": " & $v)
 
 # This can't be forward declared: https://github.com/nim-lang/Nim/issues/7868
 proc fromJson*[T: object|tuple](n: JsonNode, argName: string, result: var T) =
@@ -145,6 +149,8 @@ proc fromJson*[N, T](n: JsonNode, argName: string, result: var array[N, T]) =
 
 proc unpackArg[T](args: JsonNode, argName: string, argtype: typedesc[T]): T =
   mixin fromJson
+  if args == nil:
+    raise (ref ValueError)(msg: argName & ": unexpected null value")
   fromJson(args, argName, result)
 
 proc expectArrayLen(node, jsonIdent: NimNode, length: int) =

json_rpc/router.nim

@@ -174,4 +174,4 @@ macro rpc*(server: RpcRouter, path: string, body: untyped): untyped =
     `server`.register(`path`, `rpcProcWrapper`)
 
   when defined(nimDumpRpcs):
-    echo "\n", pathStr, ": ", result.repr
+    echo "\n", path, ": ", result.repr

tests/testrpcmacro.nim

@@ -22,6 +22,10 @@ type
   MyOptionalNotBuiltin = object
     val: Option[Test2]
 
+  MyEnum = enum
+    Enum0
+    Enum1
+
 let
   testObj = %*{
     "a": %1,
@@ -40,6 +44,9 @@ var s = newRpcSocketServer(["localhost:8545"])
 s.rpc("rpc.simplePath"):
   return %1
 
+s.rpc("rpc.enumParam") do(e: MyEnum):
+  return %[$e]
+
 s.rpc("rpc.differentParams") do(a: int, b: string):
   return %[%a, %b]
 
@@ -146,6 +153,14 @@ suite "Server types":
     let r = waitFor s.executeMethod("rpc.simplePath", %[])
     check r == "1"
 
+  test "Enum param paths":
+    block:
+      let r = waitFor s.executeMethod("rpc.enumParam", %[(int64(Enum1))])
+      check r == "[\"Enum1\"]"
+
+    expect(ValueError):
+      discard waitFor s.executeMethod("rpc.enumParam", %[(int64(42))])
+
   test "Different param types":
     let
       inp = %[%1, %"abc"]