rpc/handler: do not append null to stream when json may be valid (#10383

)

fixes #10376
fixes #10378

let's wait for user feedback before merging

rpc/handler.go

@@ -578,9 +578,28 @@ func writeNilIfNotPresent(stream *jsoniter.Stream) {
 	} else {
 		hasNil = false
 	}
-	if !hasNil {
-		stream.WriteNil()
+	if hasNil {
+		// not needed
+		return
+	}
+
+	var validJsonEnd bool
+	if len(b) > 0 {
+		// assumption is that api call handlers would write valid json in case of errors
+		// we are not guaranteed that they did write valid json if last elem is "}" or "]"
+		// since we don't check json nested-ness
+		// however appending "null" after "}" or "]" does not help much either
+		lastIdx := len(b) - 1
+		validJsonEnd = b[lastIdx] == '}' || b[lastIdx] == ']'
 	}
+	if validJsonEnd {
+		// not needed
+		return
+	}
+
+	// does not have nil ending
+	// does not have valid json
+	stream.WriteNil()
 }
 
 // unsubscribe is the callback function for all *_unsubscribe calls.

rpc/handler_test.go

@@ -29,6 +29,10 @@ func TestHandlerDoesNotDoubleWriteNull(t *testing.T) {
 			params:   []byte("[3]"),
 			expected: `{"jsonrpc":"2.0","id":1,"result":{}}`,
 		},
+		"err_with_valid_json": {
+			params:   []byte("[4]"),
+			expected: `{"jsonrpc":"2.0","id":1,"result":{"structLogs":[]},"error":{"code":-32000,"message":"id 4"}}`,
+		},
 	}
 
 	for name, testParams := range tests {
@@ -50,7 +54,17 @@ func TestHandlerDoesNotDoubleWriteNull(t *testing.T) {
 				if id == 2 {
 					return fmt.Errorf("id 2")
 				}
-				stream.WriteEmptyObject()
+				if id == 3 {
+					stream.WriteEmptyObject()
+					return nil
+				}
+				if id == 4 {
+					stream.WriteObjectStart()
+					stream.WriteObjectField("structLogs")
+					stream.WriteEmptyArray()
+					stream.WriteObjectEnd()
+					return fmt.Errorf("id 4")
+				}
 				return nil
 			}