compatible object null resolves to go type

[xueyc1f]

I tracked the CDP log and found that when js expression return is null, then runtime.RemoteObject.Type is object and runtime.RemoteObject.Value is nil, so I added a judgment and returned in advance
#1259

[ZekeLu]

@xueyc1f Thank you for the contribution!

I just sent #1261. Which I think handles the situation more correctly. Can you review it and give it a try?

[xueyc1f]

@ZekeLu I don't think this problem can be solved perfectly at present. If res type is int or string and other basic types, it is correct at present, but if res type is map and other types, it will still return an error encountered a null value

The code of:

var res = map[string]interface{}{}
err := chromedp.CallFunctionOn(`(data)=>{console.log(data); return null}`, &res, optFunc, map[string]interface{}{"a": 1, "b": "b", "c": true})
// err is encountered a null value
// if var res = &map[string]interface{}{}, CallFunctionOn is correct.
err = json.Unmarshal([]byte("null"), &res)
// err is null, so CallFunctionOn hould be handled like json.Unmarshal

[ZekeLu]

You are correct. A chan, func, interface, map, pointer, and slice value could be nil. I think we should handle map and slice too. Would you like to send a fix?
Thank you!

[xueyc1f]

Sure, I can submit fix later

[xueyc1f]

Hi @ZekeLu , I tested that json.Unmarshal supports such parsing, so I think parseRemoteObject should also provide similar compatibility. If v.Value is not null, the error is returned by json.Unmarshal

var i chan struct{}
err := json.Unmarshal([]byte(`null`), &i) // err is nil

err := json.Unmarshal([]byte(`{}`), &i) // err is json: cannot unmarshal object into Go value of type chan struct {}

[ZekeLu]

var i int
err := Run(ctx, Evaluate("null", i))

After this change, the err in the above code will be ErrJSNull, but I think it's better to report it as json: Unmarshal(non-pointer int).

If we just need to handle the undefined and null cases, the code could simply be:

value := v.Value
if value == nil {
	// Change the value to the json literal null to make json.Unmarshal happy.
	value = []byte("null")
}

return json.Unmarshal(value, res)

But for the following code,

var i int
err := Run(ctx, Evaluate("undefined", &i))

We want the error to be ErrJSUndefined. That means, if I write such code, I would expect the js is evaluated to an integer. If not, it should report an error. See this test:

chromedp/query_test.go

Lines 595 to 608 in 2c1b719

	func TestValueUndefined(t *testing.T) {
	t.Parallel()
	ctx, cancel := testAllocate(t, "form.html")
	defer cancel()
	var value string
	err := Run(ctx, Value("foo", &value, ByID))
	want := `could not retrieve attribute "value": encountered an undefined value`
	got := fmt.Sprint(err)
	if !strings.Contains(got, want) {
	t.Fatalf("want error %q, got %q", want, got)
	}
	}

So I would suggest to change it like this:

value := v.Value

if value == nil {
	rv := reflect.ValueOf(res)
	if rv.Kind() == reflect.Pointer {
		switch rv.Elem().Kind() {
		// Common kinds that can be nil.
		case reflect.Pointer, reflect.Map, reflect.Slice:
		// It's weird that res is a pointer to the following kinds,
		// but they can be nil too.
		case reflect.Chan, reflect.Func, reflect.Interface:
		default:
			// When the value that `res` points to can not be set to nil,
			// return [ErrJSUndefined] or [ErrJSNull] respectively.
			if v.Type == "undefined" {
				return ErrJSUndefined
			}
			return ErrJSNull
		}
	}

	// Change the value to the json literal null to make json.Unmarshal happy.
	value = []byte("null")
}

return json.Unmarshal(value, res)

[xueyc1f]

@ZekeLu Based on your suggestions and the defect of go json: Unmarshal null [golang/go/issues/33835], I summarized the following processing of parseRemoteObject to go type:

• If js expression is undefined, it cannot resolve to any go type, so return ErrJSUndefined directly
• If js expression is null, it can only resolve to interface, map, ptr, slice type. If other types such as array, string and other zero values cannot be nil, and chan, func cannot be serialized to json, return ErrJSNull
  Do you have any other supplements or suggestions?

[ZekeLu]

I would consider this problem from the perspective of the client of chromedp.

1. If the user code is like this:

var i int
err := chromedp.Run(ctx, chromedp.Evaluate("any js expression", i))

It's a mistake and we should let the user know. The json package can report such kind of error. (json: Unmarshal(non-pointer int). That's perfect. So let the json package do the job.

2. If the user code is like this:

var i int
err := chromedp.Run(ctx, chromedp.Evaluate("js expression that will never evaluate to undefined/null", &i))

I would assume that the user don't expect undefined/null at all. If it happens, we should report ErrJSUndefined or ErrJSNull to the user. If the user thinks that it's fine to receive undefined/null, then the user should write the code like this:

var i *int
err := chromedp.Run(ctx, chromedp.Evaluate("js expression that evaluates to integer/undefined/null", &i))

3. If it doesn't work at all, the user can get the remote object and handle it himself/herself:

var res *runtime.RemoteObject
err := chromedp.Run(ctx, chromedp.Evaluate("any js expression", &res))

[xueyc1f]

I sent a new submission. I think it seems to meet your above settings. If have any questions, please let me know

[ZekeLu]

Please don't forget to update the doc comment for Evaluate:

chromedp/eval.go

Lines 15 to 34 in 2c1b719

	// Evaluate is an action to evaluate the JavaScript expression, unmarshaling
	// the result of the script evaluation to res.
	//
	// When res is nil, the script result will be ignored.
	//
	// When res is a *[]byte, the raw JSON-encoded value of the script
	// result will be placed in res.
	//
	// When res is a **runtime.RemoteObject, res will be set to the low-level
	// protocol type, and no attempt will be made to convert the result.
	// The original objects could be maintained in memory until the page is
	// navigated or closed. `runtime.ReleaseObject` or `runtime.ReleaseObjectGroup`
	// can be used to ask the browser to release the original objects.
	//
	// For all other cases, the result of the script will be returned "by value" (i.e.,
	// JSON-encoded), and subsequently an attempt will be made to json.Unmarshal
	// the script result to res. When the script result is "undefined" or "null",
	// and the value that res points to is not a pointer, it returns [ErrJSUndefined]
	// of [ErrJSNull] respectively.
	func Evaluate(expression string, res interface{}, opts ...EvaluateOption) EvaluateAction {

[ZekeLu]

Please take a look at my suggested changes. Thanks!

[xueyc1f]

Thanks for your suggestion, it's better