On 11 Jul 2013, at 18:05, Nicolas Cellier <nicolas.cellier.aka.nice@gmail.com> wrote:
Catching all/any Exception is generally considered a code smell
In Pharo 3.0 AssertionFailure inherits from Error, which takes away my reason to catch Exception instead of Error.
2013/7/11 Sven Van Caekenberghe <sven@stfx.eu>
On 11 Jul 2013, at 16:08, Vincent Blondeau <vincent.blondeau@polytech-lille.net> wrote:
Le 11/07/2013 15:42, Sven Van Caekenberghe a écrit :
Vincent,
On 11 Jul 2013, at 15:06, Vincent Blondeau <vincent.blondeau@polytech-lille.net> wrote:
I use a Zinc Server. So when I do the request, Zinc catch the Exception and send an error as response. ZnSingleThreadedServer>>#handleRequestProtected: request "Handle request and return a response. If a Smalltalk Error is thrown, return a HTTP Server Error response."
^ [ self authenticateAndDelegateRequest: request ] on: Error do: [ :exception | self debugMode ifTrue: [ exception pass ] ifFalse: [ self logServerError: exception. ZnResponse serverError: exception printString ] ]
AFAICT this code catches Error, not Exception, hence it would let Notification through. No ?
Sven
Indeed but I use the Zinc-Rest-Server and all Exceptions are caught:
ZnRestServerDelegate>>handleRequest: request | call | (call := self match: request) ifNil: [ ^ self noHandlerFound: request ]. (self authenticate: call) ifFalse: [ ^ self callUnauthorized: request ]. ^ [ self execute: call ] on: Exception do: [ :exception | request server debugMode ifTrue: [ exception pass ] ifFalse: [ request server logServerError: exception. self serverError: request exception: exception ] ]
Indeed.
I know why I wrote that (I often write assertions in code, and AssertionFailed is not an Error, hence the broader net) - I will think about it.
In the mean time, you could subclass ZnRestServerDelegate and override #handleRequest:
Maybe I can also improve the overriding options a bit, like with an introduction of an #exceptionsToCatch accessor.
HTH,
Sven
-- Sven Van Caekenberghe Proudly supporting Pharo http://pharo.org http://association.pharo.org http://consortium.pharo.org