Poll() should report ErrPollingTimeout correctly no matter what "res" is

Fixes #1087

call.go

@@ -23,44 +23,49 @@ type CallAction Action
 // Note: any exception encountered will be returned as an error.
 func CallFunctionOn(functionDeclaration string, res interface{}, opt CallOption, args ...interface{}) CallAction {
 	return ActionFunc(func(ctx context.Context) error {
-		// set up parameters
-		p := runtime.CallFunctionOn(functionDeclaration).
-			WithSilent(true)
+		_, err := callFunctionOn(ctx, functionDeclaration, res, opt, args...)
+		return err
+	})
+}
 
-		switch res.(type) {
-		case nil, **runtime.RemoteObject:
-		default:
-			p = p.WithReturnByValue(true)
-		}
+func callFunctionOn(ctx context.Context, functionDeclaration string, res interface{}, opt CallOption, args ...interface{}) (bool, error) {
+	// set up parameters
+	p := runtime.CallFunctionOn(functionDeclaration).
+		WithSilent(true)
 
-		// apply opt
-		if opt != nil {
-			p = opt(p)
-		}
+	switch res.(type) {
+	case **runtime.RemoteObject:
+	default:
+		p = p.WithReturnByValue(true)
+	}
 
-		// arguments
-		if len(args) > 0 {
-			ea := &errAppender{args: make([]*runtime.CallArgument, 0, len(args))}
-			for _, arg := range args {
-				ea.append(arg)
-			}
-			if ea.err != nil {
-				return ea.err
-			}
-			p = p.WithArguments(ea.args)
-		}
+	// apply opt
+	if opt != nil {
+		p = opt(p)
+	}
 
-		// call
-		v, exp, err := p.Do(ctx)
-		if err != nil {
-			return err
+	// arguments
+	if len(args) > 0 {
+		ea := &errAppender{args: make([]*runtime.CallArgument, 0, len(args))}
+		for _, arg := range args {
+			ea.append(arg)
 		}
-		if exp != nil {
-			return exp
+		if ea.err != nil {
+			return false, ea.err
 		}
+		p = p.WithArguments(ea.args)
+	}
 
-		return parseRemoteObject(v, res)
-	})
+	// call
+	v, exp, err := p.Do(ctx)
+	if err != nil {
+		return false, err
+	}
+	if exp != nil {
+		return false, exp
+	}
+
+	return parseRemoteObject(v, res)
 }
 
 // CallOption is a function to modify the runtime.CallFunctionOnParams

eval.go

@@ -58,34 +58,41 @@ func Evaluate(expression string, res interface{}, opts ...EvaluateOption) Evalua
 			return exp
 		}
 
-		return parseRemoteObject(v, res)
+		_, err = parseRemoteObject(v, res)
+		return err
 	})
 }
 
-func parseRemoteObject(v *runtime.RemoteObject, res interface{}) error {
+func parseRemoteObject(v *runtime.RemoteObject, res interface{}) (undefined bool, err error) {
+	// undefined indicates that the result is a javascript "undefined" value.
+	// Poll needs this value to decide whether it's a timeout.
+	undefined = v.Type == "undefined"
+
 	if res == nil {
-		return nil
+		return
 	}
 
 	switch x := res.(type) {
 	case **runtime.RemoteObject:
 		*x = v
-		return nil
+		return
 
 	case *[]byte:
 		*x = v.Value
-		return nil
+		return
 	}
 
-	if v.Type == "undefined" {
+	if undefined {
 		// The unmarshal below would fail with the cryptic
 		// "unexpected end of JSON input" error, so try to give
 		// a better one here.
-		return fmt.Errorf("encountered an undefined value")
+		err = fmt.Errorf("encountered an undefined value")
+		return
 	}
 
 	// unmarshal
-	return json.Unmarshal(v.Value, res)
+	err = json.Unmarshal(v.Value, res)
+	return
 }
 
 // EvaluateAsDevTools is an action that evaluates a Javascript expression as

poll.go

@@ -66,17 +66,16 @@ func (p *pollTask) Do(ctx context.Context) error {
 	args = append(args, p.timeout.Milliseconds())
 	args = append(args, p.args...)
 
-	err := CallFunctionOn(waitForPredicatePageFunction, p.res,
+	undefined, err := callFunctionOn(ctx, waitForPredicatePageFunction, p.res,
 		func(p *runtime.CallFunctionOnParams) *runtime.CallFunctionOnParams {
 			return p.WithExecutionContextID(execCtx).
 				WithAwaitPromise(true).
 				WithUserGesture(true)
 		},
 		args...,
-	).Do(ctx)
+	)
 
-	// FIXME: sentinel error?
-	if err != nil && err.Error() == "encountered an undefined value" {
+	if undefined {
 		return ErrPollingTimeout
 	}
 

poll_test.go

@@ -216,3 +216,24 @@ func TestPollRemoteObject(t *testing.T) {
 		t.Fatalf("want class name of remote object to be %q, got %q", wantClassName, res.ClassName)
 	}
 }
+
+func TestPollTimeout(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := testAllocate(t, "poll.html")
+	defer cancel()
+
+	if err := Run(ctx, Poll("false", nil, WithPollingTimeout(10*time.Millisecond))); err != ErrPollingTimeout {
+		t.Errorf("got error: %v, want error: %v", err, ErrPollingTimeout)
+	}
+
+	var res1 *runtime.RemoteObject
+	if err := Run(ctx, Poll("false", &res1, WithPollingTimeout(10*time.Millisecond))); err != ErrPollingTimeout {
+		t.Errorf("got error: %v, want error: %v", err, ErrPollingTimeout)
+	}
+
+	var res2 []byte
+	if err := Run(ctx, Poll("false", &res2, WithPollingTimeout(10*time.Millisecond))); err != ErrPollingTimeout {
+		t.Errorf("got error: %v, want error: %v", err, ErrPollingTimeout)
+	}
+}