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

Commit 6c57782

Browse files
author
Ivan Mirić
committed
Remove logging from RemoteObject helper functions
Resolves #130 (comment)
1 parent 9c5aaab commit 6c57782

File tree

7 files changed

+87
-51
lines changed

7 files changed

+87
-51
lines changed

‎common/execution_context.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ func (e *ExecutionContext) evaluate(apiCtx context.Context, forceCallable bool,
128128
}
129129
if remoteObject != nil {
130130
if returnByValue {
131-
res, err = valueFromRemoteObject(apiCtx, remoteObject, e.logger.log.WithContext(e.ctx))
131+
res, err = valueFromRemoteObject(apiCtx, remoteObject)
132132
if err != nil {
133133
return nil, fmt.Errorf("unable to extract value from remote object: %w", err)
134134
}
@@ -164,7 +164,7 @@ func (e *ExecutionContext) evaluate(apiCtx context.Context, forceCallable bool,
164164
}
165165
if remoteObject != nil {
166166
if returnByValue {
167-
res, err = valueFromRemoteObject(apiCtx, remoteObject, e.logger.log.WithContext(apiCtx))
167+
res, err = valueFromRemoteObject(apiCtx, remoteObject)
168168
if err != nil {
169169
return nil, fmt.Errorf("unable to extract value from remote object: %w", err)
170170
}

‎common/frame_session.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ package common
2323
import (
2424
"context"
2525
"encoding/json"
26-
"errors"
2726
"fmt"
2827
"strings"
2928
"sync"
@@ -478,15 +477,9 @@ func (fs *FrameSession) onConsoleAPICalled(event *runtime.EventConsoleAPICalled)
478477

479478
var parsedObjects []interface{}
480479
for _, robj := range event.Args {
481-
i, err := parseRemoteObject(robj, l)
480+
i, err := parseRemoteObject(robj)
482481
if err != nil {
483-
var oe *objectOverflowError
484-
if errors.As(err, &oe) {
485-
l.Warn(err)
486-
} else {
487-
// If this throws it's a bug :)
488-
k6Throw(fs.ctx, "unable to parse remote object value: %w", err)
489-
}
482+
handleParseRemoteObjectErr(fs.ctx, err, l)
490483
}
491484
parsedObjects = append(parsedObjects, i)
492485
}

‎common/js_handle.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,13 @@ func (h *BaseJSHandle) JSONValue() goja.Value {
155155
if result, _, err = action.Do(cdp.WithExecutor(h.ctx, h.session)); err != nil {
156156
k6common.Throw(rt, fmt.Errorf("unable to get properties for JS handle %T: %w", action, err))
157157
}
158-
res, err := valueFromRemoteObject(h.ctx, result, h.logger.log.WithContext(h.ctx))
158+
res, err := valueFromRemoteObject(h.ctx, result)
159159
if err != nil {
160160
k6common.Throw(rt, fmt.Errorf("unable to extract value from remote object: %w", err))
161161
}
162162
return res
163163
}
164-
res, err := valueFromRemoteObject(h.ctx, h.remoteObject, h.logger.log.WithContext(h.ctx))
164+
res, err := valueFromRemoteObject(h.ctx, h.remoteObject)
165165
if err != nil {
166166
k6common.Throw(rt, fmt.Errorf("unable to extract value from remote object: %w", err))
167167
}

‎common/remote_object.go

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,15 @@ package common
2323
import (
2424
"context"
2525
"encoding/json"
26+
"errors"
27+
"fmt"
2628
"math"
2729
"strconv"
2830
"strings"
2931

3032
cdpruntime "github.com/chromedp/cdproto/runtime"
3133
"github.com/dop251/goja"
34+
"github.com/hashicorp/go-multierror"
3235
"github.com/sirupsen/logrus"
3336
k6common "go.k6.io/k6/js/common"
3437
)
@@ -40,28 +43,42 @@ func (*objectOverflowError) Error() string {
4043
return "object is too large and will be parsed partially"
4144
}
4245

43-
func parseRemoteObjectPreview(op *cdpruntime.ObjectPreview, logger *logrus.Entry) (map[string]interface{}, error) {
46+
type objectPropertyParseError struct {
47+
error
48+
property string
49+
}
50+
51+
// Error returns the reason of the failure, including the wrapper parsing error
52+
// message.
53+
func (pe *objectPropertyParseError) Error() string {
54+
return fmt.Sprintf("failed parsing object property %q: %s", pe.property, pe.error)
55+
}
56+
57+
// Unwrap returns the wrapped parsing error.
58+
func (pe *objectPropertyParseError) Unwrap() error {
59+
return pe.error
60+
}
61+
62+
func parseRemoteObjectPreview(op *cdpruntime.ObjectPreview) (map[string]interface{}, error) {
4463
obj := make(map[string]interface{})
45-
var err error
64+
var result error
4665
if op.Overflow {
47-
err = &objectOverflowError{}
66+
result = multierror.Append(result, &objectOverflowError{})
4867
}
4968

5069
for _, p := range op.Properties {
51-
val, err := parseRemoteObjectValue(p.Type, p.Value, p.ValuePreview, logger)
70+
val, err := parseRemoteObjectValue(p.Type, p.Value, p.ValuePreview)
5271
if err != nil {
53-
logger.WithError(err).Errorf("failed parsing object property %q", p.Name)
72+
result = multierror.Append(result, &objectPropertyParseError{err, p.Name})
5473
continue
5574
}
5675
obj[p.Name] = val
5776
}
5877

59-
return obj, err
78+
return obj, result
6079
}
6180

62-
func parseRemoteObjectValue(
63-
t cdpruntime.Type, val string, op *cdpruntime.ObjectPreview, logger *logrus.Entry,
64-
) (interface{}, error) {
81+
func parseRemoteObjectValue(t cdpruntime.Type, val string, op *cdpruntime.ObjectPreview) (interface{}, error) {
6582
switch t {
6683
case cdpruntime.TypeAccessor:
6784
return "accessor", nil
@@ -81,7 +98,7 @@ func parseRemoteObjectValue(
8198
return val, nil
8299
case cdpruntime.TypeObject:
83100
if op != nil {
84-
return parseRemoteObjectPreview(op, logger)
101+
return parseRemoteObjectPreview(op)
85102
}
86103
if val == "Object" {
87104
return val, nil
@@ -98,9 +115,9 @@ func parseRemoteObjectValue(
98115
return v, nil
99116
}
100117

101-
func parseRemoteObject(obj *cdpruntime.RemoteObject, logger *logrus.Entry) (interface{}, error) {
118+
func parseRemoteObject(obj *cdpruntime.RemoteObject) (interface{}, error) {
102119
if obj.UnserializableValue == "" {
103-
return parseRemoteObjectValue(obj.Type, string(obj.Value), obj.Preview, logger)
120+
return parseRemoteObjectValue(obj.Type, string(obj.Value), obj.Preview)
104121
}
105122

106123
switch obj.UnserializableValue.String() {
@@ -117,12 +134,33 @@ func parseRemoteObject(obj *cdpruntime.RemoteObject, logger *logrus.Entry) (inte
117134
return nil, UnserializableValueError{obj.UnserializableValue}
118135
}
119136

120-
func valueFromRemoteObject(
121-
ctx context.Context, remoteObject *cdpruntime.RemoteObject, logger *logrus.Entry,
122-
) (goja.Value, error) {
123-
val, err := parseRemoteObject(remoteObject, logger)
137+
func valueFromRemoteObject(ctx context.Context, robj *cdpruntime.RemoteObject) (goja.Value, error) {
138+
val, err := parseRemoteObject(robj)
124139
if val == "undefined" {
125140
return goja.Undefined(), err
126141
}
127142
return k6common.GetRuntime(ctx).ToValue(val), err
128143
}
144+
145+
func handleParseRemoteObjectErr(ctx context.Context, err error, logger *logrus.Entry) {
146+
var (
147+
ooe *objectOverflowError
148+
ope *objectPropertyParseError
149+
)
150+
merr, ok := err.(*multierror.Error)
151+
if !ok {
152+
// If this throws it's a bug :)
153+
k6Throw(ctx, "unable to parse remote object value: %w", err)
154+
}
155+
for _, e := range merr.Errors {
156+
switch {
157+
case errors.As(e, &ooe):
158+
logger.Warn(ooe)
159+
case errors.As(e, &ope):
160+
logger.WithError(ope).Error()
161+
default:
162+
// If this throws it's a bug :)
163+
k6Throw(ctx, "unable to parse remote object value: %w", e)
164+
}
165+
}
166+
}

‎common/remote_object_test.go

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/chromedp/cdproto/runtime"
3030
"github.com/dop251/goja"
3131
"github.com/mailru/easyjson"
32-
"github.com/sirupsen/logrus"
3332
"github.com/stretchr/testify/assert"
3433
"github.com/stretchr/testify/require"
3534
k6common "go.k6.io/k6/js/common"
@@ -47,8 +46,7 @@ func TestValueFromRemoteObject(t *testing.T) {
4746
UnserializableValue: unserializableValue,
4847
}
4948

50-
logger := NewLogger(ctx, logrus.New(), false, nil)
51-
arg, err := valueFromRemoteObject(ctx, remoteObject, logger.log.WithContext(ctx))
49+
arg, err := valueFromRemoteObject(ctx, remoteObject)
5250
require.True(t, goja.IsNull(arg))
5351
require.ErrorIs(t, UnserializableValueError{unserializableValue}, err)
5452
})
@@ -62,8 +60,7 @@ func TestValueFromRemoteObject(t *testing.T) {
6260
UnserializableValue: unserializableValue,
6361
}
6462

65-
logger := NewLogger(ctx, logrus.New(), false, nil)
66-
arg, err := valueFromRemoteObject(ctx, remoteObject, logger.log.WithContext(ctx))
63+
arg, err := valueFromRemoteObject(ctx, remoteObject)
6764

6865
require.True(t, goja.IsNull(arg))
6966
assert.ErrorIs(t, UnserializableValueError{unserializableValue}, err)
@@ -99,8 +96,7 @@ func TestValueFromRemoteObject(t *testing.T) {
9996
Type: "number",
10097
UnserializableValue: runtime.UnserializableValue(v.value),
10198
}
102-
logger := NewLogger(ctx, logrus.New(), false, nil)
103-
arg, err := valueFromRemoteObject(ctx, remoteObject, logger.log.WithContext(ctx))
99+
arg, err := valueFromRemoteObject(ctx, remoteObject)
104100
require.NoError(t, err)
105101
require.NotNil(t, arg)
106102
if v.value == "NaN" {
@@ -149,8 +145,7 @@ func TestValueFromRemoteObject(t *testing.T) {
149145
Value: marshalled,
150146
}
151147

152-
logger := NewLogger(ctx, logrus.New(), false, nil)
153-
arg, err := valueFromRemoteObject(ctx, remoteObject, logger.log.WithContext(ctx))
148+
arg, err := valueFromRemoteObject(ctx, remoteObject)
154149

155150
require.Nil(t, err)
156151
require.Equal(t, p.value, p.toFn(arg))
@@ -172,8 +167,7 @@ func TestValueFromRemoteObject(t *testing.T) {
172167
},
173168
}
174169

175-
logger := NewLogger(ctx, logrus.New(), false, nil)
176-
val, err := valueFromRemoteObject(ctx, remoteObject, logger.log.WithContext(ctx))
170+
val, err := valueFromRemoteObject(ctx, remoteObject)
177171
require.NoError(t, err)
178172
assert.Equal(t, rt.ToValue(map[string]interface{}{"num": float64(1)}), val)
179173
})
@@ -183,15 +177,12 @@ func TestValueFromRemoteObject(t *testing.T) {
183177
func TestParseRemoteObject(t *testing.T) {
184178
t.Parallel()
185179

186-
rt := goja.New()
187-
ctx := k6common.WithRuntime(context.Background(), rt)
188-
189180
testCases := []struct {
190181
name string
191182
preview *runtime.ObjectPreview
192183
value string
193184
expected map[string]interface{}
194-
expErr error
185+
expErr string
195186
}{
196187
{
197188
name: "most_types",
@@ -233,28 +224,37 @@ func TestParseRemoteObject(t *testing.T) {
233224
},
234225
},
235226
{
236-
name: "overflow",
227+
name: "err_overflow",
237228
preview: &runtime.ObjectPreview{Overflow: true},
238229
expected: map[string]interface{}{},
239-
expErr: &objectOverflowError{},
230+
expErr: "object is too large and will be parsed partially",
231+
},
232+
{
233+
name: "err_parsing_property",
234+
preview: &runtime.ObjectPreview{
235+
Properties: []*runtime.PropertyPreview{
236+
{Name: "failprop", Type: runtime.TypeObject, Value: "some"},
237+
},
238+
},
239+
expected: map[string]interface{}{},
240+
expErr: "failed parsing object property",
240241
},
241242
}
242243

243244
for _, tc := range testCases {
244245
tc := tc
245246
t.Run(tc.name, func(t *testing.T) {
247+
t.Parallel()
246248
remoteObject := &runtime.RemoteObject{
247249
Type: "object",
248250
ObjectID: runtime.RemoteObjectID("object_id_0123456789"),
249251
Preview: tc.preview,
250252
Value: easyjson.RawMessage(tc.value),
251253
}
252-
lg := logrus.New()
253-
logger := NewLogger(ctx, lg, false, nil)
254-
val, err := parseRemoteObject(remoteObject, logger.log.WithContext(ctx))
254+
val, err := parseRemoteObject(remoteObject)
255255
assert.Equal(t, tc.expected, val)
256-
if tc.expErr != nil {
257-
assert.ErrorAs(t, err, &tc.expErr)
256+
if tc.expErr != "" {
257+
assert.Contains(t, err.Error(), tc.expErr)
258258
} else {
259259
assert.NoError(t, err)
260260
}

‎go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
github.com/dop251/goja v0.0.0-20211022113120-dc8c55024d06
88
github.com/fatih/color v1.13.0
99
github.com/gorilla/websocket v1.4.2
10+
github.com/hashicorp/go-multierror v1.1.1
1011
github.com/mailru/easyjson v0.7.7
1112
github.com/mccutchen/go-httpbin v1.1.2-0.20190116014521-c5cb2f4802fa
1213
github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c
@@ -29,6 +30,7 @@ require (
2930
github.com/ghodss/yaml v1.0.0 // indirect
3031
github.com/go-sourcemap/sourcemap v2.1.3+incompatible // indirect
3132
github.com/golang/protobuf v1.4.3 // indirect
33+
github.com/hashicorp/errwrap v1.0.0 // indirect
3234
github.com/josharian/intern v1.0.0 // indirect
3335
github.com/klauspost/compress v1.13.6 // indirect
3436
github.com/kubernetes/helm v2.9.0+incompatible // indirect

‎go.sum

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,14 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t
132132
github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
133133
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
134134
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
135+
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
135136
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
136137
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
137138
github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
138139
github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
139140
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
141+
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
142+
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
140143
github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
141144
github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
142145
github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=

0 commit comments

Comments
 (0)