Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Ensure Safe Handler Looping inApplication.process_update/error#4802

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Bibo-Joshi wants to merge10 commits intomaster
base:master
Choose a base branch
Loading
fromsafe-handler-loop
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletionschanges/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
bugfixes = """
Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``.

.. hint::
Calling ``Application.add/remove_handler`` now has no influence on calls to :meth:`process_update` that are
already in progress. The same holds for ``Application.add/remove_error_handler`` and ``Application.process_error``, respectively.

.. warning::
This behavior should currently be considered an implementation detail and not as guaranteed behavior.
"""
[[pull_requests]]
uid = "4802"
author_uid = "Bibo-Joshi"
closes_threads = ["4803"]
50 changes: 45 additions & 5 deletionssrc/telegram/ext/_application.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1256,8 +1256,14 @@ async def process_update(self, update: object) -> None:
context = None
any_blocking = False # Flag which is set to True if any handler specifies block=True

for handlers in self.handlers.values():
# We copy the lists to avoid issues with concurrent modification of the
# handlers (groups or handlers in groups) while iterating over it via add/remove_handler.
# Currently considered implementation detail as described in docstrings of
# add/remove_handler
# do *not* use `copy.deepcopy` here, as we don't want to deepcopy the handlers themselves
for handlers in [v.copy() for v in self.handlers.values()]:
try:
# no copy needed b/c we copy above
for handler in handlers:
check = handler.check_update(update) # Should the handler handle this update?
if check is None or check is False:
Expand DownExpand Up@@ -1343,6 +1349,14 @@ def add_handler(self, handler: BaseHandler[Any, CCT, Any], group: int = DEFAULT_
might lead to race conditions and undesired behavior. In particular, current
conversation states may be overridden by the loaded data.

Hint:
This method currently has no influence on calls to :meth:`process_update` that are
already in progress.

.. warning::
This behavior should currently be considered an implementation detail and not as
guaranteed behavior.

Args:
handler (:class:`telegram.ext.BaseHandler`): A BaseHandler instance.
group (:obj:`int`, optional): The group identifier. Default is ``0``.
Expand DownExpand Up@@ -1444,6 +1458,14 @@ def remove_handler(
) -> None:
"""Remove a handler from the specified group.

Hint:
This method currently has no influence on calls to :meth:`process_update` that are
already in progress.

.. warning::
This behavior should currently be considered an implementation detail and not as
guaranteed behavior.

Args:
handler (:class:`telegram.ext.BaseHandler`): A :class:`telegram.ext.BaseHandler`
instance.
Expand DownExpand Up@@ -1774,6 +1796,14 @@ def add_error_handler(
Examples:
:any:`Errorhandler Bot <examples.errorhandlerbot>`

Hint:
This method currently has no influence on calls to :meth:`process_error` that are
already in progress.

.. warning::
This behavior should currently be considered an implementation detail and not as
guaranteed behavior.

.. seealso:: :wiki:`Exceptions, Warnings and Logging <Exceptions%2C-Warnings-and-Logging>`

Args:
Expand All@@ -1797,6 +1827,14 @@ async def callback(update: Optional[object], context: CallbackContext)
def remove_error_handler(self, callback: HandlerCallback[object, CCT, None]) -> None:
"""Removes an error handler.

Hint:
This method currently has no influence on calls to :meth:`process_error` that are
already in progress.

.. warning::
This behavior should currently be considered an implementation detail and not as
guaranteed behavior.

Args:
callback (:term:`coroutine function`): The error handler to remove.

Expand DownExpand Up@@ -1838,10 +1876,12 @@ async def process_error(
:class:`telegram.ext.ApplicationHandlerStop`. :obj:`False`, otherwise.
"""
if self.error_handlers:
for (
callback,
block,
) in self.error_handlers.items():
# We copy the list to avoid issues with concurrent modification of the
# error handlers while iterating over it via add/remove_error_handler.
# Currently considered implementation detail as described in docstrings of
# add/remove_error_handler
error_handler_items = list(self.error_handlers.items())
for callback, block in error_handler_items:
try:
context = self.context_types.context.from_error(
update=update,
Expand Down
89 changes: 89 additions & 0 deletionstests/ext/test_application.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2583,6 +2583,70 @@ async def callback(update, context):
assert received_updates == {2}
assert len(caplog.records) == 0

@pytest.mark.parametrize("change_type", ["remove", "add"])
async def test_process_update_handler_change_groups_during_iteration(self, app, change_type):
run_groups = set()

async def dummy_callback(_, __, g: int):
run_groups.add(g)

for group in range(10, 20):
handler = TypeHandler(int, functools.partial(dummy_callback, g=group))
app.add_handler(handler, group=group)

async def wait_callback(_, context):
# Trigger a change of the app.handlers dict during the iteration
if change_type == "remove":
context.application.remove_handler(handler, group)
else:
context.application.add_handler(
TypeHandler(int, functools.partial(dummy_callback, g=42)), group=42
)

app.add_handler(TypeHandler(int, wait_callback))

async with app:
await app.process_update(1)

# check that exactly those handlers were called that were configured when
# process_update was called
assert run_groups == set(range(10, 20))

async def test_process_update_handler_change_group_during_iteration(self, app):
async def dummy_callback(_, __):
pass

checked_handlers = set()

class TrackHandler(TypeHandler):
def __init__(self, name: str, *args, **kwargs):
self.name = name
super().__init__(*args, **kwargs)

def check_update(self, update: object) -> bool:
checked_handlers.add(self.name)
return super().check_update(update)

remove_handler = TrackHandler("remove", int, dummy_callback)
add_handler = TrackHandler("add", int, dummy_callback)

class TriggerHandler(TypeHandler):
def check_update(self, update: object) -> bool:
# Trigger a change of the app.handlers *in the same group* during the iteration
app.remove_handler(remove_handler)
app.add_handler(add_handler)
# return False to ensure that additional handlers in the same group are checked
return False

app.add_handler(TriggerHandler(str, dummy_callback))
app.add_handler(remove_handler)
async with app:
await app.process_update("string update")

# check that exactly those handlers were checked that were configured when
# process_update was called
assert checked_handlers == {"remove"}

async def test_process_error_exception_in_building_context(self, monkeypatch, caplog, app):
# Makes sure that exceptions in building the context don't stop the application
exception = ValueError("TestException")
Expand DownExpand Up@@ -2622,3 +2686,28 @@ async def callback(update, context):

assert received_errors == {2}
assert len(caplog.records) == 0

@pytest.mark.parametrize("change_type", ["remove", "add"])
async def test_process_error_change_during_iteration(self, app, change_type):
called_handlers = set()

async def dummy_process_error(name: str, *_, **__):
called_handlers.add(name)

add_error_handler = functools.partial(dummy_process_error, "add_handler")
remove_error_handler = functools.partial(dummy_process_error, "remove_handler")

async def trigger_change(*_, **__):
if change_type == "remove":
app.remove_error_handler(remove_error_handler)
else:
app.add_error_handler(add_error_handler)

app.add_error_handler(trigger_change)
app.add_error_handler(remove_error_handler)
async with app:
await app.process_error(update=None, error=None)

# check that exactly those handlers were checked that were configured when
# add_error_handler was called
assert called_handlers == {"remove_handler"}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp