Improve both client and server resilience against fields and elements with null value (#195)

* Improve resilience against null fields

* Fix client processMessage when handling error

* Improve both client and server resilience against fields and elements with null value

json_rpc/client.nim

@@ -89,16 +89,20 @@ proc processMessage*(client: RpcClient, line: string): Result[void, string] =
     var requestFut: Future[JsonString]
     let id = response.id.get
     if not client.awaiting.pop(id, requestFut):
-      return err("Cannot find message id \"" & $id & "\"")
-
+      let msg = "Cannot find message id \"" & $id & "\":"
+      requestFut.fail(newException(JsonRpcError, msg))
+      return ok()
+  
     if response.error.isSome:
       let error = JrpcSys.encode(response.error.get)
       requestFut.fail(newException(JsonRpcError, error))
       return ok()
 
     # Up to this point, the result should contains something    
-    if response.result.string.len == 0:
-      return err("missing or invalid response result")
+    if response.result.string.len == 0:      
+      let msg = "missing or invalid response result" 
+      requestFut.fail(newException(JsonRpcError, msg))
+      return ok()
 
     requestFut.complete(response.result)
     return ok()

json_rpc/jsonmarshal.nim

@@ -14,7 +14,11 @@ export
   json_serialization
 
 createJsonFlavor JrpcConv,
-  requireAllFields = false
-
+  automaticObjectSerialization = false,
+  requireAllFields = false,
+  omitOptionalFields = true, # Skip optional fields==none in Writer
+  allowUnknownFields = true,
+  skipNullFields = true      # Skip optional fields==null in Reader
+                           
 # JrpcConv is a namespace/flavor for encoding and decoding
 # parameters and return value of a rpc method.

json_rpc/private/jrpc_sys.nim

@@ -141,11 +141,14 @@ type
 # don't mix the json-rpc system encoding with the
 # actual response/params encoding
 createJsonFlavor JrpcSys,
-  requireAllFields = false
+  automaticObjectSerialization = false,
+  requireAllFields = false,
+  omitOptionalFields = true, # Skip optional fields==none in Writer
+  allowUnknownFields = true,
+  skipNullFields = true      # Skip optional fields==null in Reader
 
 ResponseError.useDefaultSerializationIn JrpcSys
 RequestTx.useDefaultWriterIn JrpcSys
-ResponseRx.useDefaultReaderIn JrpcSys
 RequestRx.useDefaultReaderIn JrpcSys
 
 const
@@ -253,6 +256,18 @@ proc writeValue*(w: var JsonWriter[JrpcSys], val: ResponseTx)
     w.writeField("error", val.error)
   w.endRecord()
 
+proc readValue*(r: var JsonReader[JrpcSys], val: var ResponseRx)
+       {.gcsafe, raises: [IOError, SerializationError].} =
+  # We need to overload ResponseRx reader because
+  # we don't want to skip null fields
+  r.parseObjectWithoutSkip(key):
+    case key
+    of "jsonrpc": r.readValue(val.jsonrpc)
+    of "id"     : r.readValue(val.id)
+    of "result" : val.result = r.parseAsString()
+    of "error"  : r.readValue(val.error)
+    else: discard
+
 proc writeValue*(w: var JsonWriter[JrpcSys], val: RequestBatchTx)
        {.gcsafe, raises: [IOError].} =
   if val.kind == rbkMany:

json_rpc/private/server_handler_wrapper.nim

@@ -149,11 +149,10 @@ proc setupPositional(code: NimNode;
       `paramsIdent`.positional[`pos`].kind
     paramVar = quote do:
       `paramsObj`.`paramIdent`
+    innerNode = jsonToNim(paramVar, paramType, paramVal, paramName)
 
   # e.g. (A: int, B: Option[int], C: string, D: Option[int], E: Option[string])
   if paramType.isOptionalArg:
-    let
-      innerNode = jsonToNim(paramVar, paramType, paramVal, paramName)
     if pos >= minLength:
       # allow both empty and null after mandatory args
       # D & E fall into this category
@@ -171,7 +170,9 @@ proc setupPositional(code: NimNode;
     # mandatory args
     # A and C fall into this category
     # unpack Nim type and assign from json
-    code.add jsonToNim(paramVar, paramType, paramVal, paramName)
+    code.add quote do:
+      if `paramKind` != JsonValueKind.Null:
+        `innerNode`
 
 proc makeParams(retType: NimNode, params: NimNode): seq[NimNode] =
   ## Convert rpc params into handler params

json_rpc/server.nim

@@ -64,6 +64,19 @@ proc executeMethod*(server: RpcServer,
   let params = paramsTx(args)
   server.executeMethod(methodName, params)
 
+proc executeMethod*(server: RpcServer,
+                    methodName: string,
+                    args: JsonString): Future[JsonString]
+                      {.gcsafe, raises: [JsonRpcError].} =
+
+  let params = try:
+    let x = JrpcSys.decode(args.string, RequestParamsRx)
+    x.toTx
+  except SerializationError as exc:
+    raise newException(JsonRpcError, exc.msg)
+
+  server.executeMethod(methodName, params)
+
 # Wrapper for message processing
 
 proc route*(server: RpcServer, line: string): Future[string] {.gcsafe.} =

tests/test_jrpc_sys.nim

@@ -244,3 +244,12 @@ suite "jrpc_sys conversion":
     check:
       rx.kind == rbkMany
       rx.many.len == 3
+
+  test "skip null value":
+    let jsonBytes = """{"jsonrpc":null, "id":null, "method":null, "params":null}"""
+    let x = JrpcSys.decode(jsonBytes, RequestRx)
+    check:
+      x.jsonrpc.isNone
+      x.id.kind == riNull
+      x.`method`.isNone
+      x.params.kind == rpPositional

tests/testrpcmacro.nim

@@ -39,11 +39,16 @@ type
     Enum0
     Enum1
 
+  MuscleCar = object
+    color: string
+    wheel: int
+
 MyObject.useDefaultSerializationIn JrpcConv
 Test.useDefaultSerializationIn JrpcConv
 Test2.useDefaultSerializationIn JrpcConv
 MyOptional.useDefaultSerializationIn JrpcConv
 MyOptionalNotBuiltin.useDefaultSerializationIn JrpcConv
+MuscleCar.useDefaultSerializationIn JrpcConv
 
 proc readValue*(r: var JsonReader[JrpcConv], val: var MyEnum)
        {.gcsafe, raises: [IOError, SerializationError].} =
@@ -118,6 +123,9 @@ s.rpc("rpc.optionalArg2") do(a, b: string, c, d: Option[string]) -> string:
   if d.isSome: ret.add d.get()
   return ret
 
+s.rpc("echo") do(car: MuscleCar) -> JsonString:
+  return JrpcConv.encode(car).JsonString
+
 type
   OptionalFields = object
     a: int
@@ -345,12 +353,22 @@ suite "Server types":
       r1 = waitFor s.executeMethod("rpc.optionalStringArg", %[%data])
       r2 = waitFor s.executeMethod("rpc.optionalStringArg", %[])
       r3 = waitFor s.executeMethod("rpc.optionalStringArg", %[newJNull()])
-    echo r1
-    echo r2
-    echo r3
     check r1 == %data.get()
     check r2 == %"nope"
     check r3 == %"nope"
 
+  test "Null object fields":
+    let r = waitFor s.executeMethod("echo", """{"car":{"color":"red","wheel":null}}""".JsonString)
+    check r == """{"color":"red","wheel":0}"""
+
+    let x = waitFor s.executeMethod("echo", """{"car":{"color":null,"wheel":77}}""".JsonString)
+    check x == """{"color":"","wheel":77}"""
+
+    let y = waitFor s.executeMethod("echo", """{"car":null}""".JsonString)
+    check y == """{"color":"","wheel":0}"""
+
+    let z = waitFor s.executeMethod("echo", "[null]".JsonString)
+    check z == """{"color":"","wheel":0}"""
+
 s.stop()
 waitFor s.closeWait()