Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Detect javascript errors when visiting a new web page #185

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Detect javascript errors when visiting a new web page #185

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bacbdf2
Detect javascript errors when visiting a new web page
Nov 23, 2017
5d5199d
Decode the json encoded error string when throwing exception.
May 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/ZombieDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ public function visit($url)
}
});
JS;
$this->server->evalJS($js);
$out = $this->server->evalJS($js);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Are you sure, that JavaScript errors (if any) will be returned as method call result?
  2. Does same error catching technique apply to other js-enabled drivers, like Selenium?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aik099 visiting a URL in Selenium uses a native Selenium method. So we don't need to implement error handling (Selenium does).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aik099 see the JS code just above. On error, it outputs some stuff to the stream.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zombie "browser" object's method "visit" has a callback as its second argument that will be called if an error occurs. Currently this error information is just thrown away. I found it very helpful to have this error information in my project when making behat tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof has been quiet on this. I guess it is ok as it is then?

if (!empty($out)) {
throw new DriverException(sprintf('Error when loading page %s: %s', $url, $out));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right fix would rather be to prefix the error output with ZombieServer::ERROR_PREFIX and then evalJS will throw the exception for us.

Copy link
Author

@TerjeBr TerjeBr Nov 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you will not get the same degree of specialized error message. You will get a more general error message, and you will not readily see the url that it tried to visit.

You can also see that the same coding pattern is applied to the click function at line 719, and also in triggerBrowserEvent at line 890

}
}

/**
Expand Down