Skip to content

Commit

Permalink
Add error handling to getProperties
Browse files Browse the repository at this point in the history
Updates: #804
Related: #688
  • Loading branch information
inancgumus committed Mar 14, 2023
1 parent 0d5de5e commit 95a0d8b
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 19 deletions.
2 changes: 1 addition & 1 deletion api/js_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type JSHandle interface {
Dispose()
Evaluate(pageFunc goja.Value, args ...goja.Value) any
EvaluateHandle(pageFunc goja.Value, args ...goja.Value) (JSHandle, error)
GetProperties() map[string]JSHandle
GetProperties() (map[string]JSHandle, error)
GetProperty(propertyName string) JSHandle
JSONValue() goja.Value
ObjectID() cdpruntime.RemoteObjectID
Expand Down
11 changes: 8 additions & 3 deletions browser/mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,17 @@ func mapJSHandle(vu moduleVU, jsh api.JSHandle) mapping {
}
return mapJSHandle(vu, h), nil
},
"getProperties": func() *goja.Object {
"getProperties": func() (mapping, error) {
props, err := jsh.GetProperties()
if err != nil {
return nil, err //nolint:wrapcheck
}

dst := make(map[string]any)
for k, v := range jsh.GetProperties() {
for k, v := range props {
dst[k] = mapJSHandle(vu, v)
}
return rt.ToValue(dst).ToObject(rt)
return dst, nil
},
"getProperty": func(propertyName string) *goja.Object {
var (
Expand Down
12 changes: 8 additions & 4 deletions common/element_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -1094,10 +1094,14 @@ func (h *ElementHandle) queryAll(selector string, eval evalFunc) ([]api.ElementH
return nil, fmt.Errorf("getting element handle for selector %q: %w", selector, ErrJSHandleInvalid)
}
defer handles.Dispose()
var (
props = handles.GetProperties()
els = make([]api.ElementHandle, 0, len(props))
)

props, err := handles.GetProperties()
if err != nil {
// GetProperties has a rich error already, so we don't need to wrap it.
return nil, err //nolint:wrapcheck
}

els := make([]api.ElementHandle, 0, len(props))
for _, prop := range props {
if el := prop.AsElement(); el != nil {
els = append(els, el)
Expand Down
26 changes: 19 additions & 7 deletions common/element_handle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ func TestQueryAll(t *testing.T) {
handles := &jsHandleStub{
asElementFn: nilHandle,
}
handles.getPropertiesFn = func() map[string]api.JSHandle {
handles.getPropertiesFn = func() (map[string]api.JSHandle, error) {
return map[string]api.JSHandle{
"1": handles,
"2": handles,
}
}, nil
}

return handles
Expand All @@ -105,13 +105,25 @@ func TestQueryAll(t *testing.T) {
"3": &jsHandleStub{asElementFn: nilHandle},
}
return &jsHandleStub{
getPropertiesFn: func() map[string]api.JSHandle {
return childHandles
getPropertiesFn: func() (map[string]api.JSHandle, error) {
return childHandles, nil
},
}
},
wantHandles: 2,
},
"returns_err": {
selector: "*",
returnHandle: func() any {
return &jsHandleStub{
getPropertiesFn: func() (map[string]api.JSHandle, error) {
return nil, errors.New("any error")
},
}
},
wantHandles: 0,
wantErr: true,
},
} {
name, tt := name, tt

Expand Down Expand Up @@ -172,7 +184,7 @@ type jsHandleStub struct {
disposeCalls int

asElementFn func() api.ElementHandle
getPropertiesFn func() map[string]api.JSHandle
getPropertiesFn func() (map[string]api.JSHandle, error)
}

func (s *jsHandleStub) AsElement() api.ElementHandle {
Expand All @@ -186,9 +198,9 @@ func (s *jsHandleStub) Dispose() {
s.disposeCalls++
}

func (s *jsHandleStub) GetProperties() map[string]api.JSHandle {
func (s *jsHandleStub) GetProperties() (map[string]api.JSHandle, error) {
if s.getPropertiesFn == nil {
return nil
return nil, nil //nolint:nilnil
}
return s.getPropertiesFn()
}
6 changes: 3 additions & 3 deletions common/js_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,18 @@ func (h *BaseJSHandle) EvaluateHandle(pageFunc goja.Value, args ...goja.Value) (
}

// GetProperties retreives the JS handle's properties.
func (h *BaseJSHandle) GetProperties() map[string]api.JSHandle {
func (h *BaseJSHandle) GetProperties() (map[string]api.JSHandle, error) {
handles, err := h.getProperties()
if err != nil {
k6ext.Panic(h.ctx, "getProperties: %w", err)
return nil, err
}

jsHandles := make(map[string]api.JSHandle, len(handles))
for k, v := range handles {
jsHandles[k] = v
}

return jsHandles
return jsHandles, nil
}

// getProperties is like GetProperties, but does not panic.
Expand Down
4 changes: 3 additions & 1 deletion tests/js_handle_get_properties_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ func TestJSHandleGetProperties(t *testing.T) {
`))
require.NoError(t, err, "expected no error when evaluating handle")

props := handle.GetProperties()
props, err := handle.GetProperties()
require.NoError(t, err, "expected no error when getting properties")

value := props["prop1"].JSONValue().String()
assert.Equal(t, value, "one", `expected property value of "one", got %q`, value)
}

0 comments on commit 95a0d8b

Please sign in to comment.