__getattr__ will cause Exception "RecursionError" in android_device.py

[johnklee]

From android_device.py at line 1,114:

  def __getattr__(self, name):
    """Tries to return a snippet client registered with `name`.
    This is for backward compatibility of direct accessing snippet clients.
    """
    client = self.services.snippets.get_snippet_client(name)
    if client:
      return client
    return self.__getattribute__(name)

it will cause exception RecursionError when self.services does not exist for somehow and we try to access an attribute not exist in android device object. Take below sample code for example:

import sys

class Test:
  def get_client(self):
    return ""

class MyObj:
  def __init__(self):
    self.exist_attr = 'test'

  def __getattr__(self, name):
    print(f'Get attr {name}')
    client = self.test.get_client()
    if client:
      return client

    #raise AttributeError(f'{name} not exist')
    return self.__getattribute__(name)

sys.setrecursionlimit(100)
my_obj = MyObj()
print(my_obj.exist_attr)  # print 'test'
# my_obj.test = Test()  <-- Uncomment this line will avoid RecursionError
print(hasattr(my_obj, 'notexist_attr'))  # Cause RecursionError: maximum recursion depth exceeded while calling a Python object

So if we execute above test script:

$ python test_hasattr.py
test
Get attr notexist_attr
Get attr test
...
Traceback (most recent call last):
  File "/tmp/test_hasattr.py", line 27, in <module>
    print(hasattr(my_obj, 'notexist_attr'))  # Cause RecursionError: maximum recursion depth exceeded while calling a Python object
  File "/tmp/test_hasattr.py", line 15, in __getattr__
    client = self.test.get_client()
  File "/tmp/test_hasattr.py", line 15, in __getattr__
    client = self.test.get_client()
  File "/tmp/test_hasattr.py", line 15, in __getattr__
    client = self.test.get_client()
  [Previous line repeated 91 more times]
  File "/tmp/test_hasattr.py", line 14, in __getattr__
    print(f'Get attr {name}')
RecursionError: maximum recursion depth exceeded while calling a Python object

So the proposed implementation for getattr is:

  def __getattr__(self, name):
    """Tries to return a snippet client registered with `name`.
    This is for backward compatibility of direct accessing snippet clients.
    """
    if name != 'services':
      client = self.services.snippets.get_snippet_client(name)
      if client:
        return client

    raise AttributeError(f'{name} not exist')

[xpconanfan]

This impl is intentional as the docstring explained.

[johnklee]

Thanks for the reply. I understand the docstring of __getattr__:

"""Tries to return a snippet client registered with `name`.
    This is for backward compatibility of direct accessing snippet clients.
    """

however, I did observe RecursionError from myside and it is happened when self.services is not available. But I do observe that self.services has been initialized in constructor of class AndroidDevice. So the issue here is trying to avoid RecursionError which it is hard for debugging and it will be a good idea to improve it by using exception for more information. FYI

[xpconanfan]

Sure, but the proposed solution breaks existing behavior. So we'd rather bite this bullet than having it not working at all.

[johnklee]

Understood! Thanks for the response. The issue is confirmed to be the misuse of class AndroidDevice from our implementation. Normally it won't happen or encounter by other users. So feel free to close this issue and thanks for your patience and time here. By the way, the proposed solution is optional and it can be improved to both throw more informative exception and keep existing behavior if we want. Thanks!