search

nodeca / navit

Simple client testing from your scripts
MIT License
48 stars 7 forks source link

Question on behavior when .get.evaluate encounters a JavaScript error (in browser scope) #21

Closed MaarekElets closed 9 years ago

MaarekElets commented 9 years ago

I'm running into situations where a navit chain that I'm running inside of an async.eachSeries is failing to call the loop callback (named next in this case). The area where I am sure this is occuring is in a .get.evaluate where the site in question has a javascript error (out of my control in this case) and will fail.

In going over the code, the calls to the loop callback are located in the .run callback function and I see no way that if that function were to run that the next() would not be called. I'm continuing to go over my code to see if I've failed to account for a condition but wanted to check here first to ask if there were any conditions that would cause the .run function to not be called? If the chain terminates prematurely due to a non-falsey return from an evaluation my understanding has been that the run callback would fire with a non-false error parameter (which I then log and move on to the next in the chain). Am I incorrect in this understanding? If needed I can include some of the code, but mostly this is a general question.

puzrin commented 9 years ago

.run callback is not expected to be lost by design. If you have such case and sure it's a navit problem - give a minimal working code example how to reproduce.

MaarekElets commented 9 years ago

That's what I expected. I'm going to tear my code down and see if I can find the exact conditions that cause it to hang, if it ends up being a Navit issue I'll repost it in here. I just needed to eliminate that being in the design of navit before committing the time and effort to the teardown.

puzrin commented 9 years ago

Create a new issue when required. If your code in evaluate can throw error - just wrap it with try/catch. Also you can enable debug output via env if needed.

We don't control browser errors at all. Probably could add timeout if we missed it.

MaarekElets commented 9 years ago

Thanks, will do.

MaarekElets commented 9 years ago

I don't want to open a new issue to deal with this, but I've managed to confirm that the issue I was having with phantomJS hanging is phantomJS related and not due to .get.evaluate.

The issue was actually occurring during the pageWait function in do.js where Phantom is never responding to the .get('loadDone'...) {} call that comes from the bridge queue in node-phantom-simple and thus the callback (where the timeout exceeded is at) is never called causing the whole system to hang.

As this is a PhantomJS issue and it's as good as abandoned now, I'm moving on to SlimerJS (which you had the foresight to patch support in for). I did discover a Windows specific bug in the node-phantom-simple fork Navit is using and put in a PR that resolves it. Now things work great (except for needing to figure out how to make SlimerJS behave headless on Windows but that's likely just a quick scan through the Docs to find).