You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martin Grigorov <mg...@apache.org> on 2014/06/02 21:39:12 UTC
Marker interface for "internal" exceptions
Hi,
Recently I had the chance to review some customer applications and I've
noticed a pattern in both applications:
a custom impl of IRequestCycleListener#onException() checks whether the
given exception is instance of something that the application knows how to
deal with and if it is not then a RenderPageRequestHandler is returned
trying to render the last successfully rendered page. I.e. the idea is to
show something like an error feedback in the last/current page instead of
redirecting to InternalErrorPage.
The problem in both impls was that internal/special exceptions like
ResponseIOException, PackageResource.PackageResourceBlockedException and
StalePageException (and maybe ListenerInvocationNotAllowedException,
AuthorizationException) have been managed by the custom code instead of
letting Wicket to do its business (in DefaultExceptionMapper).
The fix was to add a check for these exceptions and do nothing. But Wicket
can add more such special exceptions in the future and the application
logic may break...
What do you think about introducing a special marker interface for all
exceptions which represent some erroneous state and be better handled by
Wicket rather than by custom code ?
This way a new exception type will handled automatically in the future.
Or maybe you have a better idea ?
Martin Grigorov
Wicket Training and Consulting
Re: Marker interface for "internal" exceptions
Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Yes you are absolutely right.
On Mon, Jun 2, 2014 at 10:50 PM, Martin Grigorov <mg...@apache.org>
wrote:
> Hi Ernesto,
>
>
> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
> reiern70@gmail.com> wrote:
>
> > Martin,
> >
> > Just an idea,
> >
> > Besides your marker interface maybe IExceptionMapper can be re-factored
> as
> >
> > 1-
> > public interface IExceptionMapper
> > {
> > /** return true if it knows how to handle and exception */
> > boolean canHandle(Exception e);
> >
> > /**
> > * @param e
> > *
> > * @return {@link IRequestHandler} for given exception
> > */
> > IRequestHandler map(Exception e);
> > }
> >
> > 2- Allow a list a of IExceptionMapper's. So, that user can register
> mappers
> > before the default one.
> > 3- This list will be iterated and ask canHandle(e) on each mapper to see
> if
> > it can handle the exception. The default mapper could handle "all"
> > exceptions not handle by previous IExceptionMappers.
> >
> > Maybe overkill?
> >
>
> What is the difference with the current way ?
>
> Now we have a list of IRequestCycleListeners and Wicket asks them one by
> one until some returns non-null. If all return null then the last resort is
> used - the IExceptionMapper.
>
>
> >
> > On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
> > wrote:
> >
> > > Hi,
> > >
> > > Recently I had the chance to review some customer applications and I've
> > > noticed a pattern in both applications:
> > >
> > > a custom impl of IRequestCycleListener#onException() checks whether the
> > > given exception is instance of something that the application knows how
> > to
> > > deal with and if it is not then a RenderPageRequestHandler is returned
> > > trying to render the last successfully rendered page. I.e. the idea is
> to
> > > show something like an error feedback in the last/current page instead
> of
> > > redirecting to InternalErrorPage.
> > >
> > > The problem in both impls was that internal/special exceptions like
> > > ResponseIOException, PackageResource.PackageResourceBlockedException
> and
> > > StalePageException (and maybe ListenerInvocationNotAllowedException,
> > > AuthorizationException) have been managed by the custom code instead of
> > > letting Wicket to do its business (in DefaultExceptionMapper).
> > >
> > > The fix was to add a check for these exceptions and do nothing. But
> > Wicket
> > > can add more such special exceptions in the future and the application
> > > logic may break...
> > >
> > > What do you think about introducing a special marker interface for all
> > > exceptions which represent some erroneous state and be better handled
> by
> > > Wicket rather than by custom code ?
> > > This way a new exception type will handled automatically in the future.
> > >
> > > Or maybe you have a better idea ?
> > >
> > >
> > > Martin Grigorov
> > > Wicket Training and Consulting
> > >
> >
> >
> >
> > --
> > Regards - Ernesto Reinaldo Barreiro
> >
>
--
Regards - Ernesto Reinaldo Barreiro
Re: Marker interface for "internal" exceptions
Posted by Martin Grigorov <mg...@apache.org>.
It is handled, but it is thrown if a component or behavior in the page is
not "usable" for some reason.
But the rest of the page could be still usable and that's why I think it is
not one of the special ones.
Same for AuthorizationException - it could be a problem with the page
itself but it could be a problem with a specific component within the page.
Martin Grigorov
Wicket Training and Consulting
On Wed, Jun 4, 2014 at 7:05 PM, Sven Meier <sv...@meiers.net> wrote:
> What about ListenerInvocationNotAllowedException?
>
> It is 'handled' by DefaultExceptionMapper too.
>
> Regards
> Sven
>
>
> On 06/03/2014 09:03 AM, Martin Grigorov wrote:
>
>> Looking at DefaultExceptionMapper#internalMap() I see the following
>> candidates:
>> - org.apache.wicket.core.request.mapper.StalePageException
>> - org.apache.wicket.protocol.http.servlet.ResponseIOException
>> - org.apache.wicket.request.resource.PackageResource.
>> PackageResourceBlockedException
>>
>> Possible names of the marker:
>> - WicketBadState
>> - WicketInternalException
>> - WicketFrameworkException
>>
>>
>> Martin Grigorov
>> Wicket Training and Consulting
>>
>>
>> On Tue, Jun 3, 2014 at 6:34 AM, Sven Meier <sv...@meiers.net> wrote:
>>
>> What would be the name of that interface and which classes would
>>> implement
>>> it?
>>>
>>> Sven
>>>
>>>
>>> On 06/02/2014 11:27 PM, Martin Grigorov wrote:
>>>
>>> On Mon, Jun 2, 2014 at 11:14 PM, Sven Meier <sv...@meiers.net> wrote:
>>>>
>>>> Hi,
>>>>
>>>>> the problem I see with an exception interface or #canHandleException()
>>>>> is
>>>>> that both promise something (i.e. this exception will be handled) but
>>>>> you
>>>>> cannot be sure that this promise will be kept.
>>>>> What if you have installed an alternative IExceptionMapper which maps
>>>>> things differently? Or #canHandleException() get's out of sync with the
>>>>> actual code in #map(Exception)?
>>>>>
>>>>> How about adding a hook method #onMapped(Exception,IRequestHandler) to
>>>>> DefaultExceptionMapper. Subclasses could check for
>>>>> RenderPageRequeatHandlers *after* Wicket's default mapping was
>>>>> performed. A
>>>>> subclass could return an override with the previous page and an error
>>>>> message instead.
>>>>>
>>>>> I think the application code should not deal with "special"
>>>>> exceptions
>>>>>
>>>> like
>>>> ResponseIOException (because this means the connection to the browser is
>>>> closed) or StalePageException (because this means the previous page is
>>>> stale/obsolete for some reason and needs to be re-rendered to be useful
>>>> again).
>>>> The idea of the special marker interface is to tell the application "you
>>>> better leave this problem to be handled by the framework".
>>>> By using this marker interface the application code will be
>>>> future-proof.
>>>>
>>>>
>>>> My 2 cents
>>>>
>>>>> Sven
>>>>>
>>>>>
>>>>>
>>>>> On 06/02/2014 10:50 PM, Martin Grigorov wrote:
>>>>>
>>>>> Hi Ernesto,
>>>>>
>>>>>>
>>>>>> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
>>>>>> reiern70@gmail.com> wrote:
>>>>>>
>>>>>> Martin,
>>>>>>
>>>>>> Just an idea,
>>>>>>>
>>>>>>> Besides your marker interface maybe IExceptionMapper can be
>>>>>>> re-factored
>>>>>>> as
>>>>>>>
>>>>>>> 1-
>>>>>>> public interface IExceptionMapper
>>>>>>> {
>>>>>>> /** return true if it knows how to handle and exception */
>>>>>>> boolean canHandle(Exception e);
>>>>>>>
>>>>>>> /**
>>>>>>> * @param e
>>>>>>> *
>>>>>>> * @return {@link IRequestHandler} for given exception
>>>>>>> */
>>>>>>> IRequestHandler map(Exception e);
>>>>>>> }
>>>>>>>
>>>>>>> 2- Allow a list a of IExceptionMapper's. So, that user can register
>>>>>>> mappers
>>>>>>> before the default one.
>>>>>>> 3- This list will be iterated and ask canHandle(e) on each mapper to
>>>>>>> see
>>>>>>> if
>>>>>>> it can handle the exception. The default mapper could handle "all"
>>>>>>> exceptions not handle by previous IExceptionMappers.
>>>>>>>
>>>>>>> Maybe overkill?
>>>>>>>
>>>>>>> What is the difference with the current way ?
>>>>>>>
>>>>>>> Now we have a list of IRequestCycleListeners and Wicket asks them
>>>>>> one by
>>>>>> one until some returns non-null. If all return null then the last
>>>>>> resort
>>>>>> is
>>>>>> used - the IExceptionMapper.
>>>>>>
>>>>>>
>>>>>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <
>>>>>> mgrigorov@apache.org
>>>>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Recently I had the chance to review some customer applications and
>>>>>>>> I've
>>>>>>>> noticed a pattern in both applications:
>>>>>>>>
>>>>>>>> a custom impl of IRequestCycleListener#onException() checks whether
>>>>>>>> the
>>>>>>>> given exception is instance of something that the application knows
>>>>>>>> how
>>>>>>>>
>>>>>>>> to
>>>>>>>>
>>>>>>> deal with and if it is not then a RenderPageRequestHandler is
>>>>>>> returned
>>>>>>>
>>>>>>>> trying to render the last successfully rendered page. I.e. the idea
>>>>>>>> is
>>>>>>>> to
>>>>>>>> show something like an error feedback in the last/current page
>>>>>>>> instead
>>>>>>>> of
>>>>>>>> redirecting to InternalErrorPage.
>>>>>>>>
>>>>>>>> The problem in both impls was that internal/special exceptions like
>>>>>>>> ResponseIOException, PackageResource.PackageResourceBlockedExceptio
>>>>>>>> n
>>>>>>>> and
>>>>>>>> StalePageException (and maybe ListenerInvocationNotAllowedEx
>>>>>>>> ception,
>>>>>>>> AuthorizationException) have been managed by the custom code instead
>>>>>>>> of
>>>>>>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>>>>>>
>>>>>>>> The fix was to add a check for these exceptions and do nothing. But
>>>>>>>>
>>>>>>>> Wicket
>>>>>>>>
>>>>>>> can add more such special exceptions in the future and the
>>>>>>> application
>>>>>>>
>>>>>>>> logic may break...
>>>>>>>>
>>>>>>>> What do you think about introducing a special marker interface for
>>>>>>>> all
>>>>>>>> exceptions which represent some erroneous state and be better
>>>>>>>> handled
>>>>>>>> by
>>>>>>>> Wicket rather than by custom code ?
>>>>>>>> This way a new exception type will handled automatically in the
>>>>>>>> future.
>>>>>>>>
>>>>>>>> Or maybe you have a better idea ?
>>>>>>>>
>>>>>>>>
>>>>>>>> Martin Grigorov
>>>>>>>> Wicket Training and Consulting
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>> Regards - Ernesto Reinaldo Barreiro
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>
Re: Marker interface for "internal" exceptions
Posted by Sven Meier <sv...@meiers.net>.
What about ListenerInvocationNotAllowedException?
It is 'handled' by DefaultExceptionMapper too.
Regards
Sven
On 06/03/2014 09:03 AM, Martin Grigorov wrote:
> Looking at DefaultExceptionMapper#internalMap() I see the following
> candidates:
> - org.apache.wicket.core.request.mapper.StalePageException
> - org.apache.wicket.protocol.http.servlet.ResponseIOException
> - org.apache.wicket.request.resource.PackageResource.PackageResourceBlockedException
>
> Possible names of the marker:
> - WicketBadState
> - WicketInternalException
> - WicketFrameworkException
>
>
> Martin Grigorov
> Wicket Training and Consulting
>
>
> On Tue, Jun 3, 2014 at 6:34 AM, Sven Meier <sv...@meiers.net> wrote:
>
>> What would be the name of that interface and which classes would implement
>> it?
>>
>> Sven
>>
>>
>> On 06/02/2014 11:27 PM, Martin Grigorov wrote:
>>
>>> On Mon, Jun 2, 2014 at 11:14 PM, Sven Meier <sv...@meiers.net> wrote:
>>>
>>> Hi,
>>>> the problem I see with an exception interface or #canHandleException() is
>>>> that both promise something (i.e. this exception will be handled) but you
>>>> cannot be sure that this promise will be kept.
>>>> What if you have installed an alternative IExceptionMapper which maps
>>>> things differently? Or #canHandleException() get's out of sync with the
>>>> actual code in #map(Exception)?
>>>>
>>>> How about adding a hook method #onMapped(Exception,IRequestHandler) to
>>>> DefaultExceptionMapper. Subclasses could check for
>>>> RenderPageRequeatHandlers *after* Wicket's default mapping was
>>>> performed. A
>>>> subclass could return an override with the previous page and an error
>>>> message instead.
>>>>
>>>> I think the application code should not deal with "special" exceptions
>>> like
>>> ResponseIOException (because this means the connection to the browser is
>>> closed) or StalePageException (because this means the previous page is
>>> stale/obsolete for some reason and needs to be re-rendered to be useful
>>> again).
>>> The idea of the special marker interface is to tell the application "you
>>> better leave this problem to be handled by the framework".
>>> By using this marker interface the application code will be future-proof.
>>>
>>>
>>> My 2 cents
>>>> Sven
>>>>
>>>>
>>>>
>>>> On 06/02/2014 10:50 PM, Martin Grigorov wrote:
>>>>
>>>> Hi Ernesto,
>>>>>
>>>>> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
>>>>> reiern70@gmail.com> wrote:
>>>>>
>>>>> Martin,
>>>>>
>>>>>> Just an idea,
>>>>>>
>>>>>> Besides your marker interface maybe IExceptionMapper can be re-factored
>>>>>> as
>>>>>>
>>>>>> 1-
>>>>>> public interface IExceptionMapper
>>>>>> {
>>>>>> /** return true if it knows how to handle and exception */
>>>>>> boolean canHandle(Exception e);
>>>>>>
>>>>>> /**
>>>>>> * @param e
>>>>>> *
>>>>>> * @return {@link IRequestHandler} for given exception
>>>>>> */
>>>>>> IRequestHandler map(Exception e);
>>>>>> }
>>>>>>
>>>>>> 2- Allow a list a of IExceptionMapper's. So, that user can register
>>>>>> mappers
>>>>>> before the default one.
>>>>>> 3- This list will be iterated and ask canHandle(e) on each mapper to
>>>>>> see
>>>>>> if
>>>>>> it can handle the exception. The default mapper could handle "all"
>>>>>> exceptions not handle by previous IExceptionMappers.
>>>>>>
>>>>>> Maybe overkill?
>>>>>>
>>>>>> What is the difference with the current way ?
>>>>>>
>>>>> Now we have a list of IRequestCycleListeners and Wicket asks them one by
>>>>> one until some returns non-null. If all return null then the last resort
>>>>> is
>>>>> used - the IExceptionMapper.
>>>>>
>>>>>
>>>>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mgrigorov@apache.org
>>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>> Recently I had the chance to review some customer applications and
>>>>>>> I've
>>>>>>> noticed a pattern in both applications:
>>>>>>>
>>>>>>> a custom impl of IRequestCycleListener#onException() checks whether
>>>>>>> the
>>>>>>> given exception is instance of something that the application knows
>>>>>>> how
>>>>>>>
>>>>>>> to
>>>>>> deal with and if it is not then a RenderPageRequestHandler is returned
>>>>>>> trying to render the last successfully rendered page. I.e. the idea is
>>>>>>> to
>>>>>>> show something like an error feedback in the last/current page instead
>>>>>>> of
>>>>>>> redirecting to InternalErrorPage.
>>>>>>>
>>>>>>> The problem in both impls was that internal/special exceptions like
>>>>>>> ResponseIOException, PackageResource.PackageResourceBlockedException
>>>>>>> and
>>>>>>> StalePageException (and maybe ListenerInvocationNotAllowedException,
>>>>>>> AuthorizationException) have been managed by the custom code instead
>>>>>>> of
>>>>>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>>>>>
>>>>>>> The fix was to add a check for these exceptions and do nothing. But
>>>>>>>
>>>>>>> Wicket
>>>>>> can add more such special exceptions in the future and the application
>>>>>>> logic may break...
>>>>>>>
>>>>>>> What do you think about introducing a special marker interface for all
>>>>>>> exceptions which represent some erroneous state and be better handled
>>>>>>> by
>>>>>>> Wicket rather than by custom code ?
>>>>>>> This way a new exception type will handled automatically in the
>>>>>>> future.
>>>>>>>
>>>>>>> Or maybe you have a better idea ?
>>>>>>>
>>>>>>>
>>>>>>> Martin Grigorov
>>>>>>> Wicket Training and Consulting
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>> Regards - Ernesto Reinaldo Barreiro
>>>>>>
>>>>>>
>>>>>>
Re: Marker interface for "internal" exceptions
Posted by Martin Grigorov <mg...@apache.org>.
Looking at DefaultExceptionMapper#internalMap() I see the following
candidates:
- org.apache.wicket.core.request.mapper.StalePageException
- org.apache.wicket.protocol.http.servlet.ResponseIOException
- org.apache.wicket.request.resource.PackageResource.PackageResourceBlockedException
Possible names of the marker:
- WicketBadState
- WicketInternalException
- WicketFrameworkException
Martin Grigorov
Wicket Training and Consulting
On Tue, Jun 3, 2014 at 6:34 AM, Sven Meier <sv...@meiers.net> wrote:
> What would be the name of that interface and which classes would implement
> it?
>
> Sven
>
>
> On 06/02/2014 11:27 PM, Martin Grigorov wrote:
>
>> On Mon, Jun 2, 2014 at 11:14 PM, Sven Meier <sv...@meiers.net> wrote:
>>
>> Hi,
>>>
>>> the problem I see with an exception interface or #canHandleException() is
>>> that both promise something (i.e. this exception will be handled) but you
>>> cannot be sure that this promise will be kept.
>>> What if you have installed an alternative IExceptionMapper which maps
>>> things differently? Or #canHandleException() get's out of sync with the
>>> actual code in #map(Exception)?
>>>
>>> How about adding a hook method #onMapped(Exception,IRequestHandler) to
>>> DefaultExceptionMapper. Subclasses could check for
>>> RenderPageRequeatHandlers *after* Wicket's default mapping was
>>> performed. A
>>> subclass could return an override with the previous page and an error
>>> message instead.
>>>
>>> I think the application code should not deal with "special" exceptions
>> like
>> ResponseIOException (because this means the connection to the browser is
>> closed) or StalePageException (because this means the previous page is
>> stale/obsolete for some reason and needs to be re-rendered to be useful
>> again).
>> The idea of the special marker interface is to tell the application "you
>> better leave this problem to be handled by the framework".
>> By using this marker interface the application code will be future-proof.
>>
>>
>> My 2 cents
>>> Sven
>>>
>>>
>>>
>>> On 06/02/2014 10:50 PM, Martin Grigorov wrote:
>>>
>>> Hi Ernesto,
>>>>
>>>>
>>>> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
>>>> reiern70@gmail.com> wrote:
>>>>
>>>> Martin,
>>>>
>>>>> Just an idea,
>>>>>
>>>>> Besides your marker interface maybe IExceptionMapper can be re-factored
>>>>> as
>>>>>
>>>>> 1-
>>>>> public interface IExceptionMapper
>>>>> {
>>>>> /** return true if it knows how to handle and exception */
>>>>> boolean canHandle(Exception e);
>>>>>
>>>>> /**
>>>>> * @param e
>>>>> *
>>>>> * @return {@link IRequestHandler} for given exception
>>>>> */
>>>>> IRequestHandler map(Exception e);
>>>>> }
>>>>>
>>>>> 2- Allow a list a of IExceptionMapper's. So, that user can register
>>>>> mappers
>>>>> before the default one.
>>>>> 3- This list will be iterated and ask canHandle(e) on each mapper to
>>>>> see
>>>>> if
>>>>> it can handle the exception. The default mapper could handle "all"
>>>>> exceptions not handle by previous IExceptionMappers.
>>>>>
>>>>> Maybe overkill?
>>>>>
>>>>> What is the difference with the current way ?
>>>>>
>>>> Now we have a list of IRequestCycleListeners and Wicket asks them one by
>>>> one until some returns non-null. If all return null then the last resort
>>>> is
>>>> used - the IExceptionMapper.
>>>>
>>>>
>>>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mgrigorov@apache.org
>>>> >
>>>>
>>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>>> Recently I had the chance to review some customer applications and
>>>>>> I've
>>>>>> noticed a pattern in both applications:
>>>>>>
>>>>>> a custom impl of IRequestCycleListener#onException() checks whether
>>>>>> the
>>>>>> given exception is instance of something that the application knows
>>>>>> how
>>>>>>
>>>>>> to
>>>>>
>>>>> deal with and if it is not then a RenderPageRequestHandler is returned
>>>>>> trying to render the last successfully rendered page. I.e. the idea is
>>>>>> to
>>>>>> show something like an error feedback in the last/current page instead
>>>>>> of
>>>>>> redirecting to InternalErrorPage.
>>>>>>
>>>>>> The problem in both impls was that internal/special exceptions like
>>>>>> ResponseIOException, PackageResource.PackageResourceBlockedException
>>>>>> and
>>>>>> StalePageException (and maybe ListenerInvocationNotAllowedException,
>>>>>> AuthorizationException) have been managed by the custom code instead
>>>>>> of
>>>>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>>>>
>>>>>> The fix was to add a check for these exceptions and do nothing. But
>>>>>>
>>>>>> Wicket
>>>>>
>>>>> can add more such special exceptions in the future and the application
>>>>>> logic may break...
>>>>>>
>>>>>> What do you think about introducing a special marker interface for all
>>>>>> exceptions which represent some erroneous state and be better handled
>>>>>> by
>>>>>> Wicket rather than by custom code ?
>>>>>> This way a new exception type will handled automatically in the
>>>>>> future.
>>>>>>
>>>>>> Or maybe you have a better idea ?
>>>>>>
>>>>>>
>>>>>> Martin Grigorov
>>>>>> Wicket Training and Consulting
>>>>>>
>>>>>>
>>>>>> --
>>>>> Regards - Ernesto Reinaldo Barreiro
>>>>>
>>>>>
>>>>>
>
Re: Marker interface for "internal" exceptions
Posted by Sven Meier <sv...@meiers.net>.
What would be the name of that interface and which classes would
implement it?
Sven
On 06/02/2014 11:27 PM, Martin Grigorov wrote:
> On Mon, Jun 2, 2014 at 11:14 PM, Sven Meier <sv...@meiers.net> wrote:
>
>> Hi,
>>
>> the problem I see with an exception interface or #canHandleException() is
>> that both promise something (i.e. this exception will be handled) but you
>> cannot be sure that this promise will be kept.
>> What if you have installed an alternative IExceptionMapper which maps
>> things differently? Or #canHandleException() get's out of sync with the
>> actual code in #map(Exception)?
>>
>> How about adding a hook method #onMapped(Exception,IRequestHandler) to
>> DefaultExceptionMapper. Subclasses could check for
>> RenderPageRequeatHandlers *after* Wicket's default mapping was performed. A
>> subclass could return an override with the previous page and an error
>> message instead.
>>
> I think the application code should not deal with "special" exceptions like
> ResponseIOException (because this means the connection to the browser is
> closed) or StalePageException (because this means the previous page is
> stale/obsolete for some reason and needs to be re-rendered to be useful
> again).
> The idea of the special marker interface is to tell the application "you
> better leave this problem to be handled by the framework".
> By using this marker interface the application code will be future-proof.
>
>
>> My 2 cents
>> Sven
>>
>>
>>
>> On 06/02/2014 10:50 PM, Martin Grigorov wrote:
>>
>>> Hi Ernesto,
>>>
>>>
>>> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
>>> reiern70@gmail.com> wrote:
>>>
>>> Martin,
>>>> Just an idea,
>>>>
>>>> Besides your marker interface maybe IExceptionMapper can be re-factored
>>>> as
>>>>
>>>> 1-
>>>> public interface IExceptionMapper
>>>> {
>>>> /** return true if it knows how to handle and exception */
>>>> boolean canHandle(Exception e);
>>>>
>>>> /**
>>>> * @param e
>>>> *
>>>> * @return {@link IRequestHandler} for given exception
>>>> */
>>>> IRequestHandler map(Exception e);
>>>> }
>>>>
>>>> 2- Allow a list a of IExceptionMapper's. So, that user can register
>>>> mappers
>>>> before the default one.
>>>> 3- This list will be iterated and ask canHandle(e) on each mapper to see
>>>> if
>>>> it can handle the exception. The default mapper could handle "all"
>>>> exceptions not handle by previous IExceptionMappers.
>>>>
>>>> Maybe overkill?
>>>>
>>>> What is the difference with the current way ?
>>> Now we have a list of IRequestCycleListeners and Wicket asks them one by
>>> one until some returns non-null. If all return null then the last resort
>>> is
>>> used - the IExceptionMapper.
>>>
>>>
>>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
>>>> wrote:
>>>>
>>>> Hi,
>>>>> Recently I had the chance to review some customer applications and I've
>>>>> noticed a pattern in both applications:
>>>>>
>>>>> a custom impl of IRequestCycleListener#onException() checks whether the
>>>>> given exception is instance of something that the application knows how
>>>>>
>>>> to
>>>>
>>>>> deal with and if it is not then a RenderPageRequestHandler is returned
>>>>> trying to render the last successfully rendered page. I.e. the idea is
>>>>> to
>>>>> show something like an error feedback in the last/current page instead
>>>>> of
>>>>> redirecting to InternalErrorPage.
>>>>>
>>>>> The problem in both impls was that internal/special exceptions like
>>>>> ResponseIOException, PackageResource.PackageResourceBlockedException
>>>>> and
>>>>> StalePageException (and maybe ListenerInvocationNotAllowedException,
>>>>> AuthorizationException) have been managed by the custom code instead of
>>>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>>>
>>>>> The fix was to add a check for these exceptions and do nothing. But
>>>>>
>>>> Wicket
>>>>
>>>>> can add more such special exceptions in the future and the application
>>>>> logic may break...
>>>>>
>>>>> What do you think about introducing a special marker interface for all
>>>>> exceptions which represent some erroneous state and be better handled by
>>>>> Wicket rather than by custom code ?
>>>>> This way a new exception type will handled automatically in the future.
>>>>>
>>>>> Or maybe you have a better idea ?
>>>>>
>>>>>
>>>>> Martin Grigorov
>>>>> Wicket Training and Consulting
>>>>>
>>>>>
>>>> --
>>>> Regards - Ernesto Reinaldo Barreiro
>>>>
>>>>
Re: Marker interface for "internal" exceptions
Posted by Martin Grigorov <mg...@apache.org>.
On Mon, Jun 2, 2014 at 11:14 PM, Sven Meier <sv...@meiers.net> wrote:
> Hi,
>
> the problem I see with an exception interface or #canHandleException() is
> that both promise something (i.e. this exception will be handled) but you
> cannot be sure that this promise will be kept.
> What if you have installed an alternative IExceptionMapper which maps
> things differently? Or #canHandleException() get's out of sync with the
> actual code in #map(Exception)?
>
> How about adding a hook method #onMapped(Exception,IRequestHandler) to
> DefaultExceptionMapper. Subclasses could check for
> RenderPageRequeatHandlers *after* Wicket's default mapping was performed. A
> subclass could return an override with the previous page and an error
> message instead.
>
I think the application code should not deal with "special" exceptions like
ResponseIOException (because this means the connection to the browser is
closed) or StalePageException (because this means the previous page is
stale/obsolete for some reason and needs to be re-rendered to be useful
again).
The idea of the special marker interface is to tell the application "you
better leave this problem to be handled by the framework".
By using this marker interface the application code will be future-proof.
>
> My 2 cents
> Sven
>
>
>
> On 06/02/2014 10:50 PM, Martin Grigorov wrote:
>
>> Hi Ernesto,
>>
>>
>> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
>> reiern70@gmail.com> wrote:
>>
>> Martin,
>>>
>>> Just an idea,
>>>
>>> Besides your marker interface maybe IExceptionMapper can be re-factored
>>> as
>>>
>>> 1-
>>> public interface IExceptionMapper
>>> {
>>> /** return true if it knows how to handle and exception */
>>> boolean canHandle(Exception e);
>>>
>>> /**
>>> * @param e
>>> *
>>> * @return {@link IRequestHandler} for given exception
>>> */
>>> IRequestHandler map(Exception e);
>>> }
>>>
>>> 2- Allow a list a of IExceptionMapper's. So, that user can register
>>> mappers
>>> before the default one.
>>> 3- This list will be iterated and ask canHandle(e) on each mapper to see
>>> if
>>> it can handle the exception. The default mapper could handle "all"
>>> exceptions not handle by previous IExceptionMappers.
>>>
>>> Maybe overkill?
>>>
>>> What is the difference with the current way ?
>>
>> Now we have a list of IRequestCycleListeners and Wicket asks them one by
>> one until some returns non-null. If all return null then the last resort
>> is
>> used - the IExceptionMapper.
>>
>>
>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
>>> wrote:
>>>
>>> Hi,
>>>>
>>>> Recently I had the chance to review some customer applications and I've
>>>> noticed a pattern in both applications:
>>>>
>>>> a custom impl of IRequestCycleListener#onException() checks whether the
>>>> given exception is instance of something that the application knows how
>>>>
>>> to
>>>
>>>> deal with and if it is not then a RenderPageRequestHandler is returned
>>>> trying to render the last successfully rendered page. I.e. the idea is
>>>> to
>>>> show something like an error feedback in the last/current page instead
>>>> of
>>>> redirecting to InternalErrorPage.
>>>>
>>>> The problem in both impls was that internal/special exceptions like
>>>> ResponseIOException, PackageResource.PackageResourceBlockedException
>>>> and
>>>> StalePageException (and maybe ListenerInvocationNotAllowedException,
>>>> AuthorizationException) have been managed by the custom code instead of
>>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>>
>>>> The fix was to add a check for these exceptions and do nothing. But
>>>>
>>> Wicket
>>>
>>>> can add more such special exceptions in the future and the application
>>>> logic may break...
>>>>
>>>> What do you think about introducing a special marker interface for all
>>>> exceptions which represent some erroneous state and be better handled by
>>>> Wicket rather than by custom code ?
>>>> This way a new exception type will handled automatically in the future.
>>>>
>>>> Or maybe you have a better idea ?
>>>>
>>>>
>>>> Martin Grigorov
>>>> Wicket Training and Consulting
>>>>
>>>>
>>>
>>> --
>>> Regards - Ernesto Reinaldo Barreiro
>>>
>>>
>
Re: Marker interface for "internal" exceptions
Posted by Sven Meier <sv...@meiers.net>.
Hi,
the problem I see with an exception interface or #canHandleException() is that both promise something (i.e. this exception will be handled) but you cannot be sure that this promise will be kept.
What if you have installed an alternative IExceptionMapper which maps things differently? Or #canHandleException() get's out of sync with the actual code in #map(Exception)?
How about adding a hook method #onMapped(Exception,IRequestHandler) to DefaultExceptionMapper. Subclasses could check for RenderPageRequeatHandlers *after* Wicket's default mapping was performed. A subclass could return an override with the previous page and an error message instead.
My 2 cents
Sven
On 06/02/2014 10:50 PM, Martin Grigorov wrote:
> Hi Ernesto,
>
>
> On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
> reiern70@gmail.com> wrote:
>
>> Martin,
>>
>> Just an idea,
>>
>> Besides your marker interface maybe IExceptionMapper can be re-factored as
>>
>> 1-
>> public interface IExceptionMapper
>> {
>> /** return true if it knows how to handle and exception */
>> boolean canHandle(Exception e);
>>
>> /**
>> * @param e
>> *
>> * @return {@link IRequestHandler} for given exception
>> */
>> IRequestHandler map(Exception e);
>> }
>>
>> 2- Allow a list a of IExceptionMapper's. So, that user can register mappers
>> before the default one.
>> 3- This list will be iterated and ask canHandle(e) on each mapper to see if
>> it can handle the exception. The default mapper could handle "all"
>> exceptions not handle by previous IExceptionMappers.
>>
>> Maybe overkill?
>>
> What is the difference with the current way ?
>
> Now we have a list of IRequestCycleListeners and Wicket asks them one by
> one until some returns non-null. If all return null then the last resort is
> used - the IExceptionMapper.
>
>
>> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
>> wrote:
>>
>>> Hi,
>>>
>>> Recently I had the chance to review some customer applications and I've
>>> noticed a pattern in both applications:
>>>
>>> a custom impl of IRequestCycleListener#onException() checks whether the
>>> given exception is instance of something that the application knows how
>> to
>>> deal with and if it is not then a RenderPageRequestHandler is returned
>>> trying to render the last successfully rendered page. I.e. the idea is to
>>> show something like an error feedback in the last/current page instead of
>>> redirecting to InternalErrorPage.
>>>
>>> The problem in both impls was that internal/special exceptions like
>>> ResponseIOException, PackageResource.PackageResourceBlockedException and
>>> StalePageException (and maybe ListenerInvocationNotAllowedException,
>>> AuthorizationException) have been managed by the custom code instead of
>>> letting Wicket to do its business (in DefaultExceptionMapper).
>>>
>>> The fix was to add a check for these exceptions and do nothing. But
>> Wicket
>>> can add more such special exceptions in the future and the application
>>> logic may break...
>>>
>>> What do you think about introducing a special marker interface for all
>>> exceptions which represent some erroneous state and be better handled by
>>> Wicket rather than by custom code ?
>>> This way a new exception type will handled automatically in the future.
>>>
>>> Or maybe you have a better idea ?
>>>
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>>
>>
>>
>> --
>> Regards - Ernesto Reinaldo Barreiro
>>
Re: Marker interface for "internal" exceptions
Posted by Martin Grigorov <mg...@apache.org>.
Hi Ernesto,
On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro <
reiern70@gmail.com> wrote:
> Martin,
>
> Just an idea,
>
> Besides your marker interface maybe IExceptionMapper can be re-factored as
>
> 1-
> public interface IExceptionMapper
> {
> /** return true if it knows how to handle and exception */
> boolean canHandle(Exception e);
>
> /**
> * @param e
> *
> * @return {@link IRequestHandler} for given exception
> */
> IRequestHandler map(Exception e);
> }
>
> 2- Allow a list a of IExceptionMapper's. So, that user can register mappers
> before the default one.
> 3- This list will be iterated and ask canHandle(e) on each mapper to see if
> it can handle the exception. The default mapper could handle "all"
> exceptions not handle by previous IExceptionMappers.
>
> Maybe overkill?
>
What is the difference with the current way ?
Now we have a list of IRequestCycleListeners and Wicket asks them one by
one until some returns non-null. If all return null then the last resort is
used - the IExceptionMapper.
>
> On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
> wrote:
>
> > Hi,
> >
> > Recently I had the chance to review some customer applications and I've
> > noticed a pattern in both applications:
> >
> > a custom impl of IRequestCycleListener#onException() checks whether the
> > given exception is instance of something that the application knows how
> to
> > deal with and if it is not then a RenderPageRequestHandler is returned
> > trying to render the last successfully rendered page. I.e. the idea is to
> > show something like an error feedback in the last/current page instead of
> > redirecting to InternalErrorPage.
> >
> > The problem in both impls was that internal/special exceptions like
> > ResponseIOException, PackageResource.PackageResourceBlockedException and
> > StalePageException (and maybe ListenerInvocationNotAllowedException,
> > AuthorizationException) have been managed by the custom code instead of
> > letting Wicket to do its business (in DefaultExceptionMapper).
> >
> > The fix was to add a check for these exceptions and do nothing. But
> Wicket
> > can add more such special exceptions in the future and the application
> > logic may break...
> >
> > What do you think about introducing a special marker interface for all
> > exceptions which represent some erroneous state and be better handled by
> > Wicket rather than by custom code ?
> > This way a new exception type will handled automatically in the future.
> >
> > Or maybe you have a better idea ?
> >
> >
> > Martin Grigorov
> > Wicket Training and Consulting
> >
>
>
>
> --
> Regards - Ernesto Reinaldo Barreiro
>
Re: Marker interface for "internal" exceptions
Posted by Ernesto Reinaldo Barreiro <re...@gmail.com>.
Martin,
Just an idea,
Besides your marker interface maybe IExceptionMapper can be re-factored as
1-
public interface IExceptionMapper
{
/** return true if it knows how to handle and exception */
boolean canHandle(Exception e);
/**
* @param e
*
* @return {@link IRequestHandler} for given exception
*/
IRequestHandler map(Exception e);
}
2- Allow a list a of IExceptionMapper's. So, that user can register mappers
before the default one.
3- This list will be iterated and ask canHandle(e) on each mapper to see if
it can handle the exception. The default mapper could handle "all"
exceptions not handle by previous IExceptionMappers.
Maybe overkill?
On Mon, Jun 2, 2014 at 9:39 PM, Martin Grigorov <mg...@apache.org>
wrote:
> Hi,
>
> Recently I had the chance to review some customer applications and I've
> noticed a pattern in both applications:
>
> a custom impl of IRequestCycleListener#onException() checks whether the
> given exception is instance of something that the application knows how to
> deal with and if it is not then a RenderPageRequestHandler is returned
> trying to render the last successfully rendered page. I.e. the idea is to
> show something like an error feedback in the last/current page instead of
> redirecting to InternalErrorPage.
>
> The problem in both impls was that internal/special exceptions like
> ResponseIOException, PackageResource.PackageResourceBlockedException and
> StalePageException (and maybe ListenerInvocationNotAllowedException,
> AuthorizationException) have been managed by the custom code instead of
> letting Wicket to do its business (in DefaultExceptionMapper).
>
> The fix was to add a check for these exceptions and do nothing. But Wicket
> can add more such special exceptions in the future and the application
> logic may break...
>
> What do you think about introducing a special marker interface for all
> exceptions which represent some erroneous state and be better handled by
> Wicket rather than by custom code ?
> This way a new exception type will handled automatically in the future.
>
> Or maybe you have a better idea ?
>
>
> Martin Grigorov
> Wicket Training and Consulting
>
--
Regards - Ernesto Reinaldo Barreiro