Ticket #555 (defect)
Opened 2 years ago
Last modified 2 years ago
Error propagation in hooks
Status: closed (fixed)
| Reported by: | lawouach | Assigned to: | lawouach |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.0 |
| Component: | CherryPy code | Keywords: | |
| Cc: |
In the run method of the Hook class, only KeyboardInterrupt?, SystemExit? are re-raised, other errors are not. This is all fine except for CherryPy own errors.
I have for instance two hooks checking differentaspect of my requests. One of them raises a cherrypy.HTTPError(415, 'Unsupported Media Type') in a 'on_start_resource', thus very early in the process. Currently this error is logged but doesn ot stop the process.
I think this is not a correct behavior. I'd rather re-raise that error and thus avoid processing further more.
So what about changing the first except of the run() method from: except (KeyboardInterrupt?, SystemExit?)
to: except (cherrypy.HTTPRedirect, cherrypy.HTTPError, KeyboardInterrupt?, SystemExit?)
Allowing tools to shortcut the all process when needed.
Change History
08/20/06 15:00:29: Modified by fumanchu
08/21/06 12:41:18: Modified by fumanchu
- status changed from new to closed.
- resolution set to fixed.
- summary changed from Eroor propagation in hooks to Error propagation in hooks.
Fixed in [1261].
Failsafe flags are now per-callback, not per-callpoint. If you want a given hook to be failsafe, either set callback.failsafe = True or (if you cannot do that) call attach(point, callback, failsafe=True).
The previous semantic of 'failsafe' unfortunately also meant 'fail silently'. This has been corrected; now, when errors occur during the run of callbacks for a given call point, the last one will be raised (after all failsafe callbacks have completed).
I understand the need, but the solution is probably too broad as stated. For example, you probably want all on_end hooks (which are mostly resource cleanup) to run even if one of them raises an HTTPError.
I think we're going to have to bite the bullet and associate a failsafe flag to each registered hook callback, rather thatn associating it with all-hooks-for-a-given-hookpoint. We can have certain hookpoints default differently, but the writer of the callback should have the ability to specify whether that function must be guaranteed to run or not.