-
Notifications
You must be signed in to change notification settings - Fork 327
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.
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
Handle case where GET_LOG is not supported #354
Conversation
In the interim, if you're running into this issue, you can add the following to your protected function storeConsoleLogsFor($browsers)
{
try {
parent::storeConsoleLogsFor($browsers);
} catch (UnknownServerException $exception) {
// Some drivers do not support the GET_LOG command
}
} |
Hard to believe this is a Dusk problem. Seems more like something that needs to be handled on Facebook WebDriver if the browser truly don't offer the HttpCommand. |
Technically the the get log command isn't part of the web driver spec. It's only supported by Chrome. See w3c/webdriver#406 |
Another way to handle this is to only try to get logs if we're using a Chrome driver, since it's a Chrome-specific feature. But that means that Dusk would need to be updated when other browsers add support (which should happen at some time). |
We should only try when using Chrome. Can you re-submit with that? |
If we only implement this for Chrome, it means, for instance, anybody using PhantomJS with Dusk loses the ability to retrieve console. Selenium has decided on offering getLog API a long time ago and Dusk relies on Facebook WebDriver which relies on Selenium. The fact that Firefox haven't decided on the spec for this yet should not be a reason to only support this on Chrome. Again, it's not a Dusk problem. Dusk is just a wrapper on Facebook WebDriver for you and if you use Facebook WebDriver and Firefox, you're gonna get the same error. An alternative, imo, would be to have a on/off flag for Firefox users to be able to disable console. |
@deleugpn It's not Firefox-only. Chrome is the only browser that supports the get log command. It looks like it was in the early spec, but was removed for some reason. I've been running Dusk tests on BrowserStack and ran into this on everything I tried it on. I think #355 is a good temporary fix, but I don't know that it addresses the underlying issue, which is that this is a Chrome-only feature. @taylorotwell How about disabling if we're not using Chrome, but giving an option to override that? |
What about PhantomJS? What about any Selenium compatible browser? |
@deleugpn how about my latest commit? It looks like Webkit supports the get log command, which means that PhantomJS supports it as well. I'm not sure if a whitelist of a blacklist is better—my reading on the topic suggests that it's not supported by most browsers, but I haven't tested them all. |
Many drivers (Firefox, MSIE, etc) do not support retrieving console logs (the GET_LOG) command, resulting in:
This catches that error and writes an error message to the log file instead. This would only affect people using alternate drivers.