Merge pull request #169 from grafana/fix/49-cannot-find-context-with-…

…specified-id

Fix/49 cannot find context with specified id

common/browser.go

@@ -242,14 +242,16 @@ func (b *Browser) onAttachedToTarget(ev *target.EventAttachedToTarget) {
 		b.sessionIDtoTargetID[ev.SessionID] = evti.TargetID
 		b.sessionIDtoTargetIDMu.Unlock()
 	case "page":
-		var opener *Page = nil
+		// Opener is nil for the initial page
+		var opener *Page
 		b.pagesMu.RLock()
 		if t, ok := b.pages[evti.OpenerID]; ok {
 			opener = t
 		}
-		b.logger.Debugf("Browser:onAttachedToTarget:page", "sid:%v tid:%v opener nil:%t", ev.SessionID, evti.TargetID, opener == nil)
 		b.pagesMu.RUnlock()
 
+		b.logger.Debugf("Browser:onAttachedToTarget:page", "sid:%v tid:%v opener nil:%t", ev.SessionID, evti.TargetID, opener == nil)
+
 		p, err := NewPage(b.ctx, b.conn.getSession(ev.SessionID), browserCtx, evti.TargetID, opener, true, b.logger)
 		if err != nil {
 			isRunning := atomic.LoadInt64(&b.state) == BrowserStateOpen && b.IsConnected() //b.conn.isConnected()
@@ -279,6 +281,8 @@ func (b *Browser) onAttachedToTarget(ev *target.EventAttachedToTarget) {
 	}
 }
 
+// onDetachedFromTarget event can be issued multiple times per target if multiple
+// sessions have been attached to it. So we'll remove the page only once.
 func (b *Browser) onDetachedFromTarget(ev *target.EventDetachedFromTarget) {
 	b.sessionIDtoTargetIDMu.RLock()
 	targetID, ok := b.sessionIDtoTargetID[ev.SessionID]

common/connection.go

@@ -345,7 +345,7 @@ func (c *Connection) recvLoop() {
 	}
 }
 
-func (c *Connection) send(msg *cdproto.Message, recvCh chan *cdproto.Message, res easyjson.Unmarshaler) error {
+func (c *Connection) send(ctx context.Context, msg *cdproto.Message, recvCh chan *cdproto.Message, res easyjson.Unmarshaler) error {
 	select {
 	case c.sendCh <- msg:
 	case err := <-c.errorCh:
@@ -358,9 +358,12 @@ func (c *Connection) send(msg *cdproto.Message, recvCh chan *cdproto.Message, re
 	case <-c.done:
 		c.logger.Debugf("Connection:send:<-c.done", "wsURL:%q sid:%v", c.wsURL, msg.SessionID)
 		return nil
+	case <-ctx.Done():
+		c.logger.Errorf("Connection:send:<-ctx.Done()", "wsURL:%q sid:%v err:%v", c.wsURL, msg.SessionID, c.ctx.Err())
+		return ctx.Err()
 	case <-c.ctx.Done():
 		c.logger.Errorf("Connection:send:<-c.ctx.Done()", "wsURL:%q sid:%v err:%v", c.wsURL, msg.SessionID, c.ctx.Err())
-		return nil
+		return ctx.Err()
 	}
 
 	// Block waiting for response.
@@ -395,13 +398,12 @@ func (c *Connection) send(msg *cdproto.Message, recvCh chan *cdproto.Message, re
 		return &websocket.CloseError{Code: code}
 	case <-c.done:
 		c.logger.Debugf("Connection:send:<-c.done #2", "sid:%v tid:%v wsURL:%q", msg.SessionID, tid, c.wsURL)
+	case <-ctx.Done():
+		c.logger.Debugf("Connection:send:<-ctx.Done()", "sid:%v tid:%v wsURL:%q err:%v", msg.SessionID, tid, c.wsURL, c.ctx.Err())
+		return ctx.Err()
 	case <-c.ctx.Done():
 		c.logger.Debugf("Connection:send:<-c.ctx.Done()", "sid:%v tid:%v wsURL:%q err:%v", msg.SessionID, tid, c.wsURL, c.ctx.Err())
-		// TODO: this brings many bugs to the surface
 		return c.ctx.Err()
-		// TODO: add a timeout?
-		// case <-timeout:
-		// 	return
 	}
 	return nil
 }
@@ -443,15 +445,13 @@ func (c *Connection) sendLoop() {
 		case code := <-c.closeCh:
 			c.logger.Debugf("Connection:sendLoop:<-c.closeCh", "wsURL:%q code:%d", c.wsURL, code)
 			_ = c.closeConnection(code)
+			return
 		case <-c.done:
 			c.logger.Debugf("Connection:sendLoop:<-c.done#2", "wsURL:%q", c.wsURL)
+			return
 		case <-c.ctx.Done():
 			c.logger.Debugf("connection:sendLoop", "returning, ctx.Err: %q", c.ctx.Err())
 			return
-			// case <-time.After(time.Second * 10):
-			// 	c.logger.Errorf("connection:sendLoop", "returning, timeout")
-			// 	c.Close()
-			// 	return
 		}
 	}
 }
@@ -513,5 +513,5 @@ func (c *Connection) Execute(ctx context.Context, method string, params easyjson
 		Method: cdproto.MethodType(method),
 		Params: buf,
 	}
-	return c.send(msg, ch, res)
+	return c.send(c.ctx, msg, ch, res)
 }

common/context.go

@@ -64,3 +64,14 @@ func GetProcessID(ctx context.Context) int {
 	v, _ := ctx.Value(ctxKeyPid).(int)
 	return v // it will return zero on error
 }
+
+// contextWithDoneChan returns a new context that is canceled when the done channel
+// is closed. The context will leak if the done channel is never closed.
+func contextWithDoneChan(ctx context.Context, done chan struct{}) context.Context {
+	ctx, cancel := context.WithCancel(ctx)
+	go func() {
+		<-done
+		cancel()
+	}()
+	return ctx
+}

common/frame.go

@@ -542,7 +542,7 @@ func (f *Frame) waitForFunction(
 			rt.ToValue(polling),
 		}, args...)...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("frame cannot wait for function: %w", err)
 	}
 	return result, nil
 }
@@ -569,14 +569,14 @@ func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptio
 
 	ec := f.executionContexts[mainWorld]
 	if ec == nil {
-		return nil, fmt.Errorf("cannot find execution context: %q", mainWorld)
+		return nil, fmt.Errorf("wait for selector cannot find execution context: %q", mainWorld)
 	}
 	// an element should belong to the current execution context.
 	// otherwise, we should adopt it to this execution context.
 	if ec != handle.execCtx {
 		defer handle.Dispose()
 		if handle, err = ec.adoptElementHandle(handle); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("wait for selector cannot adopt element handle: %w", err)
 		}
 	}
 
@@ -1481,7 +1481,11 @@ func (f *Frame) evaluate(
 	if ec == nil {
 		return nil, fmt.Errorf("cannot find execution context: %q", world)
 	}
-	return ec.evaluate(apiCtx, opts, pageFunc, args...)
+	eh, err := ec.evaluate(apiCtx, opts, pageFunc, args...)
+	if err != nil {
+		return nil, fmt.Errorf("frame cannot evaluate: %w", err)
+	}
+	return eh, nil
 }
 
 // frameExecutionContext represents a JS execution context that belongs to Frame.