Fix crash introduced in Python 3.8 in _AttributeInjection. #78
Conversation
Problem: getattr() is now called on all class attributes when building a Mock that has a `spec` provided. This triggers a crash in _AttributeInjection because Mocks are typically created before the configuration process has completed. Solution: Stop raising InjectionError when getattr is called on an attribute, and instead return a sentinel type that represents a null value. This allows reflective logic to complete without issue.
| except InjectorException: | ||
| return _NullInjectedAttribute() | ||
|
|
||
|
|
||
| class _AttributeInjection(object): |
There was a problem hiding this comment.
@andrewborba10 I was curious to see your changes, and it made me wonder one thing.
You added a new _AttributeInjection class like below:
class _NullInjectedAttribute(object):
"""
Returned for injected attributes that don't have an injection configuration.
"""
pass
class _AttributeInjection(object):
def __init__(self, cls):
self._cls = cls
def __get__(self, obj, owner):
# Some Python internals (starting in 3.8 specifically) may call
# getattr() for a class's attributes to do some reflective inspection.
# That triggers this codepath, potentially before inject.configure()
# has been called, which is why we should silently return a sentinel
# type representing a null value instead of raising.
try:
return instance(self._cls)
except InjectorException:
return _NullInjectedAttribute()
It seems to me that now we should delete this _AttributeInjection class that does not return the sentinel value. And maybe we could also add the type hints to the new class.
Am I missing something?
There was a problem hiding this comment.
That was definitely unintentional, I meant to modify the existing class. It sounds like Ivan might have some other changes in mind, so I'll let him apply those with the fix to remove the old class.
|
Hi! I'm a little busy right now. I'll try to take a look this week. |
|
@ivankorobkov Hi. Any progress in looking? 😀 |
|
@fadedDexofan Hi, I'm sorry guys, I've been really busy for the last couple of months. The PR does not look good to me, so I just can't merge it without some changes. I'll try to look into it today or tomorrow. |
|
@ivankorobkov any news on this? |
Problem: getattr() is now called on all class attributes when
building a Mock that has a
specprovided. This triggers acrash in _AttributeInjection because Mocks are typically created
before the configuration process has completed.
Solution: Stop raising InjectionError when getattr is called on
an attribute, and instead return a sentinel type that represents
a null value. This allows reflective logic to complete without
issue.