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

Functions must not suppress exceptions #35

Open
encukou opened this issue Oct 24, 2023 · 5 comments
Open

Functions must not suppress exceptions #35

encukou opened this issue Oct 24, 2023 · 5 comments
Labels
guideline To be included in guidelines PEP

Comments

@encukou
Copy link
Contributor

encukou commented Oct 24, 2023

From capi-workgroup/problems#51

Functions must not suppress unknown exceptions, except ones that specifically to do just that (like PyErr_Clear).

(#13 is related: If a function has a common “failure”, like “attribute not found” from a getattr, that can be treated as success instead, and be signaled with a dedicated return value. This avoids creating an exception object.)

@gvanrossum
Copy link
Contributor

(#13 is related: If a function has a common “failure”, like “attribute not found” from a getattr, this should generally be treated as success instead, and be signaled with a dedicated return value. This avoids creating an exception object.)

I think that's a bit strong. There are certain situations where a certain type of failure is commonly expected by the user, and then in some cases we have the option to design a more ergonomic API (e.g. dict.get(), three-arg getattr()). But I don't think every common failure deserves this treatment, as your use of "should generally" seems to imply.

@zooba
Copy link
Contributor

zooba commented Oct 24, 2023

We haven't really designed the separation between "API for people who want stability" vs "API for people who want speed", but I'd say that an API that signals failure without an exception belongs in the latter category, and should be a special case of one that exists in the former.

Otherwise, how will someone who wants a normal looking AttributeError create it? Do they have to copy-paste the message themselves?

(What would be nice is to be able to distinguish "this operation failed with AttributeError" vs "this operation succeeded but triggered an AttributeError elsewhere", but this is probably best done with exception chaining anyway, since that's how it'd work in pure Python code.)

@encukou
Copy link
Contributor Author

encukou commented Oct 25, 2023

I softened the wording. Doing this avoids creating an exception object; that is reason enough to design (some of) the API that way.

Otherwise, how will someone who wants a normal looking AttributeError create it? Do they have to copy-paste the message themselves?

I wonder if we need to restore some of the old way of handling excetpions, where “type” and “value” (and traceback) were decoupled until something needed a materialized exception object. Raising an exception that's immediately handled was much cheaper in that scheme.

Or maybe add functions like PyErr_RaiseAttributeError(owner, name)...

(What would be nice is to be able to distinguish "this operation failed with AttributeError" vs "this operation succeeded but triggered an AttributeError elsewhere"

That's another point for the 0 = not found, 1 = found, -1 = exception scheme. If an unrelated AttributeError happened, pass it on and return -1.

@zooba
Copy link
Contributor

zooba commented Oct 25, 2023

I wonder if we need to restore some of the old way of handling excetpions, where “type” and “value” (and traceback) were decoupled until something needed a materialized exception object. Raising an exception that's immediately handled was much cheaper in that scheme.

That would be fine by me, I always appreciated the low cost of that scheme. But I believe it interacts poorly with some of the interpreter optimisations?

@vstinner
Copy link
Contributor

In capi-workgroup/problems#51 (comment) list, the remaining function is PySys_GetObject(): thre is no variant which does not suppress exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guideline To be included in guidelines PEP
Projects
None yet
Development

No branches or pull requests

4 participants