Yes you are absolutely right.
On Mon, Jun 2, 2014 at 10:50 PM, Martin Grigorov <[email protected]> wrote: > Hi Ernesto, > > > On Mon, Jun 2, 2014 at 10:42 PM, Ernesto Reinaldo Barreiro < > [email protected]> 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 <[email protected]> > > 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
