Can you share more detail about this PR? Are we going to remove this test, or what improvement did we add to make it compatible? Sorry, the GitHub review is not the greatest to see diff.

#29023 (comment)

The original test was

        with self.assertRaises(KeyError):
            service_desc.FindMethodByName(_INVALID_SYMBOL_NAME)

I changed it to translate a None return value to a raise KeyError.

Thanks for the clear write-up in protocolbuffers/protobuf#9592.

Checking other find* methods (Python code), they favor the raise behavior. Do you know how the C-Extension implementation will behave? Based on code, their return type is bool.

In the code you linked, the return type is bool but internally it calls a Python function (PyObject_CallMethod(...)). In this context we have a call chain that look like Python -> C++ -> Python, where the outer Python code is the one doing the call desc_database.FindFileByName(...) and the inner is the FindFileByName implementation. I'm not an expert on this, but I believe that if the inner Python code raises an exception then it is immediately caught by the outer Python code - it doesn't even do exception folding of that C++ function, just sort of steals the context.

If the exception is not caught by the outer Python code, it looks like this:

Traceback (most recent call last):
  File "test.py", line 12, in <module>
    desc_pool.FindFileByName("foo")
KeyError: "Couldn't find file foo"

Note how all the traceback of the inner C++ -> Python is missing as result of this unusual flow.

I hope this answers your question.