Skip to content

AttributeErrors raised in .keys() or .__getitem__() during {**mymapping}are incorrectly masked #145876

Description

@NickCrews

Bug description:

I have a custom Mapping type. I am unpacking it with eg {**mymapping}. If, in either the keys() or the the __getitem__() method, I raise most kinds of errors, such as a ValueError, these are reported correctly. BUT, if I raise an AttributeError, then this error isn't reported properly, instead I get TypeError: 'MyMapping' object is not a mapping, masking the actual error:

class MyMapping:
    def __init__(
        self,
        *,
        raises_on_keys: type[Exception] | None = None,
        raises_on_getitem: type[Exception] | None = None,
    ):
        self.raises_on_keys = raises_on_keys
        self.raises_on_getitem = raises_on_getitem

    def __getitem__(self, key):
        if self.raises_on_getitem:
            raise self.raises_on_getitem("error in __getitem__")
        return key * 2

    def keys(self):
        if self.raises_on_keys:
            raise self.raises_on_keys("error in keys")
        return [1, 2, 3]


options = [
    None,
    ValueError,
    AttributeError,
]
outcomes = []
for raises_on_keys in options:
    for raises_on_getitem in options:
        try:
            d = {
                **MyMapping(
                    raises_on_keys=raises_on_keys, raises_on_getitem=raises_on_getitem
                )
            }
            outcomes.append((raises_on_keys, raises_on_getitem, "Success", d))
        except Exception as e:
            outcomes.append((raises_on_keys, raises_on_getitem, "Exception", str(e)))

# format to markdown table
print("| raises_on_keys | raises_on_getitem | outcome | result |")
print("| --- | --- | --- | --- |")
for raises_on_keys, raises_on_getitem, outcome, result in outcomes:
    raises_on_keys_str = raises_on_keys.__name__ if raises_on_keys else "None"
    raises_on_getitem_str = raises_on_getitem.__name__ if raises_on_getitem else "None"
    print(
        f"| {raises_on_keys_str} | {raises_on_getitem_str} | {outcome} | `{result}` |"
    )

Ran with uv run --python 3.14 bug.py, which resolves to python 3.14.2. This gives:

raises_on_keys raises_on_getitem error
None None ``
None ValueError ValueError: error in __getitem__
None AttributeError TypeError: 'MyMapping' object is not a mapping
ValueError None ValueError: error in keys
ValueError ValueError ValueError: error in keys
ValueError AttributeError ValueError: error in keys
AttributeError None TypeError: 'MyMapping' object is not a mapping
AttributeError ValueError TypeError: 'MyMapping' object is not a mapping
AttributeError AttributeError TypeError: 'MyMapping' object is not a mapping

What I would expect is for all of the TypeError: 'MyMapping' object is not a mapping errors to actually be AttributeError: error in keys or AttributeError: error in __getitem__ errors.

I assume this is because in the implementation, it does assumes ducktyping, and the raised attribute error is interpreted as "the passed object doesn't even have a keys()/__getitem__ method"

eg guessing this is how this is currently implemented:

try:
    for key in obj.keys():
        yield key, obj.__getitem__(key)
except AttributeError as e:
    raise TypeError(f"'{type(obj).__name__}' object is not a mapping")

What I think SHOULD happen:

try:
    keys = obj.keys
except AttributeError as e:
    raise TypeError(f"'{type(obj).__name__}' object is not a mapping")
for key in keys():
    try:
        getter = obj.__getitem__
    except AttributeError as e:
        raise TypeError(f"'{type(obj).__name__}' object is not a mapping")
    yield key, getter(key)

EDIT: Actually this should be more performant, only 2 checks, instead of N checks, one per key. (Also, for the record, this includes suggestion to improve the error messages, but that should definitely be a separate PR)

try:
    keys = obj.keys
except AttributeError as e:
    raise TypeError(f"'{type(obj).__name__}' object requires a .keys() method to be used as a mapping")
try:
    getter = obj.__getitem__
except AttributeError as e:
    raise TypeError(f"'{type(obj).__name__}' object requires a .__getitem__() method to be used as a mapping")
for key in keys():
    yield key, getter(key)

CPython versions tested on:

3.14

Operating systems tested on:

macOS

Linked PRs

Activity

  1. added a commit that references this issue on Mar 12, 2026
  2. added a commit that references this issue on Mar 12, 2026
  3. added
    pendingThe issue will be closed if no feedback is provided
    on Mar 12, 2026
  4. picnixz commented on Mar 12, 2026

    @picnixz
    Member

    @ashm-dev @zakiscoding Please don't create PR until a core dev decided whether it was a valid issue. Especially if you are using an LLM. It's a waste of my time.

  5. picnixz commented on Mar 12, 2026

    @picnixz
    Member

    So as I was saying on one of the PR:

    I suspect that this will add an overhead to non-exact dict operations. It's definitely not desirable in this case. In addition, the case is a bit niche IMO. I want to discuss this on the issue first so I'm going to close it for now.

    If we are to catch the AttributeError before doing the check, we slow down common cases (non-exact dict objects). If we catch the AttributeError after, I'm not sure if we can be entirely sure that this AttributeError we caught was the correct one.

    Nonetheless, we also need to check what happens with unpacking for sequences (e.g., *myseq). Ideally, we should have the same behavior. And finally, we should check what the specifications say about this case and how it was historically decided.

    While seemingly a trivial change, it's still a breaking change (changing the type of an exception is a breaking change) and thus we should be more careful. So, until those questions have been answered, I'd prefer to keep the current status quo.

  6. NickCrews commented on Mar 12, 2026

    @NickCrews
    ContributorAuthor

    Thanks @picnixz for the review

    If we are to catch the AttributeError before doing the check, we slow down common cases (non-exact dict objects). If we catch the AttributeError after, I'm not sure if we can be entirely sure that this AttributeError we caught was the correct one.

    Sorry, I'm not quite sure what you mean, can you elaborate what you think the requirements are, eg

    • dict-dict merges MUST NOT become slower
    • dict-custom mapping merges SHOULD NOT become more than 10% slower
      ?

    Nonetheless, we also need to check what happens with unpacking for sequences (e.g., *myseq)

    That is a great thought I didn't consider. I totally agree, this should be consistent. I'll write a test, similar to the above, and report back.

    And finally, we should check what the specifications say about this case and how it was historically decided.

    Also good to do. @ashm-dev If you want to be useful, this would be a perfect thing to do and report back. Otherwise I can try to get to this.

  7. ashm-dev commented on Mar 12, 2026

    @ashm-dev
    Contributor

    I'd be happy to take on the spec/history research. I'll dig through PEP 584 and related discussions and report back with findings.

    On the performance side — what if we cache the presence/absence of keys/__getitem__ at the type level (on PyTypeObject)? Check once on first encounter for a given type, then reuse the cached result. Plain dict | dict hits the fast path with zero overhead, custom mappings pay for one type-level lookup instead of per-call checks. Should sidestep the early-vs-late catch dilemma.

    Happy to prototype this if the approach sounds reasonable.

  8. NickCrews commented on Mar 12, 2026

    @NickCrews
    ContributorAuthor

    OK, it looks like in python 3.14.2, iterables unpacked with [*myiterable] do not mask the errors, they correctly show the underlying error that happened when the __next__() and __iter__() methods are invoked:

    from __future__ import annotations
    
    from dataclasses import dataclass
    from typing import Any
    
    
    @dataclass(slots=True)
    class Outcome:
        case_id: str
        iterator: object
        error_str: str
    
    
    class HasIter:
        def __init__(
            self,
            *,
            raises_on_iter: type[Exception] | None = None,
            raises_on_next: type[Exception] | None = None,
        ):
            self.raises_on_iter = raises_on_iter
            self.raises_on_next = raises_on_next
    
        def __iter__(self):
            if self.raises_on_iter:
                raise self.raises_on_iter(f"error in {type(self).__name__}.__iter__")
            return HasNext(raises_on_next=self.raises_on_next)
    
        def __getitem__(self, index: int) -> int:
            print("THIS SHOULD NOT BE CALLED SINCE __iter__ IS PRESENT")
            raise AssertionError(
                "__getitem__ should not be called when __iter__ is present"
            )
    
    
    class HasNext:
        def __init__(self, *, raises_on_next: type[Exception] | None = None):
            self.i = 0
            self.raises_on_next = raises_on_next
    
        def __next__(self) -> int:
            if self.raises_on_next:
                raise self.raises_on_next(f"error in {type(self).__name__}.__next__")
            if self.i < 3:
                value = self.i
                self.i += 1
                return value
            raise StopIteration
    
    
    class OnlyHasGetitem:
        def __init__(self, raises_on_getitem: type[Exception] | None = None):
            self.raises_on_getitem = raises_on_getitem
    
        def __getitem__(self, index: int) -> int:
            if self.raises_on_getitem:
                raise self.raises_on_getitem(f"error in {type(self).__name__}.__getitem__")
            if index < 3:
                return index + 1
            raise IndexError
    
    
    class HasNothing:
        pass
    
    
    def run_case(*, id: str, obj: Any) -> Outcome:
        try:
            _result = [*obj]
            error_str = ""
        except Exception as exc:
            error_str = f"{type(exc).__name__}: {exc}"
        return Outcome(case_id=id, iterator=obj, error_str=error_str)
    
    
    def emit_markdown_table(outcomes: list[Outcome]) -> None:
        case_header = "case"
        error_header = "error"
        case_width = max(len(case_header), *(len(row.case_id) for row in outcomes))
        error_width = max(len(error_header), *(len(row.error_str) for row in outcomes))
    
        print(f"| {case_header.ljust(case_width)} | {error_header.ljust(error_width)} |")
        print(f"| {'-' * case_width} | {'-' * error_width} |")
        for row in outcomes:
            print(
                f"| {row.case_id.ljust(case_width)} | {row.error_str.ljust(error_width)} |"
            )
    
    
    def main() -> None:
        error_options: list[type[Exception] | None] = [
            None,
            ValueError,
            AttributeError,
        ]
    
        def error_name(exc: type[Exception] | None) -> str:
            return exc.__name__ if exc else "None"
    
        outcomes: list[Outcome] = []
    
        for raises_on_iter in error_options:
            for raises_on_next in error_options:
                probe = HasIter(
                    raises_on_iter=raises_on_iter,
                    raises_on_next=raises_on_next,
                )
                outcomes.append(
                    run_case(
                        id=f"HasIter(raises_on_iter={error_name(raises_on_iter)}, raises_on_next={error_name(raises_on_next)})",
                        obj=probe,
                    )
                )
    
        for raises_on_getitem in error_options:
            probe = OnlyHasGetitem(raises_on_getitem=raises_on_getitem)
            outcomes.append(
                run_case(
                    id=f"OnlyHasGetitem(raises_on_getitem={error_name(raises_on_getitem)})",
                    obj=probe,
                )
            )
    
        outcomes.append(run_case(id="HasNothing", obj=HasNothing()))
    
        emit_markdown_table(outcomes)
    
    
    if __name__ == "__main__":
        main()
    case error
    HasIter(raises_on_iter=None, raises_on_next=None)
    HasIter(raises_on_iter=None, raises_on_next=ValueError) ValueError: error in HasNext.next
    HasIter(raises_on_iter=None, raises_on_next=AttributeError) AttributeError: error in HasNext.next
    HasIter(raises_on_iter=ValueError, raises_on_next=None) ValueError: error in HasIter.iter
    HasIter(raises_on_iter=ValueError, raises_on_next=ValueError) ValueError: error in HasIter.iter
    HasIter(raises_on_iter=ValueError, raises_on_next=AttributeError) ValueError: error in HasIter.iter
    HasIter(raises_on_iter=AttributeError, raises_on_next=None) AttributeError: error in HasIter.iter
    HasIter(raises_on_iter=AttributeError, raises_on_next=ValueError) AttributeError: error in HasIter.iter
    HasIter(raises_on_iter=AttributeError, raises_on_next=AttributeError) AttributeError: error in HasIter.iter
    OnlyHasGetitem(raises_on_getitem=None)
    OnlyHasGetitem(raises_on_getitem=ValueError) ValueError: error in OnlyHasGetitem.getitem
    OnlyHasGetitem(raises_on_getitem=AttributeError) AttributeError: error in OnlyHasGetitem.getitem
    HasNothing TypeError: Value after * must be an iterable, not HasNothing
  9. picnixz commented on Mar 12, 2026

    @picnixz
    Member

    dict-dict merges MUST NOT become slower
    dict-custom mapping merges SHOULD NOT become more than 10% slower
    ?

    We must not have penalties in BOTH cases or very limited ones in the second case (say 1% worse). Only after handling the exception can we add overhead (that is, just before raising the error we can add as much work as we need; error paths are not hot paths, succeesful ones are hot paths that should do minimal work).

    On the performance side — what if we cache the presence/absence of keys/getitem at the type level (on PyTypeObject)? Check once on first encounter for a given type, then reuse the cached result. Plain dict | dict hits the fast path with zero overhead, custom mappings pay for one type-level lookup instead of per-call checks. Should sidestep the early-vs-late catch dilemma.

    No. This does not make sense since you can reassign the method later.

    Again, I would appreciate that you do not just copy/paste the LLM output. Otherwise I could just do it myself. It is tiring to filter low quality suggestions offered by LLMs and it is also tiring that I always need to repeat myself about it.

    OK, it looks like in python 3.14.2, iterables unpacked with [*myiterable] do not mask the errors, they correctly show the underlying error that happened when the next() and iter() methods are invoked:

    In this case, I suggest taking a look at when, where, and how we do this. Whatever solution we pick, I want to know the real impact of those changes both on microbenchmarks and macrobenchmarks. For microbenchmarks one should assume that the unpacked value can be a very short list or dict (happens when we do arguments and keywords forwarding) or very large ones (happens when someone does a shallow copy or flatten iterables).

  10. NickCrews commented on Mar 12, 2026

    @NickCrews
    ContributorAuthor

    In this case, I suggest taking a look at when, where, and how we do this.

    @picnixz thanks. That sounds to me that you are agreeing with my thesis that this is indeed a bug, and that we MUST change the behavior of **mapping to match the behavior or *iterable, but as we do this we need to do it in the most performance-aware way possible? Or do you still want me/someone to do background research on the spec/forums/issue tracker/stack overflow to find prior art here, eg perhaps this shouldn't be considered a bug?

    I am hesitant to actually start getting into the guts of the interpreter loop, I'm not familiar enough with this codebase and I don't really want the overhead of doing my initial dev environment setup. If I don't write the PR, do you think there is any chance that someone else will come and write it?

    I would be more comfortable to support in other ways, like writing the benchmark. Should I do that by adding a test case to the pyperformance repo, similar to this PR from 2 months ago?

    Is there anything else I can do to drive this forward? Do we need to get a +1 from another cpython contributor before I keep spending effort on this, or do you think this is safe that SOMETHING will come from this? (I just don't want to waste my time)

  11. picnixz commented on Mar 12, 2026

    @picnixz
    Member

    Considering it is only changing the exception being raised, it should be fine to make it for 3.15 but I am not confident in changing it in bugfix branches. I do consider it odd that the behavior changes with sequence unpacking but I do not know if this was deliberate or not so yes I would appreciate other opinions.

    So, I do want to hear from other core devs that may have more historical knowledge and more expertise in the interpreter loop itself @serhiy-storchaka @vstinner @ncoghlan @markshannon. Also I think unpacking in comprehensions was recently accepted as a PEP so we should see how it should interact with it (it may also not be a concern).

  12. vstinner commented on Mar 13, 2026

    @vstinner
    Member

    Extract of DICT_UPDATE bytecode used by {**mapping} expression:

                int err = PyDict_Update(dict_o, update_o);
                if (err < 0) {
                    int matches = _PyErr_ExceptionMatches(tstate, PyExc_AttributeError);
                    if (matches) {
                        _PyErr_Format(tstate, PyExc_TypeError,
                                        "'%.200s' object is not a mapping",
                                        Py_TYPE(update_o)->tp_name);
                    }
                    PyStackRef_CLOSE(update);
                    ERROR_IF(true);
                }
                PyStackRef_CLOSE(update);
            }
    

    Any AttributeError error is converted to TypeError("... object is not a a mapping").

    If PyDict_Update() second argument is not a dict, it calls PyMapping_Keys() which calls PyObject_CallMethodNoArgs(o, keys). The PyObject_CallMethodNoArgs() function doesn't distinguish "the method doesn't exit (AttributeError)" from another arbitrary AttributeError.

    It sounds quite complicated and tricky to only replace AttributeError with TypeError if AttributeError is raised by the method call, and not when getting the method. There are likely many other places in Python where we replace any error of one type to another error type.

    I'm not sure that it's worth it to fix DICT_UPDATE to pass through AttributeError if it doesn't come from getting the .keys() method.

  13. 5 remaining items

  14. NickCrews commented on Mar 13, 2026

    @NickCrews
    ContributorAuthor

    Just don't do that :-)

    I mean, there are infinitely many things that aren't supported that python users do hundreds of thousands of times each day. It's just most of them have good error messages that help them correct the problem. I for sure didn't intend to raise an AttributeError in myMapping, but when I accidentally did then I spent hours being very confused trying to figure out why my class didn't fit the Mapping protocol, since that is what the error told me to look at. I want to avoid that misdirection for users.

  15. added 2 commits that reference this issue on Mar 26, 2026
  16. serhiy-storchaka commented on Mar 26, 2026

    @serhiy-storchaka
    Member

    The issue with KeyError was more complex, but not too complex. #146472 fixes it.

  17. added a commit that references this issue on Mar 29, 2026
  18. serhiy-storchaka commented on Mar 29, 2026

    @serhiy-storchaka
    Member

    Thank you for your report, @NickCrews.

    The past behavior was just an implementation artifact. At that time it looked the simplest way to do this, and it worked in common cases. Now we have spent more effort to make it working in more corner cases.

  19. picnixz commented on Mar 29, 2026

    @picnixz
    Member

    FTR, since it's a behavior change, albeit a possible bugfix, I think it's safer that we do not backport this change, so it'll only be in 3.15 for now. We will also be able to get more feedback on this if someone is unhappy with the change or if it catastrophically broke something.

  20. added
    type-featureA feature request or enhancement
    3.15bugs and security fixes
    on Mar 29, 2026
  21. added 2 commits that reference this issue on Apr 16, 2026
  22. added 2 commits that reference this issue on Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

3.15bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errortype-featureA feature request or enhancement

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions