Merge pull request #167 from zbud-msft/cherry-pick-fix-panic-202305

It is possible that in some edge cases, json.Marshal is unable to serialize map to JSON and panics. I am adding some additional logging at a higher log level and the ability for the function to recover from the panic with a deferred recover function.

Add deferred recover function when JSON serialization is done and drop the query when unable to provide a JSON to gnmi TypedValue. Add additional logging to give more context of state of map as well as data retrieved from Redis.

UT

gnmi_server/server_test.go

@@ -3467,6 +3467,24 @@ func TestTableData2MsiUseKey(t *testing.T) {
     }
 }
 
+func TestRecoverFromJSONSerializationPanic(t *testing.T) {
+    panicMarshal := func(v interface{}) ([]byte, error) {
+        panic("json.Marshal panics and is unable to serialize JSON")
+    }
+    mock := gomonkey.ApplyFunc(json.Marshal, panicMarshal)
+    defer mock.Reset()
+
+    tblPath := sdc.CreateTablePath("STATE_DB", "NEIGH_STATE_TABLE", "|", "10.0.0.57")
+    msi := make(map[string]interface{})
+    sdc.TableData2Msi(&tblPath, true, nil, &msi)
+
+    typedValue, err := sdc.Msi2TypedValue(msi)
+    if typedValue != nil && err != nil {
+        t.Errorf("Test should recover from panic and have nil TypedValue/Error after attempting JSON serialization")
+    }
+
+}
+
 func TestGnmiSetBatch(t *testing.T) {
 	mockCode := 
 `

sonic_data_client/db_client.go

@@ -303,6 +303,7 @@ func (c *DbClient) PollRun(q *queue.PriorityQueue, poll chan struct{}, w *sync.W
 		for gnmiPath, tblPaths := range c.pathG2S {
 			val, err := tableData2TypedValue(tblPaths, nil)
 			if err != nil {
+				log.V(2).Infof("Unable to create gnmi TypedValue due to err: %v", err)
 				return
 			}
 
@@ -688,6 +689,12 @@ func makeJSON_redis(msi *map[string]interface{}, key *string, op *string, mfv ma
 // emitJSON marshalls map[string]interface{} to JSON byte stream.
 func emitJSON(v *map[string]interface{}) ([]byte, error) {
 	//j, err := json.MarshalIndent(*v, "", indentString)
+	defer func() {
+		if r := recover(); r != nil {
+			log.V(2).Infof("Recovered from panic: %v", r)
+			log.V(2).Infof("Current state of map to be serialized is: %v", *v)
+		}
+	}()
 	j, err := json.Marshal(*v)
 	if err != nil {
 		return nil, fmt.Errorf("JSON marshalling error: %v", err)
@@ -726,9 +733,12 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
 		dbkeys = []string{tblPath.tableName + tblPath.delimitor + tblPath.tableKey}
 	}
 
+	log.V(4).Infof("dbkeys to be pulled from redis %v", dbkeys)
+
 	// Asked to use jsonField and jsonTableKey in the final json value
 	if tblPath.jsonField != "" && tblPath.jsonTableKey != "" {
 		val, err := redisDb.HGet(dbkeys[0], tblPath.field).Result()
+		log.V(4).Infof("Data pulled for key %s and field %s: %s", dbkeys[0], tblPath.field, val)
 		if err != nil {
 			log.V(3).Infof("redis HGet failed for %v %v", tblPath, err)
 			// ignore non-existing field which was derived from virtual path
@@ -746,7 +756,7 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
 			log.V(2).Infof("redis HGetAll failed for  %v, dbkey %s", tblPath, dbkey)
 			return err
 		}
-
+		log.V(4).Infof("Data pulled for dbkey %s: %v", dbkey, fv)
 		if tblPath.jsonTableKey != "" { // If jsonTableKey was prepared, use it
 			err = makeJSON_redis(msi, &tblPath.jsonTableKey, op, fv)
 		} else if (tblPath.tableKey != "" && !useKey) || tblPath.tableName == dbkey {
@@ -770,12 +780,16 @@ func TableData2Msi(tblPath *tablePath, useKey bool, op *string, msi *map[string]
 	return nil
 }
 
-func msi2TypedValue(msi map[string]interface{}) (*gnmipb.TypedValue, error) {
+func Msi2TypedValue(msi map[string]interface{}) (*gnmipb.TypedValue, error) {
+	log.V(4).Infof("State of map after adding redis data %v", msi)
 	jv, err := emitJSON(&msi)
 	if err != nil {
 		log.V(2).Infof("emitJSON err %s for  %v", err, msi)
 		return nil, fmt.Errorf("emitJSON err %s for  %v", err, msi)
 	}
+	if jv == nil { // json and err is nil because panic potentially happened
+		return nil, fmt.Errorf("emitJSON failed to grab json value of map due to potential panic")
+	}
 	return &gnmipb.TypedValue{
 		Value: &gnmipb.TypedValue_JsonIetfVal{
 			JsonIetfVal: jv,
@@ -806,20 +820,20 @@ func tableData2TypedValue(tblPaths []tablePath, op *string) (*gnmipb.TypedValue,
 					log.V(2).Infof("redis HGet failed for %v", tblPath)
 					return nil, err
 				}
+				log.V(4).Infof("Data pulled for key %s and field %s: %s", key, tblPath.field, val)
 				// TODO: support multiple table paths
 				return &gnmipb.TypedValue{
 					Value: &gnmipb.TypedValue_StringVal{
 						StringVal: val,
 					}}, nil
 			}
 		}
-
 		err := TableData2Msi(&tblPath, useKey, nil, &msi)
 		if err != nil {
 			return nil, err
 		}
 	}
-	return msi2TypedValue(msi)
+	return Msi2TypedValue(msi)
 }
 
 func enqueueFatalMsg(c *DbClient, msg string) {
@@ -888,7 +902,7 @@ func dbFieldMultiSubscribe(c *DbClient, gnmiPath *gnmipb.Path, onChange bool, in
 	}
 
 	sendVal := func(msi map[string]interface{}) error {
-		val, err := msi2TypedValue(msi)
+		val, err := Msi2TypedValue(msi)
 		if err != nil {
 			enqueueFatalMsg(c, err.Error())
 			return err
@@ -1140,7 +1154,7 @@ func dbTableKeySubscribe(c *DbClient, gnmiPath *gnmipb.Path, interval time.Durat
 
 	// Helper to send hash data over the stream
 	sendMsiData := func(msiData map[string]interface{}) error {
-		val, err := msi2TypedValue(msiData)
+		val, err := Msi2TypedValue(msiData)
 		if err != nil {
 			return err
 		}

sonic_data_client/mixed_db_client.go

@@ -486,7 +486,7 @@ func (c *MixedDbClient) tableData2TypedValue(tblPaths []tablePath, op *string) (
 			return nil, err
 		}
 	}
-	return msi2TypedValue(msi)
+	return Msi2TypedValue(msi)
 }
 
 func ConvertDbEntry(inputData map[string]interface{}) map[string]string {