Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Commit 468cb44

Browse files
committed
Fix interface conversion in document
The error was: panic: interface conversion: interface {} is nil, not *common.ElementHandle The cause of the error was trying to assert a nil result in Frame.document to *ElementHandle. Of course, this is probably the surface error. There may be other causes for it, or maybe not. It was hard to test Frame, so I used an abstraction called FrameExecutionContext. There are a lot of problems are happening with execution contexts. See: #125 and #49, for example. So, it's better to make them testable, starting here. Fixes #53
1 parent 3aca99a commit 468cb44

File tree

4 files changed

+168
-8
lines changed

4 files changed

+168
-8
lines changed

common/execution_context.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,3 +217,8 @@ func (e *ExecutionContext) EvaluateHandle(apiCtx context.Context, pageFunc goja.
217217
func (e *ExecutionContext) Frame() *Frame {
218218
return e.frame
219219
}
220+
221+
// ID returns the CDP runtime ID of this execution context.
222+
func (e *ExecutionContext) ID() runtime.ExecutionContextID {
223+
return e.id
224+
}

common/frame.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ type Frame struct {
117117

118118
documentHandle *ElementHandle
119119

120-
mainExecutionContext *ExecutionContext
121-
utilityExecutionContext *ExecutionContext
120+
mainExecutionContext FrameExecutionContext
121+
utilityExecutionContext FrameExecutionContext
122122
mainExecutionContextCh chan bool
123123
utilityExecutionContextCh chan bool
124124
mainExecutionContextHasWaited int32
@@ -311,11 +311,17 @@ func (f *Frame) document() (*ElementHandle, error) {
311311
if f.documentHandle != nil {
312312
return f.documentHandle, nil
313313
}
314+
314315
f.waitForExecutionContext("main")
316+
315317
result, err := f.mainExecutionContext.evaluate(f.ctx, false, false, rt.ToValue("document"), nil)
316318
if err != nil {
317-
return nil, err
319+
return nil, fmt.Errorf("frame document: cannot evaluate in main execution context: %w", err)
318320
}
321+
if result == nil {
322+
return nil, fmt.Errorf("frame document: evaluate result is nil in main execution context: %w", err)
323+
}
324+
319325
f.documentHandle = result.(*ElementHandle)
320326
return f.documentHandle, err
321327
}
@@ -350,10 +356,10 @@ func (f *Frame) navigated(name string, url string, loaderID string) {
350356
}
351357

352358
func (f *Frame) nullContext(execCtxID runtime.ExecutionContextID) {
353-
if f.mainExecutionContext != nil && f.mainExecutionContext.id == execCtxID {
359+
if f.mainExecutionContext != nil && f.mainExecutionContext.ID() == execCtxID {
354360
f.mainExecutionContext = nil
355361
f.documentHandle = nil
356-
} else if f.utilityExecutionContext != nil && f.utilityExecutionContext.id == execCtxID {
362+
} else if f.utilityExecutionContext != nil && f.utilityExecutionContext.ID() == execCtxID {
357363
f.utilityExecutionContext = nil
358364
}
359365
}
@@ -412,7 +418,7 @@ func (f *Frame) requestByID(reqID network.RequestID) *Request {
412418
return frameSession.networkManager.requestFromID(reqID)
413419
}
414420

415-
func (f *Frame) setContext(world string, execCtx *ExecutionContext) {
421+
func (f *Frame) setContext(world string, execCtx FrameExecutionContext) {
416422
if world == "main" {
417423
f.mainExecutionContext = execCtx
418424
if len(f.mainExecutionContextCh) == 0 {
@@ -507,13 +513,13 @@ func (f *Frame) waitForSelector(selector string, opts *FrameWaitForSelectorOptio
507513

508514
func (f *Frame) AddScriptTag(opts goja.Value) {
509515
rt := k6common.GetRuntime(f.ctx)
510-
k6common.Throw(rt, errors.New("Frame.AddScriptTag() has not been implemented yet!"))
516+
k6common.Throw(rt, errors.New("Frame.AddScriptTag() has not been implemented yet"))
511517
applySlowMo(f.ctx)
512518
}
513519

514520
func (f *Frame) AddStyleTag(opts goja.Value) {
515521
rt := k6common.GetRuntime(f.ctx)
516-
k6common.Throw(rt, errors.New("Frame.AddStyleTag() has not been implemented yet!"))
522+
k6common.Throw(rt, errors.New("Frame.AddStyleTag() has not been implemented yet"))
517523
applySlowMo(f.ctx)
518524
}
519525

common/frame_execution_context.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
*
3+
* xk6-browser - a browser automation extension for k6
4+
* Copyright (C) 2021 Load Impact
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU Affero General Public License as
8+
* published by the Free Software Foundation, either version 3 of the
9+
* License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Affero General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Affero General Public License
17+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*
19+
*/
20+
21+
package common
22+
23+
import (
24+
"context"
25+
26+
"github.com/chromedp/cdproto/cdp"
27+
"github.com/chromedp/cdproto/runtime"
28+
"github.com/dop251/goja"
29+
"github.com/grafana/xk6-browser/api"
30+
)
31+
32+
// FrameExecutionContext represents a JS execution context that belongs to Frame.
33+
type FrameExecutionContext interface {
34+
// adoptBackendNodeId adopts specified backend node into this execution
35+
// context from another execution context.
36+
adoptBackendNodeId(backendNodeID cdp.BackendNodeID) (*ElementHandle, error)
37+
38+
// adoptElementHandle adopts the specified element handle into this
39+
// execution context from another execution context.
40+
adoptElementHandle(elementHandle *ElementHandle) (*ElementHandle, error)
41+
42+
// evaluate will evaluate provided callable within this execution
43+
// context and return by value or handle.
44+
evaluate(
45+
apiCtx context.Context,
46+
forceCallable bool, returnByValue bool,
47+
pageFunc goja.Value, args ...goja.Value,
48+
) (res interface{}, err error)
49+
50+
// getInjectedScript returns a JS handle to the injected script of helper
51+
// functions.
52+
getInjectedScript(apiCtx context.Context) (api.JSHandle, error)
53+
54+
// Evaluate will evaluate provided page function within this execution
55+
// context.
56+
Evaluate(
57+
apiCtx context.Context,
58+
pageFunc goja.Value, args ...goja.Value,
59+
) (interface{}, error)
60+
61+
// EvaluateHandle will evaluate provided page function within this
62+
// execution context.
63+
EvaluateHandle(
64+
apiCtx context.Context,
65+
pageFunc goja.Value, args ...goja.Value,
66+
) (api.JSHandle, error)
67+
68+
// Frame returns the frame that this execution context belongs to.
69+
Frame() *Frame
70+
71+
// id returns the CDP runtime ID of this execution context.
72+
ID() runtime.ExecutionContextID
73+
}

common/frame_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
*
3+
* xk6-browser - a browser automation extension for k6
4+
* Copyright (C) 2021 Load Impact
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU Affero General Public License as
8+
* published by the Free Software Foundation, either version 3 of the
9+
* License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Affero General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Affero General Public License
17+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*
19+
*/
20+
21+
package common
22+
23+
import (
24+
"context"
25+
"testing"
26+
27+
"github.com/chromedp/cdproto/cdp"
28+
"github.com/dop251/goja"
29+
"github.com/stretchr/testify/require"
30+
)
31+
32+
func TestFrameDocument_Issue53(t *testing.T) {
33+
t.Parallel()
34+
35+
ctx := context.Background()
36+
fm := NewFrameManager(ctx, nil, nil, nil, nil)
37+
frame := NewFrame(ctx, fm, nil, cdp.FrameID("42"))
38+
39+
// frame should not panic with a nil document
40+
stub := &executionContextTestStub{
41+
evaluateFn: func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
42+
// return nil to test for panic
43+
return nil, nil
44+
},
45+
}
46+
47+
// document() waits for the main execution context
48+
go frame.setContext("main", stub)
49+
50+
require.NotPanics(t, func() {
51+
_, err := frame.document()
52+
require.Error(t, err)
53+
})
54+
55+
// frame gets the document from the evaluate call
56+
want := &ElementHandle{}
57+
stub.evaluateFn = func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
58+
return want, nil
59+
}
60+
got, err := frame.document()
61+
require.NoError(t, err)
62+
require.Equal(t, want, got)
63+
64+
// frame sets documentHandle in the document method
65+
got = frame.documentHandle
66+
require.Equal(t, want, got)
67+
}
68+
69+
type executionContextTestStub struct {
70+
ExecutionContext
71+
evaluateFn func(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error)
72+
}
73+
74+
func (e executionContextTestStub) evaluate(apiCtx context.Context, forceCallable bool, returnByValue bool, pageFunc goja.Value, args ...goja.Value) (res interface{}, err error) {
75+
return e.evaluateFn(apiCtx, forceCallable, returnByValue, pageFunc, args...)
76+
}

0 commit comments

Comments
 (0)