PRContentsWidget swallowing errors

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

PRContentsWidget swallowing errors

Nick
Hi,

I've noticed that non-validation errors are trapped by the code in PRContentsWidget>>#onAnswerCommand: aCommand

Can anyone anticipate any problems catching MAError rather than Error, so that non-validation errors are propagated as normal and validation errors are displayed. Currently the code reads:

onAnswerCommand: aCommand
aCommand isNil
ifTrue: [ ^ self context: (self context structure: self context structure) ].
[ aCommand execute ]
on: Error
do: [ :err | ^ self component errors add: err ].
self context: aCommand answer


I'd like to change Error to MAError

Have I missed anything?

Nick

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Lukas Renggli
Please give it a try and we will see if there are any bad side-effects.

Lukas

On 25 August 2010 11:02, Nick Ager <[hidden email]> wrote:

> Hi,
> I've noticed that non-validation errors are trapped by the code in
> PRContentsWidget>>#onAnswerCommand: aCommand
> Can anyone anticipate any problems catching MAError rather than Error, so
> that non-validation errors are propagated as normal and validation errors
> are displayed. Currently the code reads:
> onAnswerCommand: aCommand
> aCommand isNil
> ifTrue: [ ^ self context: (self context structure: self context structure)
> ].
> [ aCommand execute ]
> on: Error
> do: [ :err | ^ self component errors add: err ].
> self context: aCommand answer
>
> I'd like to change Error to MAError
> Have I missed anything?
> Nick
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>



--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
Hi Lukas,

Please give it a try and we will see if there are any bad side-effects.

It appears to be working as I'd hoped in *my* Pier application. I'll continue testing and assuming nothing unanticipated occurs I'll check-in the change

Thanks

Nick

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
Hi,

I tried to check this change in but the check-in failed with a 401 unauthorized

I've attached the mcz version

Nick

On 25 August 2010 11:05, Nick Ager <[hidden email]> wrote:
Hi Lukas,

Please give it a try and we will see if there are any bad side-effects.

It appears to be working as I'd hoped in *my* Pier application. I'll continue testing and assuming nothing unanticipated occurs I'll check-in the change

Thanks

Nick


_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki

Pier-Seaside-NickAger.490.mcz (171K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Lukas Renggli
This is strange, are you sure that you use the right initials and
password? You are member with full write access in the groups of
Magritte and Pier.

Lukas

On 26 August 2010 00:50, Nick Ager <[hidden email]> wrote:

> Hi,
> I tried to check this change in but the check-in failed with a 401
> unauthorized
> I've attached the mcz version
> Nick
>
> On 25 August 2010 11:05, Nick Ager <[hidden email]> wrote:
>>
>> Hi Lukas,
>>
>>> Please give it a try and we will see if there are any bad side-effects.
>>
>> It appears to be working as I'd hoped in *my* Pier application. I'll
>> continue testing and assuming nothing unanticipated occurs I'll check-in the
>> change
>> Thanks
>> Nick
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>



--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
Sorry Lukas my mistake - I was using the wrong password. I've now checked-in my changes.

On 26 August 2010 07:06, Lukas Renggli <[hidden email]> wrote:
This is strange, are you sure that you use the right initials and
password? You are member with full write access in the groups of
Magritte and Pier.

Lukas

On 26 August 2010 00:50, Nick Ager <[hidden email]> wrote:
> Hi,
> I tried to check this change in but the check-in failed with a 401
> unauthorized
> I've attached the mcz version
> Nick
>
> On 25 August 2010 11:05, Nick Ager <[hidden email]> wrote:
>>
>> Hi Lukas,
>>
>>> Please give it a try and we will see if there are any bad side-effects.
>>
>> It appears to be working as I'd hoped in *my* Pier application. I'll
>> continue testing and assuming nothing unanticipated occurs I'll check-in the
>> change
>> Thanks
>> Nick
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>



--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki


_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Reza Razavi
In reply to this post by Nick
At 11:02 25/08/2010, Nick Ager wrote:
>I've noticed that non-validation errors are trapped by the code in
>PRContentsWidget>>#onAnswerCommand: aCommand

Hi Nick,

You have certainly also noticed that this is a key method in the
Pier's request handling process. In my experience, and just from a
practical point of view, I found it useful for "production" images to
have here Error instead of MAError, since even in case of an
unexpected error, Pier keeps handling requests gracefully, while
rendering the error message on the client side. For development
images, it would be sufficient to add a 'halt' inside the exception
handling block.

I should add that, in practice, I've encountered rarely such cases,
and, as far as I remember, those cases have always been related to my
own add-ons. Nevertheless, it seems hard to definitely assess that
during the execution of ALL commands, in ALL end-user use cases, the
exceptional situations will strictly be limited to validation errors.
If this makes sense, then Error would be a better choice.

Regards,
Reza  

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Lukas Renggli
In reply to this post by Nick
Excellent, thank you.

Lukas

On Thursday, August 26, 2010, Nick Ager <[hidden email]> wrote:

> Sorry Lukas my mistake - I was using the wrong password. I've now checked-in my changes.
>
> On 26 August 2010 07:06, Lukas Renggli <[hidden email]> wrote:
> This is strange, are you sure that you use the right initials and
> password? You are member with full write access in the groups of
> Magritte and Pier.
>
> Lukas
>
> On 26 August 2010 00:50, Nick Ager <[hidden email]> wrote:
>> Hi,
>> I tried to check this change in but the check-in failed with a 401
>> unauthorized
>> I've attached the mcz version
>> Nick
>>
>> On 25 August 2010 11:05, Nick Ager <[hidden email]> wrote:
>>>
>>> Hi Lukas,
>>>
>>>> Please give it a try and we will see if there are any bad side-effects.
>>>
>>> It appears to be working as I'd hoped in *my* Pier application. I'll
>>> continue testing and assuming nothing unanticipated occurs I'll check-in the
>>> change
>>> Thanks
>>> Nick
>>
>> _______________________________________________
>> Magritte, Pier and Related Tools ...
>> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>>
>
>
>
> --
> Lukas Renggli
> www.lukas-renggli.ch
>
> _______________________________________________
> Magritte, Pier and Related Tools ...
> https://www.iam.unibe.ch/mailman/listinfo/smallwiki
>
>

--
Lukas Renggli
www.lukas-renggli.ch

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
In reply to this post by Reza Razavi
Hi Reza,


In my experience, and just from a practical point of view, I found it useful for "production" images to have here Error instead of MAError, since even in case of an unexpected error, Pier keeps handling requests gracefully, while rendering the error message on the client side. For development images, it would be sufficient to add a 'halt' inside the exception handling block.

I see your point. My understanding for the intent of the code was as a mechanism to display form validation errors, rather than general programming errors. I had a couple of bugs when I ported to Gemstone that were silently consumed (the errors weren't displayed) by PRContentsWidget's error handling and felt that this wasn't the intention of the original code. It seemed to me that programming error exceptions should be propagated and handled in a more general exception handler.

In the case you mention, where 'Pier keeps handling requests gracefully' would the errors make any sense to the user and could they take remedial action to correct the error?

Nick

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Reza Razavi
At 09:36 26/08/2010, Nick Ager wrote:
>were silently consumed (the errors weren't displayed) by
>PRContentsWidget's error handling and felt that this wasn't the
>intention of the original code.

To me, the intention of the original code has for sure not been to
hide errors, but to provide a handy solution to capture unexpected
errors. This method is the "heart" of the Pier's request interpreter
("eval"), and in that sense it seems well situated for processing
those exceptions. Of course, there is room for fine tuning, but my
understanding is that Pier has invested in avoiding such programming
errors. Still, Pier is an extendable system and in that sense
responsible for handling eventual errors in plug-ins. Error instead
of MAError may be also explained in this way.

>In the case you mention, where 'Pier keeps handling requests
>gracefully' would the errors make any sense to the user and could
>they take remedial action to correct the error?

No, programming errors don't, in general, make sense to end-users.
Remedial measures could be taken if the error is related to end-user
actions. For example, feeding unexpected content (not only by Pier,
but also by its plug-ins). But in all cases, for end-users it seems
better to have an application that keeps responsive, eventually with
meaningful error messages, instead of a *wall back* window.

Regards,
Reza        

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
Hi Raza, 

No, programming errors don't, in general, make sense to end-users. Remedial measures could be taken if the error is related to end-user actions. For example, feeding unexpected content (not only by Pier, but also by its plug-ins). But in all cases, for end-users it seems better to have an application that keeps responsive, eventually with meaningful error messages, instead of a *wall back* window.


I agree that you don't want to display walk-back errors to end users though is there a problem changing WAWalkbackErrorHandler to something that makes more sense to the end user?

Nick 

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Reza Razavi
At 10:33 26/08/2010, Nick Ager wrote:
>is there a problem changing WAWalkbackErrorHandler to something that
>makes more sense to the end user?

Seaside provides general mechanisms. More specific solutions depend
on the end-user community that one targets. In my experience, I had
happy end-users with a simple dialog box asking their agreement to
communicate their execution stack, knowing that that context may
contain proprietary information.

Regards,
Reza

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
So what to do? Should I back out the change? 

On 26 August 2010 09:48, Reza Razavi <[hidden email]> wrote:
At 10:33 26/08/2010, Nick Ager wrote:
is there a problem changing WAWalkbackErrorHandler to something that makes more sense to the end user?

Seaside provides general mechanisms. More specific solutions depend on the end-user community that one targets. In my experience, I had happy end-users with a simple dialog box asking their agreement to communicate their execution stack, knowing that that context may contain proprietary information.


Regards,
Reza
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki


_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Reza Razavi
At 10:58 26/08/2010, Nick Ager wrote:
>So what to do? Should I back out the change?

As Lukas suggested, it would be useful to give it a try, if possible
in real test conditions involving end-users, to gain a better idea of
the real risks, at least in your own app's config.

Regards,
Reza

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Nick
OK - it works for me. Let me know if you have problem in your app.

On 26 August 2010 10:17, Reza Razavi <[hidden email]> wrote:
At 10:58 26/08/2010, Nick Ager wrote:
So what to do? Should I back out the change?

As Lukas suggested, it would be useful to give it a try, if possible in real test conditions involving end-users, to gain a better idea of the real risks, at least in your own app's config.


Regards,
Reza
_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki


_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki
Reply | Threaded
Open this post in threaded view
|

Re: PRContentsWidget swallowing errors

Reza Razavi
At 11:51 26/08/2010, Nick Ager wrote:
>Let me know if you have problem in your app.

Sure.
Cheers,
Reza

_______________________________________________
Magritte, Pier and Related Tools ...
https://www.iam.unibe.ch/mailman/listinfo/smallwiki