GeezerVison

I suppose a user could be absolutely insane and delete the .keys() method of their object in the middle of throwing their error. Then this check here wouldn't really detect the original cause of the error. Ideally this check was "atomic" with our actual attempt at accessing keys. But this seems like such an edge case that the more performant check-details-only-after-error seems like the right balance of perf vs correctness.

If this API existed, I think it would be better to inspect the exc PyObject to see if it was

1. from attempting to access update_o
2. using the "keys" attribute name.
   instead of this PyObject_HasAttrWithError() check

Or, the other more heavy-handed solution would be to adjust the PyDict_Update(dict_o, update_o); line above to be a more explicit API with a result object such as

UpdateResult update_result;
PyDict_Update(dict_o, update_o, &update_result);
if (update_result.error) ...
if (update_result.error_cause = NO_KEYS) ...

but not sure about the perf, maintenance, etc costs of doing this.

Inspecting the exception object is not enough, because the user code can raise an exception which look exactly the same as when accessing the key attribute.. And we will get the original issue. We would need a special variants of PyDict_Merge() and PyMapping_Keys(). This has too much cost.

Note that dict.update() tests the existence of keys prior to calling PyMapping_Keys(), so it looks up keys twice. This adds some overhead, but this is necessary. In dictionary unpacking we can avoid such overhead.

I'm assuming that this C API is the thing responsible for the ...during the above exception...another exception was raised... message. Should the test cases above check that the user sees this error chain? Currently it looks like we only check for the root cause of the error.

This only happens when the second lookup of the keys attribute raises an exception different from AttributeError, while the first lookup was successful or raised an AttributeError. This is very uncommon case and in needs a special complex code for testing. I do not think that it is is worth to do.

The main effect is that if the second lookup of the keys fails, we do not swallow the exception (it can be KeyboardInterrupt). Swallowing exception is always bad.

Flagging that this is changing the formatting string from '%.200s' to '%T', but I'm assuming that is intentional.

Similar to above, I would love a more watertight/atomic way of checking the real cause of the error.