-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix: exit with error on retry give up #1151
Conversation
@vojtajina take a look at this PR please |
@j0tunn 🆗 👍 |
When browser not responding karma just logs "ERROR [launcher]: <browser> failed 2 times (timeout). Giving up." and hangs. We should exit with error in such case.
@vojtajina Could you please review and merge this pr? |
@@ -106,7 +110,7 @@ var BaseLauncher = function(id, emitter) { | |||
this.emit('done'); | |||
|
|||
if (this.error && this.state !== BEING_FORCE_KILLED && this.state !== RESTARTING) { | |||
emitter.emit('browser_process_failure', this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we get into this place when self.retryLimit equal 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question... Seems like because nobody calls self._done
. In our case karma-webdriver-launcher
just calls self.emit('done')
and skips this code. Looks like we need to fix karma-webdriver-launcher
.
Yep, not a karma issue but karma-webdriver-launcher, so I'm closing PR. |
@j0tunn no problems! Thanks! |
karma-webdriver-launcher related PR: karma-runner/karma-webdriver-launcher#15 |
When browser not responding karma just logs
ERROR [launcher]: <browser> failed 2 times (timeout). Giving up.
and hangs.We should exit with error in such case.