Skip to content

gh-153785: Generate AttributeError messages from context#153786

Merged
vstinner merged 48 commits into
python:mainfrom
johnslavik:gh-153785
Jul 24, 2026
Merged

gh-153785: Generate AttributeError messages from context#153786
vstinner merged 48 commits into
python:mainfrom
johnslavik:gh-153785

Conversation

@johnslavik

@johnslavik johnslavik commented Jul 16, 2026

Copy link
Copy Markdown
Member

I think this is the best place to do this. We do the same trick in KeyError.

This also fixed issue #143811 that motivated the current one.

@python-cla-bot

This comment was marked as resolved.

@johnslavik johnslavik changed the title Generate AttributeError messages from context gh-153785: Generate AttributeError messages from context Jul 16, 2026
@read-the-docs-community

read-the-docs-community Bot commented Jul 16, 2026

Copy link
Copy Markdown

@johnslavik
johnslavik requested a review from DinoV July 16, 2026 01:23
@johnslavik
johnslavik marked this pull request as draft July 16, 2026 02:11
@johnslavik
johnslavik marked this pull request as ready for review July 16, 2026 02:21
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c

When possible, :attr:`name` and :attr:`obj` are set automatically.

.. versionchanged:: 3.10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dropped the .. versionchanged:: next note that documented the generated message. This is a user-visible change to str(), so I think we should keep a versionchanged here, no?

@johnslavik johnslavik Jul 19, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@serhiy-storchaka, do you agree on bringing it back?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually do not add versionchanged for minor changes like changed error message. Otherwise our documentation would be overwhelmed by them.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following code writes 'lazy_import' object has no attribute 'missing1': it mentions lazy_import rather than module 'asyncio'. But I don't think that AttributeError_str() would be the right place to reify the asyncio module!

lazy import asyncio

class RaiseWithName:
    def __getattr__(self, name):
        raise AttributeError(name)

try:
    getattr(globals()['asyncio'], "missing1")
except AttributeError as exc:
#    assert exc.obj is ...
    assert exc.name == "missing1"
    print(str(exc))

Comment thread Doc/library/exceptions.rst
Comment thread Objects/exceptions.c
Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated
Comment thread Lib/test/test_exceptions.py
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2026-07-16-02-50-01.gh-issue-153785.fJqPKC.rst Outdated
@jaraco

jaraco commented Jul 18, 2026

Copy link
Copy Markdown
Member

I think this is the best place to do this. We do the same trick in KeyError.

This also fixed issue #143811 that motivated the current one.

* Issue: [Generate `AttributeError` messages from their context #153785](https://github.com/python/cpython/issues/153785)

"this" and "current one" is unclear here. Please update the PR description to describe the motivation. Also consider linking to the code for the trick in KeyError.

@jaraco

jaraco commented Jul 18, 2026

Copy link
Copy Markdown
Member

I think this is the best place to do this. We do the same trick in KeyError.
This also fixed issue #143811 that motivated the current one.

* Issue: [Generate `AttributeError` messages from their context #153785](https://github.com/python/cpython/issues/153785)

"this" and "current one" is unclear here. Please update the PR description to describe the motivation. Also consider linking to the code for the trick in KeyError.

I used Claude to infer the motivations and update the description.

@jaraco

jaraco commented Jul 18, 2026

Copy link
Copy Markdown
Member

It is the same trick KeyError already uses to wrap its key in repr() — see KeyError_str.

This trick has been applied to KeyError and AttributeError; should it be applied to other exceptions? Should it be implemented in such a way that the concern is shared across exceptions (explicitly or implicitly)?

@jaraco

jaraco commented Jul 18, 2026

Copy link
Copy Markdown
Member

This trick has been applied to KeyError and AttributeError; should it be applied to other exceptions? Should it be implemented in such a way that the concern is shared across exceptions (explicitly or implicitly)?

Short answer, not really.

Details

(Analysis by Claude, on my behalf.)

A worthwhile question. Worth noting first that the two "tricks" only look alike at the surface (both override tp_str); underneath they differ:

  • KeyError_str reformats what's already in args — wrapping args[0] in repr() so {}[''] prints KeyError: ''. Pure presentation.
  • AttributeError_str synthesizes a message from side-channel context (obj/name) that the raiser never put in args, reconstructing what the interpreter attached out-of-band.

Only the second is the interesting reusable pattern ("I have structured context attributes; render them lazily instead of forcing every raise site to build the string"). The natural cohort there is NameError/UnboundLocalError (name), ImportError/ModuleNotFoundError (name, path). But both of those already build their full messages eagerly at the C raise site, so the problem AttributeError has — user __getattr__ implementations conventionally raising bare or name-only — barely exists for them.

So I'd lean against building a shared framework here:

  • Two thin, differing data points; abstracting them would abstract over a coincidence of mechanism, not a shared concern.
  • The templates ("object has no attribute" vs. "name is not defined" vs. "cannot import name from") are irreducibly per-type — only the dispatch around them could be shared.
  • Every __str__ change is a compatibility surface (people do match on message text), and each type wants its own guard about when overriding is safe.

The one genuinely reusable piece is the guard predicate — "was this constructed with a default/absent message, or one that merely duplicates a structured field, so I'm safe to override?" (here: args of size 0, or size 1 equal to name). If anything gets factored out, I'd make it that small internal helper and leave each template local. Otherwise I'd revisit sharing only when a third genuine case actually lands.

@johnslavik

Copy link
Copy Markdown
Member Author

The following code writes 'lazy_import' object has no attribute 'missing1': it mentions lazy_import rather than module 'asyncio'. But I don't think that AttributeError_str() would be the right place to reify the asyncio module!

lazy import asyncio

class RaiseWithName:
    def __getattr__(self, name):
        raise AttributeError(name)

try:
    getattr(globals()['asyncio'], "missing1")
except AttributeError as exc:
#    assert exc.obj is ...
    assert exc.name == "missing1"
    print(str(exc))

@vstinner, this code does not trigger the code path from this PR (RaiseWithName is unused). We could specialize this separately -- see #154196.


When possible, :attr:`name` and :attr:`obj` are set automatically.

.. versionchanged:: 3.10

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually do not add versionchanged for minor changes like changed error message. Otherwise our documentation would be overwhelmed by them.

Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c Outdated

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

Comment thread Objects/exceptions.c Outdated
Comment thread Objects/exceptions.c
Comment thread Objects/exceptions.c
Comment on lines +2744 to +2757
if (modname && PyUnicode_Check(modname)) {
result = PyUnicode_FromFormat("module '%U' has no attribute '%U'",
modname, name);
Py_DECREF(modname);
} else {
Py_XDECREF(modname);
result = PyUnicode_FromFormat("module has no attribute '%U'", name);
}
} else if (PyType_Check(obj)) {
result = PyUnicode_FromFormat("type object '%N' has no attribute '%U'",
obj, name);
} else {
result = PyUnicode_FromFormat("'%T' object has no attribute '%U'",
obj, name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The %U doesn't escape properly quote characters. Example:

exc=AttributeError()
exc.name = "a'b"
exc.obj = ["list"]
print(str(exc))

Current output:

'list' object has no attribute 'a'b'

I suggesting using %R instead of '%U':

Suggested change
if (modname && PyUnicode_Check(modname)) {
result = PyUnicode_FromFormat("module '%U' has no attribute '%U'",
modname, name);
Py_DECREF(modname);
} else {
Py_XDECREF(modname);
result = PyUnicode_FromFormat("module has no attribute '%U'", name);
}
} else if (PyType_Check(obj)) {
result = PyUnicode_FromFormat("type object '%N' has no attribute '%U'",
obj, name);
} else {
result = PyUnicode_FromFormat("'%T' object has no attribute '%U'",
obj, name);
if (modname && PyUnicode_Check(modname)) {
result = PyUnicode_FromFormat("module '%U' has no attribute %R",
modname, name);
Py_DECREF(modname);
} else {
Py_XDECREF(modname);
result = PyUnicode_FromFormat("module has no attribute %R", name);
}
} else if (PyType_Check(obj)) {
result = PyUnicode_FromFormat("type object '%N' has no attribute %R",
obj, name);
} else {
result = PyUnicode_FromFormat("'%T' object has no attribute %R",
obj, name);

@johnslavik johnslavik Jul 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, although we could say the same thing about module names and type names, no?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using import, I don't think that you can have a module name which contains quotes. You might hack the module name to insert quotes later.

For consistency, I would be fine with using %R to format the module name. It's also shorter 😁

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the type name, quotes or other special characters are very unlikely. I don't think that it's worth it to care about these ones. '%T' looks like the right format.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Changes applied.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, we should be consistent with the original error messages raised in corresponding tp_getattro. We can change this, but we should do this globally, and this is a separate issue.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it's worth it to use %R in similar code such as PyObject_GetAttr() and PyObject_SetAttr(). Currently, it also has the bug:

>>> getattr(object(), "a'b")
AttributeError: 'object' object has no attribute 'a'b'

We can change this, but we should do this globally, and this is a separate issue.

But I disagree with that. IMO it's fine to use %R in this PR. Similar code paths can be changed in a follow-up PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created issue #154610 and PR #154614 to change how the attribute name is formatted when formatting AttributeError error message.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for addressing my latest comments.

I wanted to merge the change, but @serhiy-storchaka seems to have concerns about %R usage.

@serhiy-storchaka

Copy link
Copy Markdown
Member

I think this deserves a separate discussion (What if the name is an instance of the str subclass like StrEnum?), but this should not be blocker for merging this PR.

@vstinner

Copy link
Copy Markdown
Member

this should not be blocker for merging this PR.

Do you mean that you're fine with %R? Or that the code should be reverted to '%U'?

@serhiy-storchaka

Copy link
Copy Markdown
Member

I am fine with any.

@vstinner
vstinner merged commit ae72d0c into python:main Jul 24, 2026
97 of 99 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Sprint Jul 24, 2026
@vstinner

Copy link
Copy Markdown
Member

I merged your enhancement, thanks @johnslavik!

@johnslavik

Copy link
Copy Markdown
Member Author

Thanks everyone for reviewing! I've learned a ton

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants