Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error return value from page.on handlers #1495

Merged
merged 5 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion browser/page_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package browser

import (
"context"
"errors"
"fmt"
"time"

Expand Down Expand Up @@ -457,7 +458,7 @@ func mapPageOn(vu moduleVU, p *common.Page) func(common.PageOnEventName, sobek.C
// Run the the event handler in the task queue to
// ensure that the handler is executed on the event loop.
tq := vu.taskQueueRegistry.get(ctx, p.TargetID())
eventHandler := func(event common.PageOnEvent) {
eventHandler := func(event common.PageOnEvent) error {
mapping := pageOnEvent.mapp(vu, event)

done := make(chan struct{})
Expand All @@ -480,8 +481,11 @@ func mapPageOn(vu moduleVU, p *common.Page) func(common.PageOnEventName, sobek.C
select {
case <-done:
case <-ctx.Done():
return errors.New("iteration ended before page.on handler completed executing")
}
}

return nil
}

return p.On(eventName, eventHandler) //nolint:wrapcheck
Expand Down
4 changes: 3 additions & 1 deletion browser/sync_page_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,15 @@ func syncMapPage(vu moduleVU, p *common.Page) mapping { //nolint:gocognit,cyclop
_, err := handler(sobek.Undefined(), vu.Runtime().ToValue(mapping))
return err
}
runInTaskQueue := func(a common.PageOnEvent) {
runInTaskQueue := func(a common.PageOnEvent) error {
tq.Queue(func() error {
if err := mapMsgAndHandleEvent(a.ConsoleMessage); err != nil {
return fmt.Errorf("executing page.on handler: %w", err)
}
return nil
})

return nil
}

return p.On(event, runInTaskQueue) //nolint:wrapcheck
Expand Down
24 changes: 17 additions & 7 deletions common/page.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ type ConsoleMessage struct {
Type string
}

type PageOnHandler func(PageOnEvent) error

// Page stores Page/tab related context.
type Page struct {
BaseEventEmitter
Expand Down Expand Up @@ -245,7 +247,7 @@ type Page struct {
backgroundPage bool

eventCh chan Event
eventHandlers map[PageOnEventName][]func(PageOnEvent)
eventHandlers map[PageOnEventName][]PageOnHandler
eventHandlersMu sync.RWMutex

mainFrameSession *FrameSession
Expand Down Expand Up @@ -284,7 +286,7 @@ func NewPage(
Keyboard: NewKeyboard(ctx, s),
jsEnabled: true,
eventCh: make(chan Event),
eventHandlers: make(map[PageOnEventName][]func(PageOnEvent)),
eventHandlers: make(map[PageOnEventName][]PageOnHandler),
frameSessions: make(map[cdp.FrameID]*FrameSession),
workers: make(map[target.SessionID]*Worker),
vu: k6ext.GetVU(ctx),
Expand Down Expand Up @@ -483,17 +485,21 @@ func (p *Page) urlTagName(url string, method string) (string, bool) {
p.eventHandlersMu.RLock()
defer p.eventHandlersMu.RUnlock()
for _, h := range p.eventHandlers[EventPageMetricCalled] {
func() {
err := func() error {
// Handlers can register other handlers, so we need to
// unlock the mutex before calling the next handler.
p.eventHandlersMu.RUnlock()
defer p.eventHandlersMu.RLock()

// Call and wait for the handler to complete.
h(PageOnEvent{
return h(PageOnEvent{
Metric: em,
})
}()
if err != nil {
p.logger.Debugf("urlTagName", "handler returned an error: %v", err)
return "", false
}
}

// If a match was found then the name field in em will have been updated.
Expand Down Expand Up @@ -521,9 +527,13 @@ func (p *Page) onConsoleAPICalled(event *cdpruntime.EventConsoleAPICalled) {
p.eventHandlersMu.RLock()
defer p.eventHandlersMu.RUnlock()
for _, h := range p.eventHandlers[EventPageConsoleAPICalled] {
h(PageOnEvent{
err := h(PageOnEvent{
ConsoleMessage: m,
})
if err != nil {
p.logger.Debugf("onConsoleAPICalled", "handler returned an error: %v", err)
return
}
}
}

Expand Down Expand Up @@ -1194,12 +1204,12 @@ type PageOnEvent struct {
// On subscribes to a page event for which the given handler will be executed
// passing in the ConsoleMessage associated with the event.
// The only accepted event value is 'console'.
func (p *Page) On(event PageOnEventName, handler func(PageOnEvent)) error {
func (p *Page) On(event PageOnEventName, handler PageOnHandler) error {
p.eventHandlersMu.Lock()
defer p.eventHandlersMu.Unlock()

if _, ok := p.eventHandlers[event]; !ok {
p.eventHandlers[event] = make([]func(PageOnEvent), 0, 1)
p.eventHandlers[event] = make([]PageOnHandler, 0, 1)
}
p.eventHandlers[event] = append(p.eventHandlers[event], handler)

Expand Down
6 changes: 4 additions & 2 deletions tests/page_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1251,14 +1251,16 @@ func TestPageOn(t *testing.T) {
)

// Console Messages should be multiplexed for every registered handler
eventHandlerOne := func(event common.PageOnEvent) {
eventHandlerOne := func(event common.PageOnEvent) error {
defer close(done1)
tc.assertFn(t, event.ConsoleMessage)
return nil
}

eventHandlerTwo := func(event common.PageOnEvent) {
eventHandlerTwo := func(event common.PageOnEvent) error {
defer close(done2)
tc.assertFn(t, event.ConsoleMessage)
return nil
}

// eventHandlerOne and eventHandlerTwo will be called from a
Expand Down
3 changes: 2 additions & 1 deletion tests/remote_obj_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ func TestConsoleLogParse(t *testing.T) {

done := make(chan bool)

eventHandler := func(event common.PageOnEvent) {
eventHandler := func(event common.PageOnEvent) error {
defer close(done)
assert.Equal(t, tt.want, event.ConsoleMessage.Text)
return nil
}

// eventHandler will be called from a separate goroutine from within
Expand Down
Loading