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

[core]: Adds support for on_error to abort further event handlers on exceptions #220

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

Conversation

prologic
Copy link
Member

Closes #128

@codecov-io
Copy link

codecov-io commented Jan 31, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@1b69122). Click here to learn what that means.
The diff coverage is 61.11%.

@@           Coverage Diff            @@
##             master    #220   +/-   ##
========================================
  Coverage          ?   77.1%           
========================================
  Files             ?      80           
  Lines             ?    6700           
  Branches          ?       0           
========================================
  Hits              ?    5166           
  Misses            ?    1534           
  Partials          ?       0
Impacted Files Coverage Δ
circuits/core/handlers.py 100% <100%> (ø)
circuits/core/events.py 96.9% <100%> (ø)
circuits/core/manager.py 92.62% <56.25%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b69122...cc19ac1. Read the comment docs.

Copy link
Contributor

@spaceone spaceone left a comment

Choose a reason for hiding this comment

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

I need to have this flag also for the specific handler, is this possible?

circuits/core/events.py Outdated Show resolved Hide resolved
circuits/core/events.py Outdated Show resolved Hide resolved
elif event.on_error == "ignore":
continue
else:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

Since strings are hard to handle (prone to typos), I think it would be better to check if on_error is in the allowed options and raise otherwise. What about using constants for those strings?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure about using constants because everybody needs to import them then.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still dislike to use constants because they have to be imported and would make API changes in the future harder.
You also have the same problem with the (in circuits currently widely used) setting of attributes to classes (e.g. handler.event = False), which wouldn't raise any error.

@spaceone
Copy link
Contributor

@prologic Why do you close this and remove the branch?

@prologic
Copy link
Member Author

Ahh I was doing some Github cleanup! Sorry, do you want to take over the branch/PR?

@spaceone
Copy link
Contributor

I can't promise to work on this currently, but I still would need it. I restore the branch to not remove the current working state.

@spaceone spaceone restored the on_error#128 branch October 26, 2020 08:42
@spaceone spaceone reopened this Oct 25, 2021
circuits/core/events.py Outdated Show resolved Hide resolved
prologic and others added 3 commits October 26, 2021 01:07
The following code demonstrates this behavior. As foo.on_error is
'throw_last' the on_error_1 handler is executed as well as on_error_2
which is the exception throwed back to the bar handler.

from circuits import Event, Component, handler, Debugger
import pytest

class foo(Event):
	on_error = 'throw_last'

class bar(Event):
	pass

class Foo(Component):
	@handler('foo', priority=2)
	def _on_error_1(self):
		raise ValueError('test')

	@handler('foo', priority=1)
	def _on_error_2(self):
		raise ValueError('test2')

	def bar(self):
		with pytest.raises(ValueError) as exc:
			x = yield self.call(foo())
		assert exc.value.args[0] == 'test2'
		print('DONE')

x = Foo()
x += Debugger()
x.fire(bar())
x.run()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement stop_on_error in event and handler
4 participants