Let try-except check the exception instance
Hi! I was working on handling some exceptions from external software (e.g. database constraint triggers) switching the handler based on the messages that had been sent. Today we can do something like (running on Python 3.6.5):
That works, but I'd like to do something like:
Creating classes to "match" the exception in their __instancecheck__. Well, I tried to do so. A simplified example would be:
Using these new classes, we would get this:
Yet,
The idea is to allow catching exceptions beyond checking their MRO, using a class that checks the exception instance by implementing a custom __instancecheck__. -- Danilo J. S. Bellini --------------- "*It is not our business to set up prohibitions, but to arrive at conventions.*" (R. Carnap)
On Thu, May 31, 2018 at 01:47:17AM -0300, Danilo J. S. Bellini wrote:
Since error messages are rarely part of the exception API, testing for them is fragile and prone to errors. For example, what if the message changes to "no money" or "out of funds" or "insufficient funds" or "keine Mittel"? Since the error message is not part of the API, that can happen at any time, without warning. If you want to make such a risky check in your own code, of course you can do so, but we shouldn't encourage it or add functionality to make it easier. You have the right to shoot yourself in the foot, but don't expect us to load the gun, hand it to you, point it at your foot, and place your finger on the trigger :-)
The obvious way to do that is to create BeyondLimit and NoFunds subclasses of DatabaseError. Why can't you do that?
That seems like a backwards way to do it to me. If you're the author of the exception class, why not just subclass your exceptions the regular way and get a real subclass? If you have enough control of the source to change the exception from DatabaseError to ExceptionHasMessage, then you ought to have enough control to change to a specific subclass as needed. And if you *don't* have that control, then you're probably monkey- patching something you don't control and again, we shouldn't encourage that. Do you have a better example of checking __instancecheck__ that doesn't involve something that's a risky hack? -- Steve
On 31 May 2018 at 02:41, Steven D'Aprano <steve@pearwood.info> wrote:
The messages are from constraint triggers in the database. I'm the author of them.
The obvious way to do that is to create BeyondLimit and NoFunds subclasses of DatabaseError. Why can't you do that?
Because I'm not the author of DatabaseError. It's an ORM exception or a database driver class exception, something I'd need to monkeypatch to change. That seems like a backwards way to do it to me. If you're the author of
the exception class, why not just subclass your exceptions the regular way and get a real subclass?
I agree. Problem is: I'm not the author of the exception class, and I'm not raising/throwing their exception instances. On 31 May 2018 at 04:19, Terry Reedy <tjreedy@udel.edu> wrote:
As Stephen said, messages are intentionally not part of the defined API. [...]
We could check error numbers instead of meaningful strings. I'm thinking on technical error keys in either way. Previously to classes like FileNotFoundError, we would need to check an OSError errno. There were no alternative: Python users are not the author of "everywhere that raises OSError". Another example is urllib.error.HTTPError. We can have a specific handler code for a given HTTP error code. Yet we don't change urllib.request.urlopen to raise something else for that specific error code. On 31 May 2018 at 06:29, Stephan Houben <stephanh42@gmail.com> wrote:
"[...] The exception matching machinery ignores the __instancecheck__ mechanism."
The __subclasscheck__ gets bypassed, as well. -- Danilo J. S. Bellini --------------- "*It is not our business to set up prohibitions, but to arrive at conventions.*" (R. Carnap)
On 5/31/2018 12:47 AM, Danilo J. S. Bellini wrote:
Yes, it works perfectly well, AND it exposes the fact that your code depends on the message, which I think is a good thing. As Stephen said, messages are intentionally not part of the defined API. As a matter of curtesy, we usually restrict message changes to new versions and do not backport the change. An exception may be made if we decide that a message is sufficiently erroneous that is likely misleads people. In any case, message dependent code may be version dependent. -- Terry Jan Reedy
Current documentation says: "An object is compatible with an exception if it is the class or a base class of the exception object or a tuple containing an item compatible with the exception." https://docs.python.org/3/reference/compound_stmts.html#the-try-statement It is, in my opinion, not very clear from this that the __instancecheck__ mechanism is bypassed. Should the documentation perhaps be adapted to explain that the class needs to actually occur in the MRO and that virtual base classes are not considered for matching purposes? "An object is compatible with an exception if it is the class or a non-virtual base class of the exception object or a tuple containing an item compatible with the exception. The exception matching machinery ignores the __instancecheck__ mechanism." Stephan 2018-05-31 9:19 GMT+02:00 Terry Reedy <tjreedy@udel.edu>:
On 31 May 2018 at 14:47, Danilo J. S. Bellini <danilo.bellini@gmail.com> wrote:
The exception machinery deliberately attempts to avoid instantiating exception objects whenever it can, but that gets significantly more difficult if we always need to create the instance before we can decide whether or not the raised exception matches the given exception handler criteria. So quite aside from any philosophy-of-design questions, we're unlikely to ever implement this simply for laziness-of-evaluation reasons. (There are already lots of circumstances that force instantiation - that doesn't mean we're keen to add more) Cheers, Nick. P.S. There's a somewhat related issue aimed at getting ABCs to work correctly as exception handlers, although that still sticks to the principle that exception handler checks solely consider the exception type, not its value: https://bugs.python.org/issue12029 -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Thu, 31 May 2018 07:49:58 -0700 Ethan Furman <ethan@stoneleaf.us> wrote:
Nick is talking about exceptions that are ultimately silenced, especially when caught from C code. You're right that, once caught from Python code, the exception *has* to be instantiated (since it is bound to a user-visible variable). Regards Antoine.
Ethan Furman wrote:
Why is this? Doesn't the exception have to be instantiated at some point, even if just to print to stderr?
If it gets caught by an except clause without an else clause, in theory there's no need to instantiate it. However, Python doesn't currently seem to take advantage of that:
-- Greg
On Thu, May 31, 2018 at 10:55 PM Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I don't think it's possible to avoid instantiating the exception at the user level, what would sys.exc_info() do about it's second return value? I believe the only cases where it's possible to avoid instantiation are inside the interpreter itself, where the exception never propagates up to user visibility.
Eric Fahlgren wrote:
I don't think it's possible to avoid instantiating the exception at the user level, what would sys.exc_info() do about it's second return value?
The exception could be instantiated if and when sys.exc_info() gets called. -- Greg
On Thu, May 31, 2018 at 10:37 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
The exception machinery deliberately attempts to avoid instantiating
exception objects whenever it can, but that gets significantly more difficult if we always need to create the instance before we can decide whether or not the raised exception matches the given exception handler criteria. Is this really true? Consider the following simple code class E(Exception): def __init__(self): print("instantiated") try: raise E except E: pass Is it truly necessary to instantiate E() in this case? Yet when I run it, I see "instantiated" printed on the console.
On Thu, 31 May 2018 14:00:24 -0400 Alexander Belopolsky <alexander.belopolsky@gmail.com> wrote:
I don't think it's truly necessary, but there's enough complication nowadays in the exception subsystem (e.g. with causes and contexts) that at some point we (perhaps I) decided it made things less hairy to always instantiate it in an "except" clause. Let me stress, though: this happens when catching exceptions in *Python*. When in C you call PyErr_ExceptionMatches, the exception should not get instantiated. Regards Antoine.
On Wed, May 30, 2018 at 10:47 PM, Danilo J. S. Bellini <danilo.bellini@gmail.com> wrote:
Matching on error messages is a recipe for pain. My experience with Go seared that lesson into my brain. (The boilerplate involved there to make custom error types leads to a lot of error message checking instead.)
Using subclasses like this is the way to go. If you control the originating code then make it happen. :) If not then work to fix it upstream. If that fails then at least use a helper that converts the exception after the fact. It could look like following: # Subclassing DatabaseError means existing try-except block will still work the same. class CustomDatabaseError(DatabaseError): .... class BeyondLimitError(CustomDatabaseError): ... class NoFundsError(CustomDatabaseError): ... def convert_exception(exc): if isinstance(exc, CustomDatabaseError): raise msg = get_db_error_msg_from_exception(exc) if msg == "beyond_limit": raise BeyondLimitError(...) elif msg == "no_funds": raise NoFundsError(...) else: raise @contextmanager def converted_exceptions(): try: yield except CustomDatabaseError: raise except DatabaseError as exc: convert_exception(exc) def db_op(...): with converted_exceptions(): # Some code that does DB ops, e.g. session.commit(). ... try: db_op(...) except BeyondLimitError: ... except NoFundsError: ... except DatabaseError: # fallback ... The big wins there are with re-usability, maintainability, and separation of concerns. Notably, it consolidates your message-handling code to one spot and keeps it focused. -eric
On Thu, May 31, 2018 at 01:47:17AM -0300, Danilo J. S. Bellini wrote:
Since error messages are rarely part of the exception API, testing for them is fragile and prone to errors. For example, what if the message changes to "no money" or "out of funds" or "insufficient funds" or "keine Mittel"? Since the error message is not part of the API, that can happen at any time, without warning. If you want to make such a risky check in your own code, of course you can do so, but we shouldn't encourage it or add functionality to make it easier. You have the right to shoot yourself in the foot, but don't expect us to load the gun, hand it to you, point it at your foot, and place your finger on the trigger :-)
The obvious way to do that is to create BeyondLimit and NoFunds subclasses of DatabaseError. Why can't you do that?
That seems like a backwards way to do it to me. If you're the author of the exception class, why not just subclass your exceptions the regular way and get a real subclass? If you have enough control of the source to change the exception from DatabaseError to ExceptionHasMessage, then you ought to have enough control to change to a specific subclass as needed. And if you *don't* have that control, then you're probably monkey- patching something you don't control and again, we shouldn't encourage that. Do you have a better example of checking __instancecheck__ that doesn't involve something that's a risky hack? -- Steve
On 31 May 2018 at 02:41, Steven D'Aprano <steve@pearwood.info> wrote:
The messages are from constraint triggers in the database. I'm the author of them.
The obvious way to do that is to create BeyondLimit and NoFunds subclasses of DatabaseError. Why can't you do that?
Because I'm not the author of DatabaseError. It's an ORM exception or a database driver class exception, something I'd need to monkeypatch to change. That seems like a backwards way to do it to me. If you're the author of
the exception class, why not just subclass your exceptions the regular way and get a real subclass?
I agree. Problem is: I'm not the author of the exception class, and I'm not raising/throwing their exception instances. On 31 May 2018 at 04:19, Terry Reedy <tjreedy@udel.edu> wrote:
As Stephen said, messages are intentionally not part of the defined API. [...]
We could check error numbers instead of meaningful strings. I'm thinking on technical error keys in either way. Previously to classes like FileNotFoundError, we would need to check an OSError errno. There were no alternative: Python users are not the author of "everywhere that raises OSError". Another example is urllib.error.HTTPError. We can have a specific handler code for a given HTTP error code. Yet we don't change urllib.request.urlopen to raise something else for that specific error code. On 31 May 2018 at 06:29, Stephan Houben <stephanh42@gmail.com> wrote:
"[...] The exception matching machinery ignores the __instancecheck__ mechanism."
The __subclasscheck__ gets bypassed, as well. -- Danilo J. S. Bellini --------------- "*It is not our business to set up prohibitions, but to arrive at conventions.*" (R. Carnap)
On 5/31/2018 12:47 AM, Danilo J. S. Bellini wrote:
Yes, it works perfectly well, AND it exposes the fact that your code depends on the message, which I think is a good thing. As Stephen said, messages are intentionally not part of the defined API. As a matter of curtesy, we usually restrict message changes to new versions and do not backport the change. An exception may be made if we decide that a message is sufficiently erroneous that is likely misleads people. In any case, message dependent code may be version dependent. -- Terry Jan Reedy
Current documentation says: "An object is compatible with an exception if it is the class or a base class of the exception object or a tuple containing an item compatible with the exception." https://docs.python.org/3/reference/compound_stmts.html#the-try-statement It is, in my opinion, not very clear from this that the __instancecheck__ mechanism is bypassed. Should the documentation perhaps be adapted to explain that the class needs to actually occur in the MRO and that virtual base classes are not considered for matching purposes? "An object is compatible with an exception if it is the class or a non-virtual base class of the exception object or a tuple containing an item compatible with the exception. The exception matching machinery ignores the __instancecheck__ mechanism." Stephan 2018-05-31 9:19 GMT+02:00 Terry Reedy <tjreedy@udel.edu>:
On 31 May 2018 at 14:47, Danilo J. S. Bellini <danilo.bellini@gmail.com> wrote:
The exception machinery deliberately attempts to avoid instantiating exception objects whenever it can, but that gets significantly more difficult if we always need to create the instance before we can decide whether or not the raised exception matches the given exception handler criteria. So quite aside from any philosophy-of-design questions, we're unlikely to ever implement this simply for laziness-of-evaluation reasons. (There are already lots of circumstances that force instantiation - that doesn't mean we're keen to add more) Cheers, Nick. P.S. There's a somewhat related issue aimed at getting ABCs to work correctly as exception handlers, although that still sticks to the principle that exception handler checks solely consider the exception type, not its value: https://bugs.python.org/issue12029 -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 05/31/2018 07:36 AM, Nick Coghlan wrote:
Why is this? Doesn't the exception have to be instantiated at some point, even if just to print to stderr? -- ~Ethan~
On Thu, 31 May 2018 07:49:58 -0700 Ethan Furman <ethan@stoneleaf.us> wrote:
Nick is talking about exceptions that are ultimately silenced, especially when caught from C code. You're right that, once caught from Python code, the exception *has* to be instantiated (since it is bound to a user-visible variable). Regards Antoine.
Ethan Furman wrote:
Why is this? Doesn't the exception have to be instantiated at some point, even if just to print to stderr?
If it gets caught by an except clause without an else clause, in theory there's no need to instantiate it. However, Python doesn't currently seem to take advantage of that:
-- Greg
On Thu, May 31, 2018 at 10:55 PM Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:
I don't think it's possible to avoid instantiating the exception at the user level, what would sys.exc_info() do about it's second return value? I believe the only cases where it's possible to avoid instantiation are inside the interpreter itself, where the exception never propagates up to user visibility.
Eric Fahlgren wrote:
I don't think it's possible to avoid instantiating the exception at the user level, what would sys.exc_info() do about it's second return value?
The exception could be instantiated if and when sys.exc_info() gets called. -- Greg
On Thu, May 31, 2018 at 10:37 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
The exception machinery deliberately attempts to avoid instantiating
exception objects whenever it can, but that gets significantly more difficult if we always need to create the instance before we can decide whether or not the raised exception matches the given exception handler criteria. Is this really true? Consider the following simple code class E(Exception): def __init__(self): print("instantiated") try: raise E except E: pass Is it truly necessary to instantiate E() in this case? Yet when I run it, I see "instantiated" printed on the console.
On Thu, 31 May 2018 14:00:24 -0400 Alexander Belopolsky <alexander.belopolsky@gmail.com> wrote:
I don't think it's truly necessary, but there's enough complication nowadays in the exception subsystem (e.g. with causes and contexts) that at some point we (perhaps I) decided it made things less hairy to always instantiate it in an "except" clause. Let me stress, though: this happens when catching exceptions in *Python*. When in C you call PyErr_ExceptionMatches, the exception should not get instantiated. Regards Antoine.
On Wed, May 30, 2018 at 10:47 PM, Danilo J. S. Bellini <danilo.bellini@gmail.com> wrote:
Matching on error messages is a recipe for pain. My experience with Go seared that lesson into my brain. (The boilerplate involved there to make custom error types leads to a lot of error message checking instead.)
Using subclasses like this is the way to go. If you control the originating code then make it happen. :) If not then work to fix it upstream. If that fails then at least use a helper that converts the exception after the fact. It could look like following: # Subclassing DatabaseError means existing try-except block will still work the same. class CustomDatabaseError(DatabaseError): .... class BeyondLimitError(CustomDatabaseError): ... class NoFundsError(CustomDatabaseError): ... def convert_exception(exc): if isinstance(exc, CustomDatabaseError): raise msg = get_db_error_msg_from_exception(exc) if msg == "beyond_limit": raise BeyondLimitError(...) elif msg == "no_funds": raise NoFundsError(...) else: raise @contextmanager def converted_exceptions(): try: yield except CustomDatabaseError: raise except DatabaseError as exc: convert_exception(exc) def db_op(...): with converted_exceptions(): # Some code that does DB ops, e.g. session.commit(). ... try: db_op(...) except BeyondLimitError: ... except NoFundsError: ... except DatabaseError: # fallback ... The big wins there are with re-usability, maintainability, and separation of concerns. Notably, it consolidates your message-handling code to one spot and keeps it focused. -eric
participants (14)
-
Alexander Belopolsky -
Antoine Pitrou -
Chris Angelico -
Danilo J. S. Bellini -
Eric Fahlgren -
Eric Snow -
Ethan Furman -
Franklin? Lee -
Greg Ewing -
Guido van Rossum -
Nick Coghlan -
Stephan Houben -
Steven D'Aprano -
Terry Reedy