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.

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

Wrong defer watcher try / catch #74

Closed
kelunik opened this issue Mar 12, 2017 · 6 comments
Closed

Wrong defer watcher try / catch #74

kelunik opened this issue Mar 12, 2017 · 6 comments
Assignees
Labels

Comments

@kelunik
Copy link
Member

kelunik commented Mar 12, 2017

$result = $callback($watcher->id, $watcher->data);
should have the try / catch around the $callback invocation, not around the foreach.

@kelunik kelunik added the bug label Mar 12, 2017
@trowski
Copy link
Member

trowski commented Mar 13, 2017

That try / catch block catches exceptions thrown from any watcher invocation, not just defers, as it is also around the call to dispatch. Any watcher callback throwing an exception effectively immediately ends the tick. In this way defers are handled the same as any other watcher.

@kelunik
Copy link
Member Author

kelunik commented Mar 13, 2017

It makes handling easier, but it won't execute any I/O watchers if there's always one defer throwing.

@trowski
Copy link
Member

trowski commented Mar 13, 2017

I can't imagine that happening in an actual app.

@kelunik
Copy link
Member Author

kelunik commented Mar 13, 2017

Same here, that'd be an extreme edge case, but it might happen, as we use Loop::defer to rethrow errors from promises.

@bwoebi
Copy link
Member

bwoebi commented Mar 13, 2017

I see no reason why this shouldn't be changed...

@trowski
Copy link
Member

trowski commented Mar 13, 2017

@bwoebi After thinking about this, I realized an app might have a IO, timer, or signal that continually throws that might block another watcher from ever executing that would stop the cycle. I'll update everywhere watcher callbacks are invoked to be wrapped by their own try / catch blocks and continue the tick after calling the loop error handler.

staabm pushed a commit to staabm/amp that referenced this issue Nov 29, 2017
Specify exact guarantees when a callback has to be called
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants