On Sun, Nov 15, 2009 at 12:58, Edi Weitz edi@agharta.de wrote:
Yes, I agree. I was probably a bit too over-enthusiastic with the third bit... :)
Still, I only have a finite amount of time. The Hunchentoot-specific error handling was thrown out with the 1.0.0 release and if we add something similar back in, I want it to be a reasonable, working, flexible, and well-documented solution. That takes time, and I'll do it when I have that time, this being not a priority for me right now.
I have spent the last day working on a patch (incl. documentation) to include an error-handling mechanism in Hunchentoot, similar to the solution Peter posted some way up the thread (which was indeed a quick hack by me).
This patch introduces an error handling protocol for connections and request handling. It introduces a new class and two new generic functions (all three exported and documented):
* defclass debugging-acceptor: An acceptor that doesn't catch errors, instead relies on the implementation's debugger hook to catch the error and report it synchronously.
* defgeneric invoke-process-connection-with-error-handling: An error handling mechanism for connection processing: Behavior for the regular hunchentoot:acceptor is the current default behavior. debuggable-acceptors only give you the debugger if their debug-connection-errors-p slot is non-NIL (NIL is the default)
* defgeneric invoke-process-request-with-error-handling: An error handling mechanism for request handling: As before, the default behavior is in place for all subclasses of hunchentoot:acceptor except for debugging-acceptor. There, it invokes the debugger for signals raised with CL:ERROR.
I hope very much that you find this worth including in the hunchentoot distribution. Of course, feedback is welcome. (-:
Cheers,
Hi Andreas,
I took a quick look and this seems like a good start to me. I'm doing something else today, but I was going to commit the patch to the BKNR repository so that people could try it out and comment, but the patch doesn't apply. Did you work against the development version?
Thanks, Edi.
patching file acceptor.lisp Hunk #1 succeeded at 174 (offset 1 line). Hunk #2 succeeded at 247 (offset 1 line). Hunk #3 FAILED at 293. Hunk #4 succeeded at 464 (offset 8 lines). 1 out of 4 hunks FAILED -- saving rejects to file acceptor.lisp.rej patching file doc/index.xml Hunk #1 succeeded at 351 (offset 3 lines). Hunk #2 succeeded at 662 (offset 3 lines). patching file packages.lisp Hunk #1 succeeded at 133 (offset 1 line). patching file request.lisp Hunk #1 FAILED at 219. 1 out of 1 hunk FAILED -- saving rejects to file request.lisp.rej
On Sun, Nov 15, 2009 at 3:07 PM, Andreas Fuchs asf@boinkor.net wrote:
On Sun, Nov 15, 2009 at 12:58, Edi Weitz edi@agharta.de wrote:
Yes, I agree. I was probably a bit too over-enthusiastic with the third bit... :)
Still, I only have a finite amount of time. The Hunchentoot-specific error handling was thrown out with the 1.0.0 release and if we add something similar back in, I want it to be a reasonable, working, flexible, and well-documented solution. That takes time, and I'll do it when I have that time, this being not a priority for me right now.
I have spent the last day working on a patch (incl. documentation) to include an error-handling mechanism in Hunchentoot, similar to the solution Peter posted some way up the thread (which was indeed a quick hack by me).
This patch introduces an error handling protocol for connections and request handling. It introduces a new class and two new generic functions (all three exported and documented):
- defclass debugging-acceptor: An acceptor that doesn't catch errors,
instead relies on the implementation's debugger hook to catch the error and report it synchronously.
- defgeneric invoke-process-connection-with-error-handling: An error
handling mechanism for connection processing: Behavior for the regular hunchentoot:acceptor is the current default behavior. debuggable-acceptors only give you the debugger if their debug-connection-errors-p slot is non-NIL (NIL is the default)
- defgeneric invoke-process-request-with-error-handling: An error
handling mechanism for request handling: As before, the default behavior is in place for all subclasses of hunchentoot:acceptor except for debugging-acceptor. There, it invokes the debugger for signals raised with CL:ERROR.
I hope very much that you find this worth including in the hunchentoot distribution. Of course, feedback is welcome. (-:
Cheers,
Andreas Fuchs, (http://%7Cim:asf@%7Cmailto:asf@)boinkor.net, antifuchs
On Sun, Nov 15, 2009 at 16:17, Edi Weitz edi@agharta.de wrote:
Hi Andreas,
I took a quick look and this seems like a good start to me. I'm doing something else today, but I was going to commit the patch to the BKNR repository so that people could try it out and comment, but the patch doesn't apply. Did you work against the development version?
Oh, no - that's a diff against 1.0.0; 1.0.0 was on which I based the patch (from clbuild's http://common-lisp.net/~loliveira/ediware/hunchentoot darcs repo), as well.
Should I bring it forward to what's in the BKNR repository?
Cheers,
On Sun, Nov 15, 2009 at 4:22 PM, Andreas Fuchs asf@boinkor.net wrote:
On Sun, Nov 15, 2009 at 16:17, Edi Weitz edi@agharta.de wrote:
Hi Andreas,
I took a quick look and this seems like a good start to me. I'm doing something else today, but I was going to commit the patch to the BKNR repository so that people could try it out and comment, but the patch doesn't apply. Did you work against the development version?
Oh, no - that's a diff against 1.0.0; 1.0.0 was on which I based the patch (from clbuild's http://common-lisp.net/~loliveira/ediware/hunchentoot darcs repo), as well.
Should I bring it forward to what's in the BKNR repository?
That would be very nice, yes. I think we should have this change linger around in the dev version a bit before we make a new release.
On Sun, Nov 15, 2009 at 17:53, Edi Weitz edi@agharta.de wrote:
Should I bring it forward to what's in the BKNR repository?
That would be very nice, yes. I think we should have this change linger around in the dev version a bit before we make a new release.
Sounds sensible. I've tested the attached patch to r4467 on the latest SBCL from CVS, and it seemed to work, so I think I should have ported it correctly. One more reason to let it stew in the dev version a bit longer, I guess (-:
Cheers,
On Sun, Nov 15, 2009 at 7:28 PM, Andreas Fuchs asf@boinkor.net wrote:
Sounds sensible. I've tested the attached patch to r4467 on the latest SBCL from CVS, and it seemed to work, so I think I should have ported it correctly. One more reason to let it stew in the dev version a bit longer, I guess (-:
Thanks, it's online now.
http://bknr.net/trac/log/trunk/thirdparty/hunchentoot/
Edi.
Hans mentioned this today and I now that I found some time to think about it, I tend to agree with him:
The problem with the new approach is that if you want to debug your web app, you need to change the acceptor class. That's a bit inconvenient compared to the good old days of setting/binding a special variable, isn't it?
Shouldn't the standard acceptor class provide the ability to switch debugging on and off at runtime? Maybe with a (class) slot or something instead of a special variable? The protocol that you added is I think still useful and should be kept nevertheless.
Thought?
Thanks, Edi.
On Mon, Nov 16, 2009 at 6:58 AM, Edi Weitz edi@agharta.de wrote:
Hans mentioned this today and I now that I found some time to think about it, I tend to agree with him:
The problem with the new approach is that if you want to debug your web app, you need to change the acceptor class. That's a bit inconvenient compared to the good old days of setting/binding a special variable, isn't it?
Hey, you're the guy who took out the special variable. ;-)
Shouldn't the standard acceptor class provide the ability to switch debugging on and off at runtime? Maybe with a (class) slot or something instead of a special variable? The protocol that you added is I think still useful and should be kept nevertheless.
Thought?
I agree with your new position--it's more useful to be able to change the behavior of a running server to choose between logging, ignoring, or debugging errors just by setting a variable. Whether it's in a slot or special variable doesn't matter a ton. Though I could imagine cases where a special variable would be handy--you could change the policy in one piece of application code without affecting others.
-Peter
On Mon, Nov 16, 2009 at 5:24 PM, Peter Seibel peter@gigamonkeys.com wrote:
Hey, you're the guy who took out the special variable. ;-)
I blame it on Hans... :)
I agree with your new position--it's more useful to be able to change the behavior of a running server to choose between logging, ignoring, or debugging errors just by setting a variable. Whether it's in a slot or special variable doesn't matter a ton. Though I could imagine cases where a special variable would be handy--you could change the policy in one piece of application code without affecting others.
At the handler level it wouldn't be possible. Right now, I don't think a special variable would buy you more flexibility given the Hunchentoot architecture. But I've been wrong before... :)
On Mon, Nov 16, 2009 at 17:35, Edi Weitz edi@agharta.de wrote:
On Mon, Nov 16, 2009 at 5:24 PM, Peter Seibel peter@gigamonkeys.com wrote:
Hey, you're the guy who took out the special variable. ;-)
I blame it on Hans... :)
I happily take a lot of blame, but this I need to decline! You were the one who convinced me that *break-on-signals* is all you need, and who am I to dissent? :P
On Mon, Nov 16, 2009 at 5:41 PM, Hans Hübner hans.huebner@gmail.com wrote:
I happily take a lot of blame, but this I need to decline! You were the one who convinced me that *break-on-signals* is all you need, and who am I to dissent? :P
Really? I'm getting old...
Hans Hübner -> Edi Weitz
You were the one who convinced me that *break-on-signals* is all you need, and who am I to dissent? :P
Just as an apropos to the discussion: I just had the cunning idea to use an around method on process-request to set up some dynamic variables that were initially unbound, and with a handler-bind that would compute the init-forms upon first use of the variable, thus only computing the bindings' init-forms when they are actually required. The *break-on-signals* approach would not mix well with this technique.
On Thu, Nov 19, 2009 at 09:45, Frode V. Fjeld frode@netfonds.no wrote:
Hans Hübner -> Edi Weitz
You were the one who convinced me that *break-on-signals* is all you need, and who am I to dissent? :P
Just as an apropos to the discussion: I just had the cunning idea to use an around method on process-request to set up some dynamic variables that were initially unbound, and with a handler-bind that would compute the init-forms upon first use of the variable, thus only computing the bindings' init-forms when they are actually required. The *break-on-signals* approach would not mix well with this technique.
To continue flogging a dead horse: I consented with the removal of the hunchentoot specific debugging mechanism because I still have something of a C++ mindset, part of which is considering exception handling as being exclusively for error handling. In C++, exception handling has undefined performance characteristics, which is why it is considered bad style to use it in non-error circumstances. In Lisp, performance is generally not that well-defined, and it is generally more open to multiple styles and paradigms, so restricting the use of signals to genuine error handling is much less sensible.
So: Apologies :)
Edi Weitz wrote:
At the handler level it wouldn't be possible. Right now, I don't think a special variable would buy you more flexibility given the Hunchentoot architecture. But I've been wrong before... :)
FWIW in Weblocks we have settled down to a scheme that's similar to the one that used to be in Hunchentoot 0. Our API lets the developer decide whether he wants to have the debugger invoked or rather have the error handled by an error page.
In another application (a command-line tool) we have three choices: invoke debugger, exit with error, exit after printing backtrace.
Leslie
-
Andreas Fuchs -
Edi Weitz -
frode＠netfonds.no -
Hans Hübner -
Leslie P. Polzer -
Peter Seibel