The Wayback Machine - https://web.archive.org/web/20221107203549/https://github.com/python/cpython/issues/99184
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

str(weakref) throws error KeyError: '__name__' if the original obj override __getattr__ #99184

Open
yanboliang opened this issue Nov 7, 2022 · 3 comments
Labels
pending The issue will be closed if no feedback is provided type-bug An unexpected behavior, bug, or error

Comments

@yanboliang
Copy link

yanboliang commented Nov 7, 2022

Bug report

A clear and concise description of what the bug is.
Include a minimal, reproducible example (https://stackoverflow.com/help/minimal-reproducible-example), if possible.

Repo:

class MyConfig(dict):
    def __getattr__(self, x):
        return self[x]

obj = MyConfig(offset=5)
obj_weakref = weakref.ref(obj)
str(obj_weakref)  # raise error: KeyError: '__name__'

Error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in __getattr__
KeyError: '__name__'

Your environment

  • CPython versions tested on: python 3.8 & 3.9
  • Operating system and architecture: Ubuntu Linux and MacOS
@sobolevn
Copy link
Member

sobolevn commented Nov 7, 2022

This does not look like a bug to me.
What do you expect to happen?

Right now weakrefobject.c has these lines:

static PyObject *
weakref_repr(PyWeakReference *self)
{
    PyObject *name, *repr;
    PyObject* obj = PyWeakref_GET_OBJECT(self);

    if (obj == Py_None) {
        return PyUnicode_FromFormat("<weakref at %p; dead>", self);
    }

    Py_INCREF(obj);
    if (_PyObject_LookupAttr(obj, &_Py_ID(__name__), &name) < 0) {
        Py_DECREF(obj);
        return NULL;
    }
    // ...

It tries to get __name__ attribute from the passed object. Since __getattr__ tries to get __name__ from self - it fails with a KeyError. It works exactly as it is written.

To fix this you can do something like:

class MyConfig(dict):
    def __getattr__(self, x):
        if x in {'__name__', '__class__', 'whatever_else'}:
             return getattr(type(self), x)
        return self[x]

@sobolevn sobolevn added the pending The issue will be closed if no feedback is provided label Nov 7, 2022
@yanboliang
Copy link
Author

yanboliang commented Nov 7, 2022

@sobolevn Thanks for your quick response! I do agree this works well in functionality's perspective, but I'd like to say if this is a good user experience. If users are just trying to override __getattr__ to implement their own functionality(like what I'm doing in the example), it means they should handle a lot of other python internal stuff at well. I think this would sort of increase the complexity, and make users can't focus on their own functionality.

@sobolevn
Copy link
Member

sobolevn commented Nov 7, 2022

So, probably you are asking from some kind of fallback for cases like this. Please, correct me if I am wrong :)

I think that repr() of weakref.ref would have a default value if _PyObject_LookupAttr(obj, &_Py_ID(__name__), &name) fails.

Example: <weakref at 0x1085b5800; to __some_default__ at 0x7ffd1071c080 (...)>

However, I don't think this is a good user experience either.
It is better to see an explicit exception early on than very strange and mostly unreadable repr somewhere in logs.

Considering your class, it looks problematic to me. It does not respect basic python's API, like: __dict__, __name__, __qualname__, __class__, __module__, etc.

They are quite basic, many other parts of CPython (and 3rd party projects) do use it.
And you will have a lot of other failure when these fundamentals cannot be accessed 🤔

Do you agree with me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending The issue will be closed if no feedback is provided type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants