Merge pull request #130 from grafana/fix/120-panic-page-goto

Implement RemoteObject conversion to native types

Author: Ivan Mirić

common/execution_context.go

@@ -40,20 +40,25 @@ var /* const */ sourceURLRegex = regexp.MustCompile(`^(?s)[\040\t]*//[@#] source
 // ExecutionContext represents a JS execution context
 type ExecutionContext struct {
 	ctx            context.Context
+	logger         *Logger
 	session        *Session
 	frame          *Frame
 	id             runtime.ExecutionContextID
 	injectedScript api.JSHandle
 }
 
 // NewExecutionContext creates a new JS execution context
-func NewExecutionContext(ctx context.Context, session *Session, frame *Frame, id runtime.ExecutionContextID) *ExecutionContext {
+func NewExecutionContext(
+	ctx context.Context, session *Session, frame *Frame,
+	id runtime.ExecutionContextID, logger *Logger,
+) *ExecutionContext {
 	return &ExecutionContext{
 		ctx:            ctx,
 		session:        session,
 		frame:          frame,
 		id:             id,
 		injectedScript: nil,
+		logger:         logger,
 	}
 }
 
@@ -69,7 +74,7 @@ func (e *ExecutionContext) adoptBackendNodeId(backendNodeID cdp.BackendNodeID) (
 		return nil, fmt.Errorf("unable to get DOM node: %w", err)
 	}
 
-	return NewJSHandle(e.ctx, e.session, e, e.frame, remoteObj).AsElement().(*ElementHandle), nil
+	return NewJSHandle(e.ctx, e.session, e, e.frame, remoteObj, e.logger).AsElement().(*ElementHandle), nil
 }
 
 // Adopts the specified element handle into this execution context from another execution context
@@ -129,7 +134,7 @@ func (e *ExecutionContext) evaluate(apiCtx context.Context, forceCallable bool,
 				}
 			} else if remoteObject.ObjectID != "" {
 				// Note: we don't use the passed in apiCtx here as it could be tied to a timeout
-				res = NewJSHandle(e.ctx, e.session, e, e.frame, remoteObject)
+				res = NewJSHandle(e.ctx, e.session, e, e.frame, remoteObject, e.logger)
 			}
 		}
 	} else {
@@ -165,7 +170,7 @@ func (e *ExecutionContext) evaluate(apiCtx context.Context, forceCallable bool,
 				}
 			} else if remoteObject.ObjectID != "" {
 				// Note: we don't use the passed in apiCtx here as it could be tied to a timeout
-				res = NewJSHandle(e.ctx, e.session, e, e.frame, remoteObject)
+				res = NewJSHandle(e.ctx, e.session, e, e.frame, remoteObject, e.logger)
 			}
 		}
 	}

common/frame_session.go

@@ -41,6 +41,7 @@ import (
 	"github.com/chromedp/cdproto/security"
 	"github.com/chromedp/cdproto/target"
 	"github.com/grafana/xk6-browser/api"
+	"github.com/sirupsen/logrus"
 	k6lib "go.k6.io/k6/lib"
 	k6stats "go.k6.io/k6/stats"
 )
@@ -77,9 +78,14 @@ type FrameSession struct {
 	childSessions map[cdp.FrameID]*FrameSession
 
 	logger *Logger
+	// logger that will properly serialize RemoteObject instances
+	serializer *logrus.Logger
 }
 
-func NewFrameSession(ctx context.Context, session *Session, page *Page, parent *FrameSession, targetID target.ID, logger *Logger) (*FrameSession, error) {
+func NewFrameSession(
+	ctx context.Context, session *Session, page *Page, parent *FrameSession,
+	targetID target.ID, logger *Logger,
+) (*FrameSession, error) {
 	logger.Debugf("NewFrameSession", "sid:%v tid:%v", session.id, targetID)
 	fs := FrameSession{
 		ctx:                  ctx, // TODO: create cancelable context that can be used to cancel and close all child sessions
@@ -96,6 +102,11 @@ func NewFrameSession(ctx context.Context, session *Session, page *Page, parent *
 		eventCh:              make(chan Event),
 		childSessions:        make(map[cdp.FrameID]*FrameSession),
 		logger:               logger,
+		serializer: &logrus.Logger{
+			Out:       logger.log.Out,
+			Level:     logger.log.Level,
+			Formatter: &consoleLogFormatter{logger.log.Formatter},
+		},
 	}
 	var err error
 	if fs.parent != nil {
@@ -455,33 +466,34 @@ func (fs *FrameSession) navigateFrame(frame *Frame, url, referrer string) (strin
 
 func (fs *FrameSession) onConsoleAPICalled(event *runtime.EventConsoleAPICalled) {
 	// TODO: switch to using browser logger instead of directly outputting to k6 logging system
-	state := k6lib.GetState(fs.ctx)
-	l := state.Logger.
+	l := fs.serializer.
 		WithTime(event.Timestamp.Time()).
 		WithField("source", "browser-console-api")
-	if state.Group.Path != "" {
-		l = l.WithField("group", state.Group.Path)
+
+	if s := k6lib.GetState(fs.ctx); s.Group.Path != "" {
+		l = l.WithField("group", s.Group.Path)
 	}
-	var convertedArgs []interface{}
-	for _, arg := range event.Args {
-		i, err := interfaceFromRemoteObject(arg)
+
+	var parsedObjects []interface{}
+	for _, robj := range event.Args {
+		i, err := parseRemoteObject(robj)
 		if err != nil {
-			// TODO(fix): this should not throw!
-			k6Throw(fs.ctx, "unable to parse remote object value: %w", err)
+			handleParseRemoteObjectErr(fs.ctx, err, l)
 		}
-		convertedArgs = append(convertedArgs, i)
+		parsedObjects = append(parsedObjects, i)
 	}
+
+	l = l.WithField("objects", parsedObjects)
+
 	switch event.Type {
-	case "log":
-		l.Info(convertedArgs...)
-	case "info":
-		l.Info(convertedArgs...)
+	case "log", "info":
+		l.Info()
 	case "warning":
-		l.Warn(convertedArgs...)
+		l.Warn()
 	case "error":
-		l.Error(convertedArgs...)
+		l.Error()
 	default:
-		l.Debug(convertedArgs...)
+		l.Debug()
 	}
 }
 
@@ -517,7 +529,7 @@ func (fs *FrameSession) onExecutionContextCreated(event *runtime.EventExecutionC
 	if i.Type == "isolated" {
 		fs.isolatedWorlds[event.Context.Name] = true
 	}
-	context := NewExecutionContext(fs.ctx, fs.session, frame, event.Context.ID)
+	context := NewExecutionContext(fs.ctx, fs.session, frame, event.Context.ID, fs.logger)
 	if world != "" {
 		frame.setContext(world, context)
 	}

common/helpers.go

@@ -28,7 +28,6 @@ import (
 	"math"
 	"os"
 	"reflect"
-	"strconv"
 	"strings"
 	"time"
 
@@ -168,42 +167,6 @@ func errorFromDOMError(domErr string) error {
 	return fmt.Errorf(domErr)
 }
 
-func interfaceFromRemoteObject(remoteObject *cdpruntime.RemoteObject) (interface{}, error) {
-	if remoteObject.ObjectID != "" {
-		return nil, ErrUnexpectedRemoteObjectWithID
-	}
-	if remoteObject.UnserializableValue != "" {
-		if remoteObject.Type == "bigint" {
-			n, err := strconv.ParseInt(strings.Replace(remoteObject.UnserializableValue.String(), "n", "", -1), 10, 64)
-			if err != nil {
-				return nil, BigIntParseError{err}
-			}
-			return n, nil
-		}
-		switch remoteObject.UnserializableValue.String() {
-		case "-0": // To handle +0 divided by negative number
-			return math.Float64frombits(0 | (1 << 63)), nil
-		case "NaN":
-			return math.NaN(), nil
-		case "Infinity":
-			return math.Inf(0), nil
-		case "-Infinity":
-			return math.Inf(-1), nil
-		default:
-			return nil, UnserializableValueError{remoteObject.UnserializableValue}
-		}
-	}
-	if remoteObject.Type == "undefined" {
-		return "undefined", nil
-	}
-	var i interface{}
-	err := json.Unmarshal(remoteObject.Value, &i)
-	if err != nil {
-		return nil, err
-	}
-	return i, nil
-}
-
 func stringSliceContains(s []string, e string) bool {
 	for _, a := range s {
 		if a == e {
@@ -213,15 +176,6 @@ func stringSliceContains(s []string, e string) bool {
 	return false
 }
 
-func valueFromRemoteObject(ctx context.Context, remoteObject *cdpruntime.RemoteObject) (goja.Value, error) {
-	rt := k6common.GetRuntime(ctx)
-	i, err := interfaceFromRemoteObject(remoteObject)
-	if i == "undefined" {
-		return goja.Undefined(), err
-	}
-	return rt.ToValue(i), err
-}
-
 func createWaitForEventHandler(
 	ctx context.Context,
 	emitter EventEmitter, events []string,