Skip to content

Commit 105f1cc

Browse files
zeroone2numeral2Bibo-Joshiharshil21
authored
Better Exception-Handling for BasePersistence.replace/insert_bot (python-telegram-bot#2564)
* Catch exceptions raised while copying __dict__/__slots__ in BasePersistence.replace/insert_bot() Also updated the docstrings to reflect the changes in behavior with unexpected errors * Tests: added to CustomClass immutable object that would trigger a setattr() exception * Tests: added new uuid_ property to own CustomClass methods * Updated AUTHORS.rst * Revert "Tests: added new uuid_ property to own CustomClass methods" This reverts commit 9e67463. * Revert "Tests: added to CustomClass immutable object that would trigger a setattr() exception" This reverts commit 1c25830 * Removed unneeded Exception cast to string f-string will perform the string-ification on their own * Removed another unneeded Exception cast to string * Added test to parse unparsable objects in __dict__ or __slots__ * Applied black and pylint style suggestions All lint tests passed * Fix typo Co-authored-by: Harshil <[email protected]> Co-authored-by: Bibo-Joshi <[email protected]> Co-authored-by: Harshil <[email protected]>
1 parent 52ce039 commit 105f1cc

3 files changed

Lines changed: 86 additions & 30 deletions

File tree

‎AUTHORS.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ The following wonderful people contributed directly or indirectly to this projec
106106
- `Vorobjev Simon <https://github.com/simonvorobjev>`_
107107
- `Wagner Macedo <https://github.com/wagnerluis1982>`_
108108
- `wjt <https://github.com/wjt>`_
109+
- `zeroone2numeral2 <https://github.com/zeroone2numeral2>`_
109110
- `zeshuaro <https://github.com/zeshuaro>`_
110111

111112
Please add yourself here alphabetically when you submit your first pull request.

‎telegram/ext/basepersistence.py‎

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ def replace_bot(cls, obj: object) -> object:
212212
:attr:`REPLACED_BOT`. Currently, this handles objects of type ``list``, ``tuple``, ``set``,
213213
``frozenset``, ``dict``, ``defaultdict`` and objects that have a ``__dict__`` or
214214
``__slots__`` attribute, excluding classes and objects that can't be copied with
215-
``copy.copy``.
215+
``copy.copy``. If the parsing of an object fails, the object will be returned unchanged and
216+
the error will be logged.
216217
217218
Args:
218219
obj (:obj:`object`): The object
@@ -278,20 +279,31 @@ def _replace_bot(cls, obj: object, memo: Dict[int, object]) -> object: # pylint
278279
return new_obj
279280
# if '__dict__' in obj.__slots__, we already cover this here, that's why the
280281
# __dict__ case comes below
281-
if hasattr(obj, '__slots__'):
282-
for attr_name in new_obj.__slots__:
283-
setattr(
284-
new_obj,
285-
attr_name,
286-
cls._replace_bot(cls._replace_bot(getattr(new_obj, attr_name), memo), memo),
287-
)
288-
memo[obj_id] = new_obj
289-
return new_obj
290-
if hasattr(obj, '__dict__'):
291-
for attr_name, attr in new_obj.__dict__.items():
292-
setattr(new_obj, attr_name, cls._replace_bot(attr, memo))
293-
memo[obj_id] = new_obj
294-
return new_obj
282+
try:
283+
if hasattr(obj, '__slots__'):
284+
for attr_name in new_obj.__slots__:
285+
setattr(
286+
new_obj,
287+
attr_name,
288+
cls._replace_bot(
289+
cls._replace_bot(getattr(new_obj, attr_name), memo), memo
290+
),
291+
)
292+
memo[obj_id] = new_obj
293+
return new_obj
294+
if hasattr(obj, '__dict__'):
295+
for attr_name, attr in new_obj.__dict__.items():
296+
setattr(new_obj, attr_name, cls._replace_bot(attr, memo))
297+
memo[obj_id] = new_obj
298+
return new_obj
299+
except Exception as exception:
300+
warnings.warn(
301+
f'Parsing of an object failed with the following exception: {exception}. '
302+
f'See the docs of BasePersistence.replace_bot for more information.',
303+
RuntimeWarning,
304+
)
305+
memo[obj_id] = obj
306+
return obj
295307

296308
return obj
297309

@@ -301,7 +313,8 @@ def insert_bot(self, obj: object) -> object:
301313
:attr:`bot`. Currently, this handles objects of type ``list``, ``tuple``, ``set``,
302314
``frozenset``, ``dict``, ``defaultdict`` and objects that have a ``__dict__`` or
303315
``__slots__`` attribute, excluding classes and objects that can't be copied with
304-
``copy.copy``.
316+
``copy.copy``. If the parsing of an object fails, the object will be returned unchanged and
317+
the error will be logged.
305318
306319
Args:
307320
obj (:obj:`object`): The object
@@ -368,20 +381,31 @@ def _insert_bot(self, obj: object, memo: Dict[int, object]) -> object: # pylint
368381
return new_obj
369382
# if '__dict__' in obj.__slots__, we already cover this here, that's why the
370383
# __dict__ case comes below
371-
if hasattr(obj, '__slots__'):
372-
for attr_name in obj.__slots__:
373-
setattr(
374-
new_obj,
375-
attr_name,
376-
self._insert_bot(self._insert_bot(getattr(new_obj, attr_name), memo), memo),
377-
)
378-
memo[obj_id] = new_obj
379-
return new_obj
380-
if hasattr(obj, '__dict__'):
381-
for attr_name, attr in new_obj.__dict__.items():
382-
setattr(new_obj, attr_name, self._insert_bot(attr, memo))
383-
memo[obj_id] = new_obj
384-
return new_obj
384+
try:
385+
if hasattr(obj, '__slots__'):
386+
for attr_name in obj.__slots__:
387+
setattr(
388+
new_obj,
389+
attr_name,
390+
self._insert_bot(
391+
self._insert_bot(getattr(new_obj, attr_name), memo), memo
392+
),
393+
)
394+
memo[obj_id] = new_obj
395+
return new_obj
396+
if hasattr(obj, '__dict__'):
397+
for attr_name, attr in new_obj.__dict__.items():
398+
setattr(new_obj, attr_name, self._insert_bot(attr, memo))
399+
memo[obj_id] = new_obj
400+
return new_obj
401+
except Exception as exception:
402+
warnings.warn(
403+
f'Parsing of an object failed with the following exception: {exception}. '
404+
f'See the docs of BasePersistence.insert_bot for more information.',
405+
RuntimeWarning,
406+
)
407+
memo[obj_id] = obj
408+
return obj
385409

386410
return obj
387411

‎tests/test_persistence.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
# along with this program. If not, see [http://www.gnu.org/licenses/].
1919
import gzip
2020
import signal
21+
import uuid
2122
from threading import Lock
2223

2324
from telegram.ext.callbackdatacache import CallbackDataCache
@@ -689,6 +690,36 @@ def __copy__(self):
689690
"BasePersistence.insert_bot does not handle objects that can not be copied."
690691
)
691692

693+
def test_bot_replace_insert_bot_unparsable_objects(self, bot, bot_persistence, recwarn):
694+
"""Here check that objects in __dict__ or __slots__ that can't
695+
be parsed are just returned verbatim."""
696+
persistence = bot_persistence
697+
persistence.set_bot(bot)
698+
699+
uuid_obj = uuid.uuid4()
700+
701+
persistence.update_bot_data({1: uuid_obj})
702+
assert persistence.bot_data[1] is uuid_obj
703+
persistence.update_chat_data(123, {1: uuid_obj})
704+
assert persistence.chat_data[123][1] is uuid_obj
705+
persistence.update_user_data(123, {1: uuid_obj})
706+
assert persistence.user_data[123][1] is uuid_obj
707+
persistence.update_callback_data(([('1', 2, {0: uuid_obj})], {'1': '2'}))
708+
assert persistence.callback_data[0][0][2][0] is uuid_obj
709+
710+
assert persistence.get_bot_data()[1] is uuid_obj
711+
assert persistence.get_chat_data()[123][1] is uuid_obj
712+
assert persistence.get_user_data()[123][1] is uuid_obj
713+
assert persistence.get_callback_data()[0][0][2][0] is uuid_obj
714+
715+
assert len(recwarn) == 2
716+
assert str(recwarn[0].message).startswith(
717+
"Parsing of an object failed with the following exception: "
718+
)
719+
assert str(recwarn[1].message).startswith(
720+
"Parsing of an object failed with the following exception: "
721+
)
722+
692723
def test_bot_replace_insert_bot_classes(self, bot, bot_persistence, recwarn):
693724
"""Here check that classes are just returned verbatim."""
694725
persistence = bot_persistence

0 commit comments

Comments
 (0)