Repository navigation
AttributeErrors raised in .keys() or .__getitem__() during {**mymapping}are incorrectly masked #145876
Description
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
Activity
pythongh-145876: Fix AttributeError masked during dict unpacking
pythongh-145876: preserve AttributeError in dict unpacking
picnixz commentedon Mar 12, 2026
@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.
picnixz commentedon Mar 12, 2026
So as I was saying on one of the PR:
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.
NickCrews commentedon Mar 12, 2026
Thanks @picnixz for the review
Sorry, I'm not quite sure what you mean, can you elaborate what you think the requirements are, eg
?
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.
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.
ashm-dev commentedon Mar 12, 2026
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 (onPyTypeObject)? Check once on first encounter for a given type, then reuse the cached result. Plaindict | dicthits 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.
NickCrews commentedon Mar 12, 2026
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:picnixz commentedon Mar 12, 2026
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).
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.
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).
NickCrews commentedon Mar 12, 2026
@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)
picnixz commentedon Mar 12, 2026
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).
vstinner commentedon Mar 13, 2026
Extract of
DICT_UPDATEbytecode used by{**mapping}expression:Any
AttributeErrorerror is converted toTypeError("... object is not a a mapping").If
PyDict_Update()second argument is not a dict, it callsPyMapping_Keys()which callsPyObject_CallMethodNoArgs(o, keys). ThePyObject_CallMethodNoArgs()function doesn't distinguish "the method doesn't exit (AttributeError)" from another arbitrary AttributeError.It sounds quite complicated and tricky to only replace
AttributeErrorwithTypeErrorifAttributeErroris 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_UPDATEto pass throughAttributeErrorif it doesn't come from getting the.keys()method.5 remaining items
NickCrews commentedon Mar 13, 2026
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.
gh-145876: Do not mask AttributeErrors raised during dictionary unpac…
pythongh-145876: Do not mask KeyErrors raised during dictionary unpac…
serhiy-storchaka commentedon Mar 26, 2026
The issue with KeyError was more complex, but not too complex. #146472 fixes it.
gh-145876: Do not mask KeyErrors raised during dictionary unpacking i…
serhiy-storchaka commentedon Mar 29, 2026
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.
picnixz commentedon Mar 29, 2026
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.
Merge remote-tracking branch 'upstream/main' into tachyon-ndjson-kole…
pythongh-145876: Do not mask AttributeErrors raised during dictionary…
pythongh-145876: Do not mask KeyErrors raised during dictionary unpac…
pythongh-145876: Do not mask AttributeErrors raised during dictionary…
pythongh-145876: Do not mask KeyErrors raised during dictionary unpac…