Chore/panic (#398)

* Removed panic in JSON Resouce handling code. Some method signature impacted * removed all panic * Removed panic on json tools related * Removed panic from Working Memory * Removed panic from Working Memory part 2 * Removed panic from JsonDom.go * Worked trying to remove all bad panic --------- Co-authored-by: Ferdinand Neman <[email protected]>
hyperjumptech · Aug 28, 2023 · 3765de3 · 3765de3
1 parent 332a254
commit 3765de3
Show file tree

Hide file tree

Showing 12 changed files with 631 additions and 408 deletions.
diff --git a/ast/ExpressionAtom.go b/ast/ExpressionAtom.go
@@ -413,5 +413,5 @@ func (e *ExpressionAtom) Evaluate(dataContext IDataContext, memory *WorkingMemor
 		return e.Value, nil
 	}
 
-	panic("should not be reached")
+	return reflect.Value{}, fmt.Errorf("this portion of code should not be reached")
 }
diff --git a/ast/KnowledgeBase.go b/ast/KnowledgeBase.go
@@ -18,7 +18,6 @@ import (
 	"bytes"
 	"fmt"
 	"github.com/google/uuid"
-	"github.com/sirupsen/logrus"
 	"io"
 	"sort"
 	"strings"
@@ -84,7 +83,6 @@ func (lib *KnowledgeLibrary) LoadKnowledgeBaseFromReader(reader io.Reader, overw
 		if r := recover(); r != nil {
 			retKb = nil
 			retErr = fmt.Errorf("panic recovered during LoadKnowledgeBaseFromReader, recover \"%v\". send us your report to https://github.com/hyperjumptech/grule-rule-engine/issues", r)
-			logrus.Panicf("panic recovered during LoadKnowledgeBaseFromReader, recover \"%v\". send us your report to https://github.com/hyperjumptech/grule-rule-engine/issues", r)
 		}
 	}()
 
@@ -94,7 +92,10 @@ func (lib *KnowledgeLibrary) LoadKnowledgeBaseFromReader(reader io.Reader, overw
 
 		return nil, err
 	}
-	knowledgeBase := catalog.BuildKnowledgeBase()
+	knowledgeBase, err := catalog.BuildKnowledgeBase()
+	if err != nil {
+		return nil, err
+	}
 	if overwrite {
 		lib.Library[fmt.Sprintf("%s:%s", knowledgeBase.Name, knowledgeBase.Version)] = knowledgeBase
 
@@ -127,21 +128,25 @@ func (lib *KnowledgeLibrary) StoreKnowledgeBaseToWriter(writer io.Writer, name,
 
 // NewKnowledgeBaseInstance will create a new instance based on KnowledgeBase blue print
 // identified by its name and version
-func (lib *KnowledgeLibrary) NewKnowledgeBaseInstance(name, version string) *KnowledgeBase {
+func (lib *KnowledgeLibrary) NewKnowledgeBaseInstance(name, version string) (*KnowledgeBase, error) {
 	knowledgeBase, ok := lib.Library[fmt.Sprintf("%s:%s", name, version)]
 	if ok {
-		newClone := knowledgeBase.Clone(pkg.NewCloneTable())
+		newClone, err := knowledgeBase.Clone(pkg.NewCloneTable())
+		if err != nil {
+			return nil, err
+		}
 		if knowledgeBase.IsIdentical(newClone) {
 			AstLog.Debugf("Successfully create instance [%s:%s]", newClone.Name, newClone.Version)
 
-			return newClone
+			return newClone, nil
 		}
 		AstLog.Fatalf("ORIGIN   : %s", knowledgeBase.GetSnapshot())
 		AstLog.Fatalf("CLONE    : %s", newClone.GetSnapshot())
-		panic("The clone is not identical")
+
+		return nil, fmt.Errorf("the clone is not identical")
 	}
 
-	return nil
+	return nil, fmt.Errorf("specified knowledge base name and version not exist")
 }
 
 // KnowledgeBase is a collection of RuleEntries. It has a name and version.
@@ -210,7 +215,7 @@ func (e *KnowledgeBase) GetSnapshot() string {
 }
 
 // Clone will clone this instance of KnowledgeBase and produce another (structure wise) identical instance.
-func (e *KnowledgeBase) Clone(cloneTable *pkg.CloneTable) *KnowledgeBase {
+func (e *KnowledgeBase) Clone(cloneTable *pkg.CloneTable) (*KnowledgeBase, error) {
 	clone := &KnowledgeBase{
 		Name:        e.Name,
 		Version:     e.Version,
@@ -228,10 +233,14 @@ func (e *KnowledgeBase) Clone(cloneTable *pkg.CloneTable) *KnowledgeBase {
 		}
 	}
 	if e.WorkingMemory != nil {
-		clone.WorkingMemory = e.WorkingMemory.Clone(cloneTable)
+		wm, err := e.WorkingMemory.Clone(cloneTable)
+		if err != nil {
+			return nil, err
+		}
+		clone.WorkingMemory = wm
 	}
 
-	return clone
+	return clone, nil
 }
 
 // AddRuleEntry add ruleentry into this knowledge base.

diff --git a/ast/RuleEntry.go b/ast/RuleEntry.go
@@ -171,7 +171,8 @@ func (e *RuleEntry) Evaluate(ctx context.Context, dataContext IDataContext, memo
 	}
 	defer func() {
 		if r := recover(); r != nil {
-			err = fmt.Errorf("Error while evaluating rule %s, panic recovered", e.RuleName)
+			err = fmt.Errorf("error while evaluating rule %s, panic recovered", e.RuleName)
+			can = false
 		}
 	}()
 	if e.Retracted {

diff --git a/ast/Serializer.go b/ast/Serializer.go
@@ -101,7 +101,7 @@ type Catalog struct {
 // BuildKnowledgeBase will rebuild a knowledgebase from this Catalog.
 // the rebuilt KnowledgeBase is identical to the original KnowledgeBase from
 // which this Catalog was built.
-func (cat *Catalog) BuildKnowledgeBase() *KnowledgeBase {
+func (cat *Catalog) BuildKnowledgeBase() (*KnowledgeBase, error) {
 	workingMem := &WorkingMemory{
 		Name:                      cat.MemoryName,
 		Version:                   cat.MemoryVersion,
@@ -280,7 +280,7 @@ func (cat *Catalog) BuildKnowledgeBase() *KnowledgeBase {
 			}
 			importTable[amet.AstID] = n
 		default:
-			panic("Unrecognized meta type")
+			return nil, fmt.Errorf("unrecognized meta type")
 		}
 	}
 
@@ -407,7 +407,7 @@ func (cat *Catalog) BuildKnowledgeBase() *KnowledgeBase {
 				whenScope.Expression = importTable[amet.ExpressionID].(*Expression)
 			}
 		default:
-			panic("Unrecognized meta type")
+			return nil, fmt.Errorf("unknown AST type")
 		}
 	}
 
@@ -450,7 +450,7 @@ func (cat *Catalog) BuildKnowledgeBase() *KnowledgeBase {
 		}
 	}
 
-	return knowledgeBase
+	return knowledgeBase, nil
 }
 
 // Equals used for testing purpose, to ensure that two catalog

diff --git a/ast/WorkingMemory.go b/ast/WorkingMemory.go
@@ -143,7 +143,7 @@ func (workingMem *WorkingMemory) Equals(that *WorkingMemory) bool {
 }
 
 // Clone will clone this WorkingMemory. The new clone will have an identical structure
-func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemory {
+func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) (*WorkingMemory, error) {
 	AstLog.Debugf("Cloning working memory %s:%s", workingMem.Name, workingMem.Version)
 	clone := NewWorkingMemory(workingMem.Name, workingMem.Version)
 
@@ -154,7 +154,7 @@ func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemor
 				clone.expressionSnapshotMap[k] = cloneTable.Records[expr.AstID].CloneInstance.(*Expression)
 			} else {
 
-				panic(fmt.Sprintf("expression  %s is not on the clone table - %s", expr.GrlText, expr.GetSnapshot()))
+				return nil, fmt.Errorf("expression  %s is not on the clone table - %s", expr.GrlText, expr.GetSnapshot())
 			}
 		}
 	}
@@ -166,7 +166,7 @@ func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemor
 				clone.expressionAtomSnapshotMap[k] = cloneTable.Records[exprAtm.AstID].CloneInstance.(*ExpressionAtom)
 			} else {
 
-				panic(fmt.Sprintf("expression atom %s is not on the clone table. ASTID %s", exprAtm.GrlText, exprAtm.AstID))
+				return nil, fmt.Errorf("expression atom %s is not on the clone table. ASTID %s", exprAtm.GrlText, exprAtm.AstID)
 			}
 		}
 	}
@@ -178,7 +178,7 @@ func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemor
 				clone.variableSnapshotMap[key] = cloneTable.Records[variable.AstID].CloneInstance.(*Variable)
 			} else {
 
-				panic(fmt.Sprintf("variable %s is not on the clone table", variable.GrlText))
+				return nil, fmt.Errorf("variable %s is not on the clone table", variable.GrlText)
 			}
 		}
 	}
@@ -194,12 +194,12 @@ func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemor
 						clone.expressionVariableMap[clonedVari][k2] = cloneTable.Records[expr.AstID].CloneInstance.(*Expression)
 					} else {
 
-						panic(fmt.Sprintf("expression %s is not on the clone table", expr.GrlText))
+						return nil, fmt.Errorf("expression %s is not on the clone table", expr.GrlText)
 					}
 				}
 			} else {
 
-				panic(fmt.Sprintf("variable %s is not on the clone table", key.GrlText))
+				return nil, fmt.Errorf("variable %s is not on the clone table", key.GrlText)
 			}
 		}
 	}
@@ -215,23 +215,23 @@ func (workingMem *WorkingMemory) Clone(cloneTable *pkg.CloneTable) *WorkingMemor
 						clone.expressionAtomVariableMap[clonedVari][k2] = cloneTable.Records[expr.AstID].CloneInstance.(*ExpressionAtom)
 					} else {
 
-						panic(fmt.Sprintf("expression atom %s is not on the clone table", expr.GrlText))
+						return nil, fmt.Errorf("expression atom %s is not on the clone table", expr.GrlText)
 					}
 				}
 			} else {
 
-				panic(fmt.Sprintf("variable %s is not on the clone table", key.GrlText))
+				return nil, fmt.Errorf("variable %s is not on the clone table", key.GrlText)
 			}
 		}
 	}
 
 	if workingMem.Equals(clone) {
 		clone.DebugContent()
 
-		return clone
+		return clone, nil
 	}
 
-	panic("Clone not equals the origin.")
+	return nil, fmt.Errorf("clone not equals the origin")
 }
 
 // IndexVariables will index all expression and expression atoms that contains a speciffic variable name

diff --git a/examples/Issue328_test.go b/examples/Issue328_test.go
@@ -40,13 +40,11 @@ func TestMethodCall_SliceOOR(t *testing.T) {
 	err = rb.BuildRuleFromResource("Test", "0.1.1", pkg.NewBytesResource([]byte(SliceOORRule)))
 	assert.NoError(t, err)
 
-	// expect no panic and no error (ReturnErrOnFailedRuleEvaluation = false)
 	eng1 := &engine.GruleEngine{MaxCycle: 5}
 	kb := lib.NewKnowledgeBaseInstance("Test", "0.1.1")
 	err = eng1.Execute(dataContext, kb)
 	assert.NoError(t, err)
 
-	// expect no panic and execute to return an error here
 	eng1 = &engine.GruleEngine{MaxCycle: 5, ReturnErrOnFailedRuleEvaluation: true}
 	kb = lib.NewKnowledgeBaseInstance("Test", "0.1.1")
 	err = eng1.Execute(dataContext, kb)