From 883820c859c4dfc39d0055bd3ea78eb31e712079 Mon Sep 17 00:00:00 2001
From: ankur22 <ankur.agarwal@grafana.com>
Date: Fri, 19 Jan 2024 13:14:40 +0000
Subject: [PATCH] Refactor parseConsoleRemoteObject to return error

There is another use case for parseConsoleRemoteObject, which is when
JSONValue is called. At the moment this API uses valueFromRemoteObject
which returns a goja.Value. Interestingly, the return value is always
converted to string before being returned to the caller, so the extra
step to convert to goja and then back to string is not needed.
---
 common/frame_session.go |  5 ++++-
 common/page.go          |  5 ++++-
 common/remote_object.go | 42 +++++++++++++++++++++++------------------
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/common/frame_session.go b/common/frame_session.go
index cdbd822ba..bf3e22801 100644
--- a/common/frame_session.go
+++ b/common/frame_session.go
@@ -625,7 +625,10 @@ func (fs *FrameSession) onConsoleAPICalled(event *cdpruntime.EventConsoleAPICall
 
 	parsedObjects := make([]string, 0, len(event.Args))
 	for _, robj := range event.Args {
-		s := parseConsoleRemoteObject(fs.logger, robj)
+		s, err := parseConsoleRemoteObject(fs.logger, robj)
+		if err != nil {
+			fs.logger.Errorf("onConsoleAPICalled", "failed to parse console message %v", err)
+		}
 		parsedObjects = append(parsedObjects, s)
 	}
 
diff --git a/common/page.go b/common/page.go
index e20c6a73e..d4fe32bb0 100644
--- a/common/page.go
+++ b/common/page.go
@@ -1398,7 +1398,10 @@ func (p *Page) consoleMsgFromConsoleEvent(e *cdpruntime.EventConsoleAPICalled) (
 	)
 
 	for _, robj := range e.Args {
-		s := parseConsoleRemoteObject(p.logger, robj)
+		s, err := parseConsoleRemoteObject(p.logger, robj)
+		if err != nil {
+			p.logger.Errorf("consoleMsgFromConsoleEvent", "failed to parse console message %v", err)
+		}
 
 		objects = append(objects, s)
 		objectHandles = append(objectHandles, NewJSHandle(
diff --git a/common/remote_object.go b/common/remote_object.go
index 84ab0e199..1911897fd 100644
--- a/common/remote_object.go
+++ b/common/remote_object.go
@@ -227,42 +227,48 @@ func valueFromRemoteObject(_ context.Context, robj *cdpruntime.RemoteObject) (an
 	return val, err
 }
 
-func parseConsoleRemoteObjectPreview(logger *log.Logger, op *cdpruntime.ObjectPreview) string {
+func parseConsoleRemoteObjectPreview(logger *log.Logger, op *cdpruntime.ObjectPreview) (string, error) {
 	obj := make(map[string]string)
 	if op.Overflow {
 		logger.Infof("parseConsoleRemoteObjectPreview", "object is too large and will be parsed partially")
 	}
 
 	for _, p := range op.Properties {
-		val := parseConsoleRemoteObjectValue(logger, p.Type, p.Subtype, p.Value, p.ValuePreview)
+		val, err := parseConsoleRemoteObjectValue(logger, p.Type, p.Subtype, p.Value, p.ValuePreview)
+		if err != nil {
+			return "", err
+		}
 		obj[p.Name] = val
 	}
 
 	bb, err := json.Marshal(obj)
 	if err != nil {
-		logger.Errorf("parseConsoleRemoteObjectPreview", "failed to marshal object to string: %v", err)
+		return "", fmt.Errorf("marshaling object to string: %w", err)
 	}
 
-	return string(bb)
+	return string(bb), nil
 }
 
-func parseConsoleRemoteArrayPreview(logger *log.Logger, op *cdpruntime.ObjectPreview) string {
+func parseConsoleRemoteArrayPreview(logger *log.Logger, op *cdpruntime.ObjectPreview) (string, error) {
 	arr := make([]any, 0, len(op.Properties))
 	if op.Overflow {
-		logger.Warnf("parseConsoleRemoteArrayPreview", "array is too large and will be parsed partially")
+		logger.Infof("parseConsoleRemoteArrayPreview", "array is too large and will be parsed partially")
 	}
 
 	for _, p := range op.Properties {
-		val := parseConsoleRemoteObjectValue(logger, p.Type, p.Subtype, p.Value, p.ValuePreview)
+		val, err := parseConsoleRemoteObjectValue(logger, p.Type, p.Subtype, p.Value, p.ValuePreview)
+		if err != nil {
+			return "", err
+		}
 		arr = append(arr, val)
 	}
 
 	bb, err := json.Marshal(arr)
 	if err != nil {
-		logger.Errorf("parseConsoleRemoteArrayPreview", "failed to marshal array to string: %v", err)
+		return "", fmt.Errorf("marshaling array to string: %w", err)
 	}
 
-	return string(bb)
+	return string(bb), nil
 }
 
 //nolint:cyclop
@@ -272,12 +278,12 @@ func parseConsoleRemoteObjectValue(
 	st cdpruntime.Subtype,
 	val string,
 	op *cdpruntime.ObjectPreview,
-) string {
+) (string, error) {
 	switch t {
 	case cdpruntime.TypeAccessor:
-		return "accessor"
+		return "accessor", nil
 	case cdpruntime.TypeFunction:
-		return "function()"
+		return "function()", nil
 	case cdpruntime.TypeString:
 		if strings.HasPrefix(val, `"`) {
 			val = strings.TrimPrefix(val, `"`)
@@ -291,13 +297,13 @@ func parseConsoleRemoteObjectValue(
 			return parseConsoleRemoteObjectPreview(logger, op)
 		}
 		if val == "Object" {
-			return val
+			return val, nil
 		}
 		if st == "null" {
-			return "null"
+			return "null", nil
 		}
 	case cdpruntime.TypeUndefined:
-		return "undefined"
+		return "undefined", nil
 	// The following cases are here to clarify that all cases have been
 	// considered, but that the result will return val without processing it.
 	case cdpruntime.TypeNumber:
@@ -306,15 +312,15 @@ func parseConsoleRemoteObjectValue(
 	case cdpruntime.TypeBigint:
 	}
 
-	return val
+	return val, nil
 }
 
 // parseConsoleRemoteObject is to be used by callers that are working with
 // console messages that are written to Chrome's console by the website under
 // test.
-func parseConsoleRemoteObject(logger *log.Logger, obj *cdpruntime.RemoteObject) string {
+func parseConsoleRemoteObject(logger *log.Logger, obj *cdpruntime.RemoteObject) (string, error) {
 	if obj.UnserializableValue != "" {
-		return obj.UnserializableValue.String()
+		return obj.UnserializableValue.String(), nil
 	}
 
 	return parseConsoleRemoteObjectValue(logger, obj.Type, obj.Subtype, string(obj.Value), obj.Preview)