You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Bart Molenkamp <b....@bizzdesign.nl> on 2007/05/08 12:18:39 UTC

An easier way to create a custom replacement for ExceptionErrorPage?

Hi,

I think it's currently too hard to show a custom exception page. To do 
so, I need to override Application.getRequestCycleFactory(), return my 
own IRequestCycleFactory implementation, that can build my subclass of 
WebRequestCycle that overrides the onRuntimeException() method.

Maybe it's easier to define a method on WebApplication:

Page onRuntimeException(Page page, RuntimeException e) {
   return null;
}

And call this method from WebRequestCycle.onRuntimeException. By default 
this would have the same behavior as it is implemented now, but it's 
much easier to create a custom error page because I only have to 
override onRuntimeException in my WebApplication subclass.

Bart.

RE: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Bart Molenkamp <b....@bizzdesign.nl>.
I created a patch, see:

https://issues.apache.org/jira/browse/WICKET-563

The error page creation is now delegated to Application. I think it is
logical to create these page in Application, because:
- Application also creates the home page. Now it can create error pages
that have the same style/page layout (using borders, panels, etc.)
- You always must override Application, so this makes it only a few
lines of extra code to override error pages, and you don't have to extend
other classes (no need to extend the IRequestCycleFactory and WebRequestCycle
anymore).

The exception handling is still done in AbstractRequestCycleProcessor.
Exceptions are caught there, and based on the type of exception it calls:
- onAuthorizationException()
- onPageExpiredException()
- onRuntimeException()

The onRuntimeException() determines if it displays the internal error
page or the exception error page.

Also, if the rendering of an error page throws an exception, this is handled
by AbstractRequestCycleProcessor. If the page returned by onAuthorizationException()
fails to render or the page returned by onPageExpiredException() fails to render,
then the onRuntimeException will be called, and if that fails, it will just throw
a WicketRuntimeException that will be caught by the servlet container. This behavior
was already implemented, but didn't work when the RequestCycle's onRuntimeException
returns a custom page (it returned null by default). If the custom error page
failed to render, the application goes into and endless loop.

My patch also makes the error page settings unused. So the
setPageExpiredErrorPage(Class ...) etc. methods can be removed.

I hope my explanation is clear.

Bart.


> -----Oorspronkelijk bericht-----
> Van: Johan Compagner [mailto:jcompagner@gmail.com]
> Verzonden: woensdag 9 mei 2007 14:59
> Aan: wicket-dev@incubator.apache.org
> Onderwerp: Re: An easier way to create a custom replacement
> for ExceptionErrorPage?
>
> yes please!
>
> On 5/9/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> >
> > I can see if I can create a patch for the exception handling
> > part.
> >
> > Should I upload it to JIRA?
> >
> > > -----Oorspronkelijk bericht-----
> > > Van: Johan Compagner [mailto:jcompagner@gmail.com]
> > >
> > > Who is going to make a patch so that we can look at some code
> > > instead of
> > > discussing it :)
> > >
> > > johan
> > >
> > >
> > > On 5/9/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > > >
> > > > > I think that it's fine to have the method on the
> > > Application class.
> > > > > You need to extend it anyway, so overriding
> > > onRuntimeException is very
> > > > > easy to do. And I do think it does belong there, because
> > > it knows how
> > > > > to create the homepage, I think it's logical that it also
> > > knows how to
> > > > > create an error page.
> > > >
> > > > For the system I'm working on, I assign a unique id to
> every request
> > > > cycle, which serves as an error ticked id once an exception
> > > occurs. It
> > > > feelts natural to assign that in the request cycle, and
> I believe it
> > > > feels just as natural to put the related error handling in
> > > the request
> > > > cycle. Application is meant for settings and factories,
> and imho, a
> > > > request related error call back would just be wrongly
> > > scoped when put
> > > > in the application class.
> > > >
> > > > > Do you mean you want to remove all those factory
> > > interfaces for simple
> > > > > factory methods on Application?
> > > >
> > > > That would be my ideal, yes.
> > > >
> > > > Eelco
> > > >
> > >
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
yes please!

On 5/9/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>
> I can see if I can create a patch for the exception handling
> part.
>
> Should I upload it to JIRA?
>
> > -----Oorspronkelijk bericht-----
> > Van: Johan Compagner [mailto:jcompagner@gmail.com]
> >
> > Who is going to make a patch so that we can look at some code
> > instead of
> > discussing it :)
> >
> > johan
> >
> >
> > On 5/9/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > >
> > > > I think that it's fine to have the method on the
> > Application class.
> > > > You need to extend it anyway, so overriding
> > onRuntimeException is very
> > > > easy to do. And I do think it does belong there, because
> > it knows how
> > > > to create the homepage, I think it's logical that it also
> > knows how to
> > > > create an error page.
> > >
> > > For the system I'm working on, I assign a unique id to every request
> > > cycle, which serves as an error ticked id once an exception
> > occurs. It
> > > feelts natural to assign that in the request cycle, and I believe it
> > > feels just as natural to put the related error handling in
> > the request
> > > cycle. Application is meant for settings and factories, and imho, a
> > > request related error call back would just be wrongly
> > scoped when put
> > > in the application class.
> > >
> > > > Do you mean you want to remove all those factory
> > interfaces for simple
> > > > factory methods on Application?
> > >
> > > That would be my ideal, yes.
> > >
> > > Eelco
> > >
> >
>

RE: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Bart Molenkamp <b....@bizzdesign.nl>.
I can see if I can create a patch for the exception handling
part.

Should I upload it to JIRA?

> -----Oorspronkelijk bericht-----
> Van: Johan Compagner [mailto:jcompagner@gmail.com]
>
> Who is going to make a patch so that we can look at some code
> instead of
> discussing it :)
>
> johan
>
>
> On 5/9/07, Eelco Hillenius <ee...@gmail.com> wrote:
> >
> > > I think that it's fine to have the method on the
> Application class.
> > > You need to extend it anyway, so overriding
> onRuntimeException is very
> > > easy to do. And I do think it does belong there, because
> it knows how
> > > to create the homepage, I think it's logical that it also
> knows how to
> > > create an error page.
> >
> > For the system I'm working on, I assign a unique id to every request
> > cycle, which serves as an error ticked id once an exception
> occurs. It
> > feelts natural to assign that in the request cycle, and I believe it
> > feels just as natural to put the related error handling in
> the request
> > cycle. Application is meant for settings and factories, and imho, a
> > request related error call back would just be wrongly
> scoped when put
> > in the application class.
> >
> > > Do you mean you want to remove all those factory
> interfaces for simple
> > > factory methods on Application?
> >
> > That would be my ideal, yes.
> >
> > Eelco
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
Who is going to make a patch so that we can look at some code instead of
discussing it :)

johan


On 5/9/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> > I think that it's fine to have the method on the Application class.
> > You need to extend it anyway, so overriding onRuntimeException is very
> > easy to do. And I do think it does belong there, because it knows how
> > to create the homepage, I think it's logical that it also knows how to
> > create an error page.
>
> For the system I'm working on, I assign a unique id to every request
> cycle, which serves as an error ticked id once an exception occurs. It
> feelts natural to assign that in the request cycle, and I believe it
> feels just as natural to put the related error handling in the request
> cycle. Application is meant for settings and factories, and imho, a
> request related error call back would just be wrongly scoped when put
> in the application class.
>
> > Do you mean you want to remove all those factory interfaces for simple
> > factory methods on Application?
>
> That would be my ideal, yes.
>
> Eelco
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Eelco Hillenius <ee...@gmail.com>.
> I think that it's fine to have the method on the Application class.
> You need to extend it anyway, so overriding onRuntimeException is very
> easy to do. And I do think it does belong there, because it knows how
> to create the homepage, I think it's logical that it also knows how to
> create an error page.

For the system I'm working on, I assign a unique id to every request
cycle, which serves as an error ticked id once an exception occurs. It
feelts natural to assign that in the request cycle, and I believe it
feels just as natural to put the related error handling in the request
cycle. Application is meant for settings and factories, and imho, a
request related error call back would just be wrongly scoped when put
in the application class.

> Do you mean you want to remove all those factory interfaces for simple
> factory methods on Application?

That would be my ideal, yes.

Eelco

RE: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Bart Molenkamp <b....@bizzdesign.nl>.
> -----Oorspronkelijk bericht-----
> Van: Eelco Hillenius [mailto:eelco.hillenius@gmail.com]

> I'm not sure whether the factory
> method would be better in application, as I believe handling a runtime
> exception is something that logically belongs in a request cycle, not
> application. The only disadvantage is that it's maybe harder to find
> for people.

I think that it's fine to have the method on the Application class.
You need to extend it anyway, so overriding onRuntimeException is very
easy to do. And I do think it does belong there, because it knows how
to create the homepage, I think it's logical that it also knows how to
create an error page.

>
> What this does touch is that we have a request cycle factory to start
> with, rather than just a simple factory method like we have with e.g.
> newRequestCycleProcessor and newSessionStore. Imo, it should be enough
> to have just that, and those separate factories are just bloat. Want
> to start a new thread about this?
>

Do you mean you want to remove all those factory interfaces for simple
factory methods on Application?

Bart.


Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Eelco Hillenius <ee...@gmail.com>.
> I need access to the exception that caused the internal error, and if
> that can be done trough InternalErrorPage, then that's fine by me.
>
> Instead of using this constructor reflection magic, maybe a simple
> factory interface can be defined that can create pages. So instead of
> having a setInternalErrorPage(...) something like
> createInternalErrorPage(Throwable t, Page p).

Yes, I agree 100%. It's also much easier to debug then. Imho, we
should see if we can get rid of those class parameters all together in
favor of factory methods.

Please note that the current pattern (override onException in a custom
request cycle) works fine, so I'm really only arguing against having
the class setting in the first place. I'm not sure whether the factory
method would be better in application, as I believe handling a runtime
exception is something that logically belongs in a request cycle, not
application. The only disadvantage is that it's maybe harder to find
for people.

What this does touch is that we have a request cycle factory to start
with, rather than just a simple factory method like we have with e.g.
newRequestCycleProcessor and newSessionStore. Imo, it should be enough
to have just that, and those separate factories are just bloat. Want
to start a new thread about this?

Eelco

> I think it's easier to understand such an interface instead of finding
> out that you can (but don't have to) add a constructor with one argument
> that takes the throwable as argument.
>
> Bart.
>
> Johan Compagner wrote:
> > ahh but this is not about our own ExceptionPage but more about
> > the InternalPage should be a bit better.
> >
> > Why not look if that page has a Exception constructor param give it to it?
> >
> > The problem i can see maybe is that we do a redirect to it.. (but i am not
> > sure)
> >
> > johan
> >
> >
> > On 5/8/07, Eelco Hillenius <ee...@gmail.com> wrote:
> >>
> >> I don't agree with that though. We (at Teachscape) are using the
> >> override as using the setting is just too simplistic. Imho an example
> >> of where convenience won from a better approach.
> >>
> >> This is what we do:
> >>
> >>         public Page onRuntimeException(Page page, RuntimeException e) {
> >>
> >>                 if (e instanceof AuthorizationException) {
> >>                         return super.onRuntimeException(page, e);
> >>                 } else if (e instanceof PageExpiredException) {
> >>                         Ts4Session session = Ts4Session.get();
> >>                         if (session.isTemporary() || session.getUserId()
> >> == null) {
> >>                                 // this was an actual session expiry
> >>                                 log
> >>                                                 .info("session expired
> >> and
> >> request cannot be honored, cycleId: "
> >>                                                                 +
> >> cycleId);
> >>                                 return super.onRuntimeException(page, e);
> >>                         } else {
> >>                                 log.error("unable to find page for an
> >> active session!");
> >>                                 // hmmm, we have a logged in user, but
> >> the
> >> page could not be
> >>                                 // found. Fall through to display an
> >> error
> >> page or exception
> >>                                 // page
> >>                         }
> >>                 }
> >>
> >>                 if (Application.DEPLOYMENT.equals(Application.get()
> >>                                 .getConfigurationType())) {
> >>                         return new ErrorPage(e, page, cycleId);
> >>                 } else {
> >>                         return new ExceptionErrorPage(e, page);
> >>                 }
> >>         }
> >>
> >> And while ErrorPage doesn't display the exception to end-users, it
> >> still wants to use it for logging etc. I don't know why we ever
> >> thought we wouldn't need the exception in a user-facing error page.
> >> FYI, the error page also has a (stateless) form for getting feedback
> >> from the user. Error log statements are all mailed, and the initial
> >> exception log and the optional user report are linked together by a
> >> unique request cycle identifier.
> >>
> >> Anyway, if you ask me, we should give our error handling a close look
> >> some time, and simplify without oversymplifying like we've done
> >> before.
> >>
> >> Eelco
> >>
> >> On 5/8/07, Johan Compagner <jc...@gmail.com> wrote:
> >> > I think we had that method on application but that was moved again.
> >> >
> >> > But why do you want to change the ExceptionErrorPage?
> >> > That is just there for debugging, Normally in production you would see
> >> the
> >> > InternalErrorPage
> >> > that can be set through the settings.
> >> >
> >> > johan
> >> >
> >> >
> >> > On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> >> > >
> >> > > Hi,
> >> > >
> >> > > I think it's currently too hard to show a custom exception page.
> >> To do
> >> > > so, I need to override Application.getRequestCycleFactory(),
> >> return my
> >> > > own IRequestCycleFactory implementation, that can build my
> >> subclass of
> >> > > WebRequestCycle that overrides the onRuntimeException() method.
> >> > >
> >> > > Maybe it's easier to define a method on WebApplication:
> >> > >
> >> > > Page onRuntimeException(Page page, RuntimeException e) {
> >> > >    return null;
> >> > > }
> >> > >
> >> > > And call this method from WebRequestCycle.onRuntimeException. By
> >> default
> >> > > this would have the same behavior as it is implemented now, but it's
> >> > > much easier to create a custom error page because I only have to
> >> > > override onRuntimeException in my WebApplication subclass.
> >> > >
> >> > > Bart.
> >> > >
> >> >
> >>
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
if we find a nice api for this fine by me.

johan


On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>
> I need access to the exception that caused the internal error, and if
> that can be done trough InternalErrorPage, then that's fine by me.
>
> Instead of using this constructor reflection magic, maybe a simple
> factory interface can be defined that can create pages. So instead of
> having a setInternalErrorPage(...) something like
> createInternalErrorPage(Throwable t, Page p).
>
> I think it's easier to understand such an interface instead of finding
> out that you can (but don't have to) add a constructor with one argument
> that takes the throwable as argument.
>
> Bart.
>
> Johan Compagner wrote:
> > ahh but this is not about our own ExceptionPage but more about
> > the InternalPage should be a bit better.
> >
> > Why not look if that page has a Exception constructor param give it to
> it?
> >
> > The problem i can see maybe is that we do a redirect to it.. (but i am
> not
> > sure)
> >
> > johan
> >
> >
> > On 5/8/07, Eelco Hillenius <ee...@gmail.com> wrote:
> >>
> >> I don't agree with that though. We (at Teachscape) are using the
> >> override as using the setting is just too simplistic. Imho an example
> >> of where convenience won from a better approach.
> >>
> >> This is what we do:
> >>
> >>         public Page onRuntimeException(Page page, RuntimeException e) {
> >>
> >>                 if (e instanceof AuthorizationException) {
> >>                         return super.onRuntimeException(page, e);
> >>                 } else if (e instanceof PageExpiredException) {
> >>                         Ts4Session session = Ts4Session.get();
> >>                         if (session.isTemporary() || session.getUserId
> ()
> >> == null) {
> >>                                 // this was an actual session expiry
> >>                                 log
> >>                                                 .info("session expired
> >> and
> >> request cannot be honored, cycleId: "
> >>                                                                 +
> >> cycleId);
> >>                                 return super.onRuntimeException(page,
> e);
> >>                         } else {
> >>                                 log.error("unable to find page for an
> >> active session!");
> >>                                 // hmmm, we have a logged in user, but
> >> the
> >> page could not be
> >>                                 // found. Fall through to display an
> >> error
> >> page or exception
> >>                                 // page
> >>                         }
> >>                 }
> >>
> >>                 if (Application.DEPLOYMENT.equals(Application.get()
> >>                                 .getConfigurationType())) {
> >>                         return new ErrorPage(e, page, cycleId);
> >>                 } else {
> >>                         return new ExceptionErrorPage(e, page);
> >>                 }
> >>         }
> >>
> >> And while ErrorPage doesn't display the exception to end-users, it
> >> still wants to use it for logging etc. I don't know why we ever
> >> thought we wouldn't need the exception in a user-facing error page.
> >> FYI, the error page also has a (stateless) form for getting feedback
> >> from the user. Error log statements are all mailed, and the initial
> >> exception log and the optional user report are linked together by a
> >> unique request cycle identifier.
> >>
> >> Anyway, if you ask me, we should give our error handling a close look
> >> some time, and simplify without oversymplifying like we've done
> >> before.
> >>
> >> Eelco
> >>
> >> On 5/8/07, Johan Compagner <jc...@gmail.com> wrote:
> >> > I think we had that method on application but that was moved again.
> >> >
> >> > But why do you want to change the ExceptionErrorPage?
> >> > That is just there for debugging, Normally in production you would
> see
> >> the
> >> > InternalErrorPage
> >> > that can be set through the settings.
> >> >
> >> > johan
> >> >
> >> >
> >> > On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> >> > >
> >> > > Hi,
> >> > >
> >> > > I think it's currently too hard to show a custom exception page.
> >> To do
> >> > > so, I need to override Application.getRequestCycleFactory(),
> >> return my
> >> > > own IRequestCycleFactory implementation, that can build my
> >> subclass of
> >> > > WebRequestCycle that overrides the onRuntimeException() method.
> >> > >
> >> > > Maybe it's easier to define a method on WebApplication:
> >> > >
> >> > > Page onRuntimeException(Page page, RuntimeException e) {
> >> > >    return null;
> >> > > }
> >> > >
> >> > > And call this method from WebRequestCycle.onRuntimeException. By
> >> default
> >> > > this would have the same behavior as it is implemented now, but
> it's
> >> > > much easier to create a custom error page because I only have to
> >> > > override onRuntimeException in my WebApplication subclass.
> >> > >
> >> > > Bart.
> >> > >
> >> >
> >>
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Bart Molenkamp <b....@bizzdesign.nl>.
I need access to the exception that caused the internal error, and if 
that can be done trough InternalErrorPage, then that's fine by me.

Instead of using this constructor reflection magic, maybe a simple 
factory interface can be defined that can create pages. So instead of 
having a setInternalErrorPage(...) something like 
createInternalErrorPage(Throwable t, Page p).

I think it's easier to understand such an interface instead of finding 
out that you can (but don't have to) add a constructor with one argument 
that takes the throwable as argument.

Bart.

Johan Compagner wrote:
> ahh but this is not about our own ExceptionPage but more about
> the InternalPage should be a bit better.
> 
> Why not look if that page has a Exception constructor param give it to it?
> 
> The problem i can see maybe is that we do a redirect to it.. (but i am not
> sure)
> 
> johan
> 
> 
> On 5/8/07, Eelco Hillenius <ee...@gmail.com> wrote:
>>
>> I don't agree with that though. We (at Teachscape) are using the
>> override as using the setting is just too simplistic. Imho an example
>> of where convenience won from a better approach.
>>
>> This is what we do:
>>
>>         public Page onRuntimeException(Page page, RuntimeException e) {
>>
>>                 if (e instanceof AuthorizationException) {
>>                         return super.onRuntimeException(page, e);
>>                 } else if (e instanceof PageExpiredException) {
>>                         Ts4Session session = Ts4Session.get();
>>                         if (session.isTemporary() || session.getUserId()
>> == null) {
>>                                 // this was an actual session expiry
>>                                 log
>>                                                 .info("session expired 
>> and
>> request cannot be honored, cycleId: "
>>                                                                 +
>> cycleId);
>>                                 return super.onRuntimeException(page, e);
>>                         } else {
>>                                 log.error("unable to find page for an
>> active session!");
>>                                 // hmmm, we have a logged in user, but 
>> the
>> page could not be
>>                                 // found. Fall through to display an 
>> error
>> page or exception
>>                                 // page
>>                         }
>>                 }
>>
>>                 if (Application.DEPLOYMENT.equals(Application.get()
>>                                 .getConfigurationType())) {
>>                         return new ErrorPage(e, page, cycleId);
>>                 } else {
>>                         return new ExceptionErrorPage(e, page);
>>                 }
>>         }
>>
>> And while ErrorPage doesn't display the exception to end-users, it
>> still wants to use it for logging etc. I don't know why we ever
>> thought we wouldn't need the exception in a user-facing error page.
>> FYI, the error page also has a (stateless) form for getting feedback
>> from the user. Error log statements are all mailed, and the initial
>> exception log and the optional user report are linked together by a
>> unique request cycle identifier.
>>
>> Anyway, if you ask me, we should give our error handling a close look
>> some time, and simplify without oversymplifying like we've done
>> before.
>>
>> Eelco
>>
>> On 5/8/07, Johan Compagner <jc...@gmail.com> wrote:
>> > I think we had that method on application but that was moved again.
>> >
>> > But why do you want to change the ExceptionErrorPage?
>> > That is just there for debugging, Normally in production you would see
>> the
>> > InternalErrorPage
>> > that can be set through the settings.
>> >
>> > johan
>> >
>> >
>> > On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>> > >
>> > > Hi,
>> > >
>> > > I think it's currently too hard to show a custom exception page. 
>> To do
>> > > so, I need to override Application.getRequestCycleFactory(), 
>> return my
>> > > own IRequestCycleFactory implementation, that can build my 
>> subclass of
>> > > WebRequestCycle that overrides the onRuntimeException() method.
>> > >
>> > > Maybe it's easier to define a method on WebApplication:
>> > >
>> > > Page onRuntimeException(Page page, RuntimeException e) {
>> > >    return null;
>> > > }
>> > >
>> > > And call this method from WebRequestCycle.onRuntimeException. By
>> default
>> > > this would have the same behavior as it is implemented now, but it's
>> > > much easier to create a custom error page because I only have to
>> > > override onRuntimeException in my WebApplication subclass.
>> > >
>> > > Bart.
>> > >
>> >
>>
> 

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
ahh but this is not about our own ExceptionPage but more about
the InternalPage should be a bit better.

Why not look if that page has a Exception constructor param give it to it?

The problem i can see maybe is that we do a redirect to it.. (but i am not
sure)

johan


On 5/8/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> I don't agree with that though. We (at Teachscape) are using the
> override as using the setting is just too simplistic. Imho an example
> of where convenience won from a better approach.
>
> This is what we do:
>
>         public Page onRuntimeException(Page page, RuntimeException e) {
>
>                 if (e instanceof AuthorizationException) {
>                         return super.onRuntimeException(page, e);
>                 } else if (e instanceof PageExpiredException) {
>                         Ts4Session session = Ts4Session.get();
>                         if (session.isTemporary() || session.getUserId()
> == null) {
>                                 // this was an actual session expiry
>                                 log
>                                                 .info("session expired and
> request cannot be honored, cycleId: "
>                                                                 +
> cycleId);
>                                 return super.onRuntimeException(page, e);
>                         } else {
>                                 log.error("unable to find page for an
> active session!");
>                                 // hmmm, we have a logged in user, but the
> page could not be
>                                 // found. Fall through to display an error
> page or exception
>                                 // page
>                         }
>                 }
>
>                 if (Application.DEPLOYMENT.equals(Application.get()
>                                 .getConfigurationType())) {
>                         return new ErrorPage(e, page, cycleId);
>                 } else {
>                         return new ExceptionErrorPage(e, page);
>                 }
>         }
>
> And while ErrorPage doesn't display the exception to end-users, it
> still wants to use it for logging etc. I don't know why we ever
> thought we wouldn't need the exception in a user-facing error page.
> FYI, the error page also has a (stateless) form for getting feedback
> from the user. Error log statements are all mailed, and the initial
> exception log and the optional user report are linked together by a
> unique request cycle identifier.
>
> Anyway, if you ask me, we should give our error handling a close look
> some time, and simplify without oversymplifying like we've done
> before.
>
> Eelco
>
> On 5/8/07, Johan Compagner <jc...@gmail.com> wrote:
> > I think we had that method on application but that was moved again.
> >
> > But why do you want to change the ExceptionErrorPage?
> > That is just there for debugging, Normally in production you would see
> the
> > InternalErrorPage
> > that can be set through the settings.
> >
> > johan
> >
> >
> > On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> > >
> > > Hi,
> > >
> > > I think it's currently too hard to show a custom exception page. To do
> > > so, I need to override Application.getRequestCycleFactory(), return my
> > > own IRequestCycleFactory implementation, that can build my subclass of
> > > WebRequestCycle that overrides the onRuntimeException() method.
> > >
> > > Maybe it's easier to define a method on WebApplication:
> > >
> > > Page onRuntimeException(Page page, RuntimeException e) {
> > >    return null;
> > > }
> > >
> > > And call this method from WebRequestCycle.onRuntimeException. By
> default
> > > this would have the same behavior as it is implemented now, but it's
> > > much easier to create a custom error page because I only have to
> > > override onRuntimeException in my WebApplication subclass.
> > >
> > > Bart.
> > >
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Eelco Hillenius <ee...@gmail.com>.
I don't agree with that though. We (at Teachscape) are using the
override as using the setting is just too simplistic. Imho an example
of where convenience won from a better approach.

This is what we do:

	public Page onRuntimeException(Page page, RuntimeException e) {

		if (e instanceof AuthorizationException) {
			return super.onRuntimeException(page, e);
		} else if (e instanceof PageExpiredException) {
			Ts4Session session = Ts4Session.get();
			if (session.isTemporary() || session.getUserId() == null) {
				// this was an actual session expiry
				log
						.info("session expired and request cannot be honored, cycleId: "
								+ cycleId);
				return super.onRuntimeException(page, e);
			} else {
				log.error("unable to find page for an active session!");
				// hmmm, we have a logged in user, but the page could not be
				// found. Fall through to display an error page or exception
				// page
			}
		}

		if (Application.DEPLOYMENT.equals(Application.get()
				.getConfigurationType())) {
			return new ErrorPage(e, page, cycleId);
		} else {
			return new ExceptionErrorPage(e, page);
		}
	}

And while ErrorPage doesn't display the exception to end-users, it
still wants to use it for logging etc. I don't know why we ever
thought we wouldn't need the exception in a user-facing error page.
FYI, the error page also has a (stateless) form for getting feedback
from the user. Error log statements are all mailed, and the initial
exception log and the optional user report are linked together by a
unique request cycle identifier.

Anyway, if you ask me, we should give our error handling a close look
some time, and simplify without oversymplifying like we've done
before.

Eelco

On 5/8/07, Johan Compagner <jc...@gmail.com> wrote:
> I think we had that method on application but that was moved again.
>
> But why do you want to change the ExceptionErrorPage?
> That is just there for debugging, Normally in production you would see the
> InternalErrorPage
> that can be set through the settings.
>
> johan
>
>
> On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> >
> > Hi,
> >
> > I think it's currently too hard to show a custom exception page. To do
> > so, I need to override Application.getRequestCycleFactory(), return my
> > own IRequestCycleFactory implementation, that can build my subclass of
> > WebRequestCycle that overrides the onRuntimeException() method.
> >
> > Maybe it's easier to define a method on WebApplication:
> >
> > Page onRuntimeException(Page page, RuntimeException e) {
> >    return null;
> > }
> >
> > And call this method from WebRequestCycle.onRuntimeException. By default
> > this would have the same behavior as it is implemented now, but it's
> > much easier to create a custom error page because I only have to
> > override onRuntimeException in my WebApplication subclass.
> >
> > Bart.
> >
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
in production you shouldn't give users exceptions
then they know exactly what you run and then it is much easier to try to
hack something

exception in in production environments should be get from the log.

So you could set your own InternalError page create an extra log entry with
more info (current users and so on)
then you have that besides each other in the log.

johan


On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>
> I didn't know it was just there for debugging. I think it is useful for
> production as well because it shows a stacktrace (and the
> InternalErrorPage can't show it).
>
> Is there any way to access the exception from an internal error page? If
> so then InternalErrorPage is good for me too.
>
> Bart.
>
> Johan Compagner wrote:
> > I think we had that method on application but that was moved again.
> >
> > But why do you want to change the ExceptionErrorPage?
> > That is just there for debugging, Normally in production you would see
> the
> > InternalErrorPage
> > that can be set through the settings.
> >
> > johan
> >
> >
> > On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
> >>
> >> Hi,
> >>
> >> I think it's currently too hard to show a custom exception page. To do
> >> so, I need to override Application.getRequestCycleFactory(), return my
> >> own IRequestCycleFactory implementation, that can build my subclass of
> >> WebRequestCycle that overrides the onRuntimeException() method.
> >>
> >> Maybe it's easier to define a method on WebApplication:
> >>
> >> Page onRuntimeException(Page page, RuntimeException e) {
> >>    return null;
> >> }
> >>
> >> And call this method from WebRequestCycle.onRuntimeException. By
> default
> >> this would have the same behavior as it is implemented now, but it's
> >> much easier to create a custom error page because I only have to
> >> override onRuntimeException in my WebApplication subclass.
> >>
> >> Bart.
> >>
> >
>
>

Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Bart Molenkamp <b....@bizzdesign.nl>.
I didn't know it was just there for debugging. I think it is useful for
production as well because it shows a stacktrace (and the
InternalErrorPage can't show it).

Is there any way to access the exception from an internal error page? If
so then InternalErrorPage is good for me too.

Bart.

Johan Compagner wrote:
> I think we had that method on application but that was moved again.
> 
> But why do you want to change the ExceptionErrorPage?
> That is just there for debugging, Normally in production you would see the
> InternalErrorPage
> that can be set through the settings.
> 
> johan
> 
> 
> On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>>
>> Hi,
>>
>> I think it's currently too hard to show a custom exception page. To do
>> so, I need to override Application.getRequestCycleFactory(), return my
>> own IRequestCycleFactory implementation, that can build my subclass of
>> WebRequestCycle that overrides the onRuntimeException() method.
>>
>> Maybe it's easier to define a method on WebApplication:
>>
>> Page onRuntimeException(Page page, RuntimeException e) {
>>    return null;
>> }
>>
>> And call this method from WebRequestCycle.onRuntimeException. By default
>> this would have the same behavior as it is implemented now, but it's
>> much easier to create a custom error page because I only have to
>> override onRuntimeException in my WebApplication subclass.
>>
>> Bart.
>>
> 


Re: An easier way to create a custom replacement for ExceptionErrorPage?

Posted by Johan Compagner <jc...@gmail.com>.
I think we had that method on application but that was moved again.

But why do you want to change the ExceptionErrorPage?
That is just there for debugging, Normally in production you would see the
InternalErrorPage
that can be set through the settings.

johan


On 5/8/07, Bart Molenkamp <b....@bizzdesign.nl> wrote:
>
> Hi,
>
> I think it's currently too hard to show a custom exception page. To do
> so, I need to override Application.getRequestCycleFactory(), return my
> own IRequestCycleFactory implementation, that can build my subclass of
> WebRequestCycle that overrides the onRuntimeException() method.
>
> Maybe it's easier to define a method on WebApplication:
>
> Page onRuntimeException(Page page, RuntimeException e) {
>    return null;
> }
>
> And call this method from WebRequestCycle.onRuntimeException. By default
> this would have the same behavior as it is implemented now, but it's
> much easier to create a custom error page because I only have to
> override onRuntimeException in my WebApplication subclass.
>
> Bart.
>