Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[better_errors] Ensure debug_info.arg_names is never None. #25992

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gnecula
Copy link
Collaborator

@gnecula gnecula commented Jan 20, 2025

Most places in the code assumed this already, but often that usage is error reporting code, which is not yet well tested.

When we cannot get the inspect.Signature, or when the args and kwargs do not match the signature, we generate the flattened argument names as: args[0], args[1], kwargs['foo'], ... Previously, in these cases we returned arg_names is None, and then the whole debug_info ended up being None, throwing away even available information.

We also add support for api_util.fun_sourceinfo even for cases when the fun.__code__ is not available. In
those cases we used to say that fun_sourceinfo is None. Now, we use the string representation of fun
to get the name of built-in functions, or we use "".

@gnecula gnecula force-pushed the debug_info_arg_names branch from df6116e to 870da21 Compare January 20, 2025 17:12
@gnecula gnecula self-assigned this Jan 20, 2025
@gnecula gnecula added the pull ready Ready for copybara import and testing label Jan 20, 2025
@gnecula gnecula force-pushed the debug_info_arg_names branch 8 times, most recently from 9b41326 to 6aa552b Compare January 21, 2025 08:03
@gnecula gnecula requested a review from dfm January 21, 2025 08:04
@gnecula gnecula force-pushed the debug_info_arg_names branch 3 times, most recently from 96b2395 to 6c5a6bc Compare January 21, 2025 09:10
Most places in the code assumed this already, but often
that usage is error reporting code, which is not yet well tested.

When we cannot get the `inspect.Signature` or when the
args and kwargs do not match the signature, we generate
the flattened argument names as: `args[0]`, `args[1]`,
`kwargs['foo']`, ... Previously, in these cases we
returned `arg_names` is None, and then the whole
debug_info ended up being `None`, throwing away even
available information.

We also add support for `api_util.fun_sourceinfo` even
for cases when the `fun.__code__` is not available. In
those cases we used to say that `fun_sourceinfo` is
`None`. Now, we use the string representation of `fun`
to get the name of built-in functions, or we use "<unknown>".
@gnecula gnecula force-pushed the debug_info_arg_names branch from 6c5a6bc to 3f73f7b Compare January 21, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants