18 Sep
2014
18 Sep
'14
3:28 p.m.
Hi Devs,
Example of a use case:
* In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
I’ve tried implementing the following:
* Modified Document.getRenderedContent() to not throw exceptions any more
* I’ve added Document.setError() and getError()
* Then added a #displayRenderedContent($renderedContent) velocimacro to display an error,
calling Document.getError()
However that’s no good because we have lots of places in our code (and in extensions we
don’t control) that call getRenderedContent() and they expect the whole page rendering to
fail in case of an error…
Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
* Add a new #try() velocimacro that would push a new empty List<Exception> in the
Execution Context to a Stack variable
* Add a new uberspector that would, based on this flag do a try/catch around the method
called (can be done easily by returning a custom VelMethod)
* The uberspector catch would add the exception to the List<Exception> in the
Execution Context
* Add a new #catch() velocimacro that would pop from the Stack in the Execution Context
and that would set 2 variables in the Velocity Context:
** $exception: the first exception
** $exceptions: the list of all exceptions that happened between the #try() and #catch()
The good part with this is:
* No change in behavior to all existing code
* Code that want to do something in case of error can do it using #try()/#catch() for java
methods throwing exceptions (like getRenderedContent())
Of course for the future we would need to decide if script services should stop doing
try/catch or not but that’s another debate I’d prefer to separate from this thread.
WDYT?
Thanks
-Vincent
18 Sep
18 Sep
3:39 p.m.
+1 strangely :)
On Thu, Sep 18, 2014 at 3:28 PM, vincent(a)massol.net <vincent(a)massol.net> wrote:
Hi Devs,
Example of a use case:
* In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
I’ve tried implementing the following:
* Modified Document.getRenderedContent() to not throw exceptions any more
* I’ve added Document.setError() and getError()
* Then added a #displayRenderedContent($renderedContent) velocimacro to display an error,
calling Document.getError()
However that’s no good because we have lots of places in our code (and in extensions we
don’t control) that call getRenderedContent() and they expect the whole page rendering to
fail in case of an error…
Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
* Add a new #try() velocimacro that would push a new empty List<Exception> in the
Execution Context to a Stack variable
* Add a new uberspector that would, based on this flag do a try/catch around the method
called (can be done easily by returning a custom VelMethod)
* The uberspector catch would add the exception to the List<Exception> in the
Execution Context
* Add a new #catch() velocimacro that would pop from the Stack in the Execution Context
and that would set 2 variables in the Velocity Context:
** $exception: the first exception
** $exceptions: the list of all exceptions that happened between the #try() and #catch()
The good part with this is:
* No change in behavior to all existing code
* Code that want to do something in case of error can do it using #try()/#catch() for
java methods throwing exceptions (like getRenderedContent())
Of course for the future we would need to decide if script services should stop doing
try/catch or not but that’s another debate I’d prefer to separate from this thread.
WDYT?
Thanks
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Thomas Mortagne
5:59 p.m.
+1
Thanks,
Marius
On Thu, Sep 18, 2014 at 4:28 PM, vincent(a)massol.net <vincent(a)massol.net> wrote:
Hi Devs,
Example of a use case:
* In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
I’ve tried implementing the following:
* Modified Document.getRenderedContent() to not throw exceptions any more
* I’ve added Document.setError() and getError()
* Then added a #displayRenderedContent($renderedContent) velocimacro to display an error,
calling Document.getError()
However that’s no good because we have lots of places in our code (and in extensions we
don’t control) that call getRenderedContent() and they expect the whole page rendering to
fail in case of an error…
Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
* Add a new #try() velocimacro that would push a new empty List<Exception> in the
Execution Context to a Stack variable
* Add a new uberspector that would, based on this flag do a try/catch around the method
called (can be done easily by returning a custom VelMethod)
* The uberspector catch would add the exception to the List<Exception> in the
Execution Context
* Add a new #catch() velocimacro that would pop from the Stack in the Execution Context
and that would set 2 variables in the Velocity Context:
** $exception: the first exception
** $exceptions: the list of all exceptions that happened between the #try() and #catch()
The good part with this is:
* No change in behavior to all existing code
* Code that want to do something in case of error can do it using #try()/#catch() for
java methods throwing exceptions (like getRenderedContent())
Of course for the future we would need to decide if script services should stop doing
try/catch or not but that’s another debate I’d prefer to separate from this thread.
WDYT?
Thanks
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
7:23 p.m.
1. I would rather add proper directives instead of macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
2. So far it has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
b. We didn't do that enough, and as a workaround we're adding exception
handling in Velocity?
On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
Hi Devs,
Example of a use case:
* In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
I’ve tried implementing the following:
* Modified Document.getRenderedContent() to not throw exceptions any more
* I’ve added Document.setError() and getError()
* Then added a #displayRenderedContent($renderedContent) velocimacro to display an error,
calling Document.getError()
However that’s no good because we have lots of places in our code (and in extensions we
don’t control) that call getRenderedContent() and they expect the whole page rendering to
fail in case of an error…
Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
* Add a new #try() velocimacro that would push a new empty List<Exception> in the
Execution Context to a Stack variable
* Add a new uberspector that would, based on this flag do a try/catch around the method
called (can be done easily by returning a custom VelMethod)
* The uberspector catch would add the exception to the List<Exception> in the
Execution Context
* Add a new #catch() velocimacro that would pop from the Stack in the Execution Context
and that would set 2 variables in the Velocity Context:
** $exception: the first exception
** $exceptions: the list of all exceptions that happened between the #try() and #catch()
The good part with this is:
* No change in behavior to all existing code
* Code that want to do something in case of error can do it using #try()/#catch() for
java methods throwing exceptions (like getRenderedContent())
Of course for the future we would need to decide if script services should stop doing
try/catch or not but that’s another debate I’d prefer to separate from this thread.
WDYT?
Thanks
-Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Sergiu Dumitriu
http://purl.org/net/sergiu
7:41 p.m.
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
1. I would rather add proper directives instead of
macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it.
2. So far it has been the job of our scriptable
wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a workaround
we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a lot
of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
Thanks
-Vincent
On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> Hi Devs,
>
> Example of a use case:
>
> * In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
>
> I’ve tried implementing the following:
> * Modified Document.getRenderedContent() to not throw exceptions any more
> * I’ve added Document.setError() and getError()
> * Then added a #displayRenderedContent($renderedContent) velocimacro to display an
error, calling Document.getError()
>
> However that’s no good because we have lots of places in our code (and in extensions
we don’t control) that call getRenderedContent() and they expect the whole page rendering
to fail in case of an error…
>
> Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
>
> * Add a new #try() velocimacro that would push a new empty List in the Execution
Context to a Stack variable
> * Add a new uberspector that would, based on this flag do a try/catch around the
method called (can be done easily by returning a custom VelMethod)
> * The uberspector catch would add the exception to the List in the Execution
Context
> * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> ** $exception: the first exception
> ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
>
> The good part with this is:
> * No change in behavior to all existing code
> * Code that want to do something in case of error can do it using #try()/#catch()
for java methods throwing exceptions (like getRenderedContent())
>
> Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
>
> WDYT?
>
> Thanks
> -Vincent
19 Sep
19 Sep
12:04 a.m.
On 18 Sep 2014 at 19:41:56, vincent(a)massol.net
(vincent@massol.net(mailto:vincent@massol.net)) wrote:
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
I’ve now implemented a directive and it’s so cool :)
It’s so simple compared to what I had, I love it.
Here’s the usage code now:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#end
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
I like it so much that I’m going to commit it and if someone doesn’t agree I’ll revert the
commit :)
Thanks Sergiu
-Vincent
1. I would rather add proper directives instead
of macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it. 2. So far it
has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a
workaround we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a
lot of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent = "
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
…
Thanks
-Vincent
> On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> > Hi Devs,
> >
> > Example of a use case:
> >
> > * In contentview.vm we need to be able to catch when getRenderedContent()
throws an exception in order to be able to still display the page title, content menu and
last modification line. We want only the content part to display an error.
> >
> > I’ve tried implementing the following:
> > * Modified Document.getRenderedContent() to not throw exceptions any more
> > * I’ve added Document.setError() and getError()
> > * Then added a #displayRenderedContent($renderedContent) velocimacro to display
an error, calling Document.getError()
> >
> > However that’s no good because we have lots of places in our code (and in
extensions we don’t control) that call getRenderedContent() and they expect the whole page
rendering to fail in case of an error…
> >
> > Thus after discussing quickly on IRC and with Thomas we’d like to propose
instead:
> >
> > * Add a new #try() velocimacro that would push a new empty List in the
Execution Context to a Stack variable
> > * Add a new uberspector that would, based on this flag do a try/catch around
the method called (can be done easily by returning a custom VelMethod)
> > * The uberspector catch would add the exception to the List in the Execution
Context
> > * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> > ** $exception: the first exception
> > ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
> >
> > The good part with this is:
> > * No change in behavior to all existing code
> > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like getRenderedContent())
> >
> > Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
> >
> > WDYT?
> >
> > Thanks
> > -Vincent
8:46 a.m.
Looks good. I like it.
Thanks,
Marius
On Fri, Sep 19, 2014 at 1:04 AM, vincent(a)massol.net <vincent(a)massol.net> wrote:
On 18 Sep 2014 at 19:41:56, vincent(a)massol.net
(vincent@massol.net(mailto:vincent@massol.net)) wrote:
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
I’ve now implemented a directive and it’s so cool :)
It’s so simple compared to what I had, I love it.
Here’s the usage code now:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#end
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
I like it so much that I’m going to commit it and if someone doesn’t agree I’ll revert
the commit :)
Thanks Sergiu
-Vincent
1. I would rather add proper directives instead
of macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it. 2. So far
it has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a
workaround we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a
lot of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent = " #end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
…
Thanks
-Vincent
> On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> > Hi Devs,
> >
> > Example of a use case:
> >
> > * In contentview.vm we need to be able to catch when getRenderedContent()
throws an exception in order to be able to still display the page title, content menu and
last modification line. We want only the content part to display an error.
> >
> > I’ve tried implementing the following:
> > * Modified Document.getRenderedContent() to not throw exceptions any more
> > * I’ve added Document.setError() and getError()
> > * Then added a #displayRenderedContent($renderedContent) velocimacro to display
an error, calling Document.getError()
> >
> > However that’s no good because we have lots of places in our code (and in
extensions we don’t control) that call getRenderedContent() and they expect the whole page
rendering to fail in case of an error…
> >
> > Thus after discussing quickly on IRC and with Thomas we’d like to propose
instead:
> >
> > * Add a new #try() velocimacro that would push a new empty List in the
Execution Context to a Stack variable
> > * Add a new uberspector that would, based on this flag do a try/catch around
the method called (can be done easily by returning a custom VelMethod)
> > * The uberspector catch would add the exception to the List in the Execution
Context
> > * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> > ** $exception: the first exception
> > ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
> >
> > The good part with this is:
> > * No change in behavior to all existing code
> > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like getRenderedContent())
> >
> > Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
> >
> > WDYT?
> >
> > Thanks
> > -Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
10:24 a.m.
#try
was not possible ? (without parenthesis)
On Fri, Sep 19, 2014 at 12:04 AM, vincent(a)massol.net <vincent(a)massol.net> wrote:
On 18 Sep 2014 at 19:41:56, vincent(a)massol.net
(vincent@massol.net(mailto:vincent@massol.net)) wrote:
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
--
Thomas Mortagne
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
I’ve now implemented a directive and it’s so cool :)
It’s so simple compared to what I had, I love it.
Here’s the usage code now:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#end
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
I like it so much that I’m going to commit it and if someone doesn’t agree I’ll revert
the commit :)
Thanks Sergiu
-Vincent
1. I would rather add proper directives instead
of macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it. 2. So far
it has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a
workaround we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a
lot of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're already
inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent = " #end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
…
Thanks
-Vincent
> On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> > Hi Devs,
> >
> > Example of a use case:
> >
> > * In contentview.vm we need to be able to catch when getRenderedContent()
throws an exception in order to be able to still display the page title, content menu and
last modification line. We want only the content part to display an error.
> >
> > I’ve tried implementing the following:
> > * Modified Document.getRenderedContent() to not throw exceptions any more
> > * I’ve added Document.setError() and getError()
> > * Then added a #displayRenderedContent($renderedContent) velocimacro to display
an error, calling Document.getError()
> >
> > However that’s no good because we have lots of places in our code (and in
extensions we don’t control) that call getRenderedContent() and they expect the whole page
rendering to fail in case of an error…
> >
> > Thus after discussing quickly on IRC and with Thomas we’d like to propose
instead:
> >
> > * Add a new #try() velocimacro that would push a new empty List in the
Execution Context to a Stack variable
> > * Add a new uberspector that would, based on this flag do a try/catch around
the method called (can be done easily by returning a custom VelMethod)
> > * The uberspector catch would add the exception to the List in the Execution
Context
> > * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> > ** $exception: the first exception
> > ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
> >
> > The good part with this is:
> > * No change in behavior to all existing code
> > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like getRenderedContent())
> >
> > Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
> >
> > WDYT?
> >
> > Thanks
> > -Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
2:48 p.m.
On 19 Sep 2014 at 10:24:51, Thomas Mortagne
(thomas.mortagne@xwiki.com(mailto:thomas.mortagne@xwiki.com)) wrote:
#try
was not possible ? (without parenthesis)
AFAIK it’s not possible since BLOCK directives in Velocity need parenthesis.
Thanks
-Vincent
On Fri, Sep 19, 2014 at 12:04 AM, vincent(a)massol.net
wrote:
>
>
> On 18 Sep 2014 at 19:41:56, vincent(a)massol.net
(vincent@massol.net(mailto:vincent@massol.net)) wrote:
>
>> Hi Sergiu,
>>
>> On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu
(sergiu@xwiki.org(mailto:sergiu@xwiki.org)) wrote:
>>
>> > 1. I would rather add proper directives instead of macros and
>> > uberspector hacks: http://stackoverflow.com/q/159292/620249
>>
>> Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I
didn’t double check. I should have since I’ve now finished my implementation and redoing
it as a directive means dumping what I’ve done a third time in the end. Anyway I agree
that a directive is better so I’ll redo it.
>
> I’ve now implemented a directive and it’s so cool :)
>
> It’s so simple compared to what I had, I love it.
>
> Here’s the usage code now:
>
> #try()
> #set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
> #if ($outputSyntax)
> ## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it
so that it can be
> ## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
> ## panels on the side, header, etc).
> #set($syntaxType = $outputSyntax.type.toIdString())
> #if (($syntaxType == "xhtml") || ($syntaxType == "html"))
> #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
> #else
> ## Make sure to print correctly the result when it's not HTML
> #set ($renderedContent = "
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
> #end
> #else
> #set ($renderedContent = $tdoc.getRenderedContent())
> #end
> #end
> …
>
> #if ("$!exception" != '')
> #displayException($exception)
> #else
> $renderedContent
> #end
>
> …
>
> I like it so much that I’m going to commit it and if someone doesn’t agree I’ll
revert the commit :)
>
> Thanks Sergiu
> -Vincent
>
>> > 2. So far it has been the job of our scriptable wrappers to catch
>> > exceptions, does this proposal mean that:
>> > a. We should stop doing that and write try-catch logic in our
>> > templates/scripts? IOW, catching exceptions in wrappers is a bad design
>> > pattern?
>>
>> I’ve not answered this question because I don’t know yet. I think we’ll discover
that as we start using the new try/catch. ATM I’m only planning to use it in
contentview.vm.
>>
>> > b. We didn't do that enough, and as a workaround we're adding
exception
>> > handling in Velocity?
>>
>> Yes, that’s true. We now have both cases in our code (code throwing exceptions,
like a lot of methods in Document, XWiki) and others catching exceptions.
>>
>> This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
>>
>> FTR here’s what I have so far in term of usage, in contentview.vm:
>>
>> #try()
>> #set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
>> #if ($outputSyntax)
>> ## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape
it so that it can be
>> ## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
>> ## panels on the side, header, etc).
>> #set($syntaxType = $outputSyntax.type.toIdString())
>> #if (($syntaxType == "xhtml") || ($syntaxType == "html"))
>> #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
>> #else
>> ## Make sure to print correctly the result when it's not HTML
>> #set ($renderedContent = "
> $escapetool.html($tdoc.getRenderedContent($outputSyntax))
> ")
>> #end
>> #else
>> #set ($renderedContent = $tdoc.getRenderedContent())
>> #end
>> #catch()
>>
>> …
>>
>
>> #if ("$!exception" != '')
>> #displayException($exception)
>> #else
>> $renderedContent
>> #end
>>
>
>> …
>>
>>
>> Thanks
>> -Vincent
>>
>> > On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
>> > > Hi Devs,
>> > >
>> > > Example of a use case:
>> > >
>> > > * In contentview.vm we need to be able to catch when
getRenderedContent() throws an exception in order to be able to still display the page
title, content menu and last modification line. We want only the content part to display
an error.
>> > >
>> > > I’ve tried implementing the following:
>> > > * Modified Document.getRenderedContent() to not throw exceptions any
more
>> > > * I’ve added Document.setError() and getError()
>> > > * Then added a #displayRenderedContent($renderedContent) velocimacro
to display an error, calling Document.getError()
>> > >
>> > > However that’s no good because we have lots of places in our code (and
in extensions we don’t control) that call getRenderedContent() and they expect the whole
page rendering to fail in case of an error…
>> > >
>> > > Thus after discussing quickly on IRC and with Thomas we’d like to
propose instead:
>> > >
>> > > * Add a new #try() velocimacro that would push a new empty List in the
Execution Context to a Stack variable
>> > > * Add a new uberspector that would, based on this flag do a try/catch
around the method called (can be done easily by returning a custom VelMethod)
>> > > * The uberspector catch would add the exception to the List in the
Execution Context
>> > > * Add a new #catch() velocimacro that would pop from the Stack in the
Execution Context and that would set 2 variables in the Velocity Context:
>> > > ** $exception: the first exception
>> > > ** $exceptions: the list of all exceptions that happened between the
#try() and #catch()
>> > >
>> > > The good part with this is:
>> > > * No change in behavior to all existing code
>> > > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like getRenderedContent())
>> > >
>> > > Of course for the future we would need to decide if script services
should stop doing try/catch or not but that’s another debate I’d prefer to separate from
this thread.
>> > >
>> > > WDYT?
>> > >
>> > > Thanks
>> > > -Vincent
1 Oct
1 Oct
12:45 p.m.
Hi,
Interesting, though I do have my worries about having to deal with actual
exceptions in velocity. Probably the most common usecase would be just to
display the exception, but others may now be tempted to look at exception
types and display custom messages for each error situation. That should
improve the quality of the UIs, but at the same time I feel like it will
increase the amount of velocity code in wiki pages.
Generally, I`m +1 since I had to deal on some occasions with exception
handling in velocity and found myself re-coding over and over again the
methanism of passing the exception from the script service to the velocity
UI.
However, I feel there are some things that are could be done to improve it
in practice:
1. Make sure that the "try" directive clears any existing context exception
before executing it's content and possibly setting a new exception in the
context. Example:
#try()
<some code>
#end
<handle $exception>
#try()
<some other code>
#end
<handle $exception, but which code's exception is this?>
2. Support nested exception handling. Example:
#try()
<some code>
#try()
<some nested code>
#end
<handle $exception>
#end
<handle $exception, but is it the nested exception or the main one?>
I agree that this point (2.) could probably be solved by (1.) if the "try"
directive clears the context exception *after* executing the code so that
any leftovers from nested exceptions are cleared.
2.1. However, my initial suggestion was to maybe support "named exceptions"
(i.e. a parameter for the "try" directive that could specify the variable
where to expose the exception). Example:
#try()
<some code>
#try("nestedException") <-- or #try($nestedException) but I think this
is more problematic since it really has to be set to $NULL before
<some nested code>
#end
<handle $nestedException>
#end
<handle $exception>
2.2 Thinking of the above and the nested usecase, I just wanted to mention
a possible #throw($exception) directive in case we want to use the main
"try" block as a catch-all handler, even if nested try blocks caught and
reported errors as they occurred (so they would now bubble the exception
towards the main try block to signal that the operation as a whole was
problematic). Example:
#try()
<some code>
#try()
<some nested code>
#end
#if ($exception)
<handle $exception>
#throw($exception) <-- this probably stops the execution here.
#end
#end
<catch-all handling of $exception>
Of course, this can be done with a variable instead in the exception
handling of each nested try blocks. Example:
#try()
<some code>
#try()
<some nested code>
#end
#if ($exception)
<handle $exception>
#set($failed=true) <-- this leaves the developer to decide by using
some IFs.
#end
#end
<handle of any $exception uncaught by the nested try blocks, also
considering the $failed variable>
Point 1. is a must IMO to avoid unwanted errors. The rest is just food for
thought, perhaps you consider them useful as well.
Thanks,
Eduard
On Fri, Sep 19, 2014 at 3:48 PM, vincent(a)massol.net <vincent(a)massol.net>
wrote:
On 19 Sep 2014 at 10:24:51, Thomas Mortagne (thomas.mortagne(a)xwiki.com
(mailto:thomas.mortagne@xwiki.com)) wrote:
#try
was not possible ? (without parenthesis)
AFAIK it’s not possible since BLOCK directives in Velocity need
parenthesis.
Thanks
-Vincent
On Fri, Sep 19, 2014 at 12:04 AM,
vincent(a)massol.net wrote:
>
>
> On 18 Sep 2014 at 19:41:56, vincent(a)massol.net (vincent(a)massol.net
(mailto:vincent@massol.net)) wrote:
>
>> Hi Sergiu,
>>
>> On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu(a)xwiki.org(mailto:
sergiu(a)xwiki.org)) wrote:
>>
>> > 1. I would rather add proper directives instead of macros and
>> > uberspector hacks: http://stackoverflow.com/q/159292/620249
>>
>> Argh… I wanted that and I asked Thomas and he told me it wasn’t
possible
and I didn’t double check. I should have since I’ve now finished
my implementation and redoing it as a directive means dumping what I’ve
done a third time in the end. Anyway I agree that a directive is better so
I’ll redo it.
>
> I’ve now implemented a directive and it’s so cool :)
>
> It’s so simple compared to what I had, I love it.
>
> Here’s the usage code now:
>
> #try()
> #set($outputSyntax =
$xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
> #if ($outputSyntax)
> ## If the passed syntax is not an HTML-compatible syntax we need to
HTML-escape it so that it can be
> ## displayed fine in HTML (since at the
point this vm file is called
we're already inside an HTML page with
> ## panels on the side, header, etc).
> #set($syntaxType = $outputSyntax.type.toIdString())
> #if (($syntaxType == "xhtml") || ($syntaxType == "html"))
> #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
> #else
> ## Make sure to print correctly the result when it's not HTML
> #set ($renderedContent = "
$escapetool.html($tdoc.getRenderedContent($outputSyntax))
")
> #end
> #else
> #set ($renderedContent = $tdoc.getRenderedContent())
> #end
> #end
> …
>
> #if ("$!exception" != '')
> #displayException($exception)
> #else
> $renderedContent
> #end
>
> …
>
> I like it so much that I’m going to commit it and if someone doesn’t
agree
I’ll revert the commit :)
>
> Thanks Sergiu
> -Vincent
>
>> > 2. So far it has been the job of our scriptable wrappers to catch
>> > exceptions, does this proposal mean that:
>> > a. We should stop doing that and write try-catch logic in our
>> > templates/scripts? IOW, catching exceptions in wrappers is a bad
design
>> > pattern?
>>
>> I’ve not answered this question because I don’t know yet. I think
we’ll
discover that as we start using the new try/catch. ATM I’m only
planning to use it in contentview.vm.
>>
>> > b. We didn't do that enough, and as a workaround we're adding
exception
>> > handling in Velocity?
>>
>> Yes, that’s true. We now have both cases in our code (code throwing
exceptions, like a lot of methods in Document, XWiki) and others catching
exceptions.
>>
>> This proposals offers a finer-grained control in vm while not
breaking
backward compatibility. Any change in our script APIs would break
compatibility (this is what I started doing before realizing it would break
too many things)…
>>
>> FTR here’s what I have so far in term of usage, in contentview.vm:
>>
>> #try()
>> #set($outputSyntax =
$xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
>> #if ($outputSyntax)
>> ## If the passed syntax is not an HTML-compatible syntax we need to
HTML-escape it so that it can be
>> ## displayed fine in HTML (since at the
point this vm file is called
we're already inside an HTML page with
>> ## panels on the side, header, etc).
>> #set($syntaxType = $outputSyntax.type.toIdString())
>> #if (($syntaxType == "xhtml") || ($syntaxType == "html"))
>> #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
>> #else
>> ## Make sure to print correctly the result when it's not HTML
>> #set ($renderedContent = "
> $escapetool.html($tdoc.getRenderedContent($outputSyntax))
> ")
>> #end
>> #else
>> #set ($renderedContent = $tdoc.getRenderedContent())
>> #end
>> #catch()
>>
>> …
>>
>
>> #if ("$!exception" != '')
>> #displayException($exception)
>> #else
>> $renderedContent
>> #end
>>
>
>> …
>>
>>
>> Thanks
>> -Vincent
>>
>> > On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
>> > > Hi Devs,
>> > >
>> > > Example of a use case:
>> > >
>> > > * In contentview.vm we need to be able to catch when
getRenderedContent() throws an exception in order to be able to still
display the page title, content menu and last modification line. We want
only the content part to display an error.
>> > >
>> > > I’ve tried implementing the following:
>> > > * Modified Document.getRenderedContent() to not throw exceptions
any more
>> > > * I’ve added
Document.setError() and getError()
>> > > * Then added a #displayRenderedContent($renderedContent)
velocimacro to display an error, calling Document.getError()
>> > >
>> > > However that’s no good because we have lots of places in our code
(and in extensions we don’t control) that call getRenderedContent() and
they expect the whole page rendering to fail in case of an error…
>> > >
>> > > Thus after discussing quickly on IRC and with Thomas we’d like to
propose instead:
>> > >
>> > > * Add a new #try() velocimacro that would push a new empty List
in the Execution Context to a Stack variable
>> > > * Add a new uberspector that
would, based on this flag do a
try/catch around the method called (can be done
easily by returning a
custom VelMethod)
>> > > * The uberspector catch would
add the exception to the List in
the Execution Context
>> > > * Add a new #catch()
velocimacro that would pop from the Stack in
the Execution Context and that would
set 2 variables in the Velocity
Context:
>> > > ** $exception: the first
exception
>> > > ** $exceptions: the list of all exceptions that happened between
the #try() and #catch()
>> > >
>> > > The good part with this is:
>> > > * No change in behavior to all existing code
>> > > * Code that want to do something in case of error can do it using
#try()/#catch() for java methods throwing exceptions (like
getRenderedContent())
>> > >
>> > > Of course for the future we would need to decide if script
services should stop doing try/catch or not but that’s another debate I’d
prefer to separate from this thread.
>> > >
>> > > WDYT?
>> > >
>> > > Thanks
>> > > -Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
1:40 p.m.
New subject: [xwiki-devs] [Proposal] Adding new velocity methods to simulate try/catch in Velocity
definitely useful in many cases.
Procedures you expect is in the view code.
The rest, maybe just a polite error message, is in the catch code.
paul
On 1 oct. 2014, at 12:45, Eduard Moraru <enygma2002(a)gmail.com> wrote:
That should
improve the quality of the UIs, but at the same time I feel like it will
increase the amount of velocity code in wiki pages.
2:44 p.m.
Hi Edy,
On 1 Oct 2014 at 12:45:52, Eduard Moraru
(enygma2002@gmail.com(mailto:enygma2002@gmail.com)) wrote:
Hi,
Interesting, though I do have my worries about having to deal with actual
exceptions in velocity. Probably the most common usecase would be just to
display the exception, but others may now be tempted to look at exception
types and display custom messages for each error situation. That should
improve the quality of the UIs, but at the same time I feel like it will
increase the amount of velocity code in wiki pages.
Generally, I`m +1 since I had to deal on some occasions with exception
handling in velocity and found myself re-coding over and over again the
methanism of passing the exception from the script service to the velocity
UI.
However, I feel there are some things that are could be done to improve it
in practice:
1. Make sure that the "try" directive clears any existing context exception
before executing it's content and possibly setting a new exception in the
context. Example:
#try()
#end
#try()
#end
I was going to say that it’s already the case but then I checked the code and indeed I
changed the implementation when I moved to the Directive approach suggested by Sergiu and
I forgot to clear it. It’s done now.
2. Support nested exception handling. Example:
#try()
#try()
#end
#end
I agree that this point (2.) could probably be solved by (1.) if the "try"
directive clears the context exception *after* executing the code so that
any leftovers from nested exceptions are cleared.
2.1. However, my initial suggestion was to maybe support "named exceptions"
(i.e. a parameter for the "try" directive that could specify the variable
where to expose the exception). Example:
#try()
#try("nestedException") <-- or #try($nestedException) but I think this
is more problematic since it really has to be set to $NULL before
#end
#end
2.2 Thinking of the above and the nested usecase, I just wanted to mention
a possible #throw($exception) directive in case we want to use the main
"try" block as a catch-all handler, even if nested try blocks caught and
reported errors as they occurred (so they would now bubble the exception
towards the main try block to signal that the operation as a whole was
problematic). Example:
#try()
#try()
#end
#if ($exception)
#throw($exception) <-- this probably stops the execution here.
#end
#end
Of course, this can be done with a variable instead in the exception
handling of each nested try blocks. Example:
#try()
#try()
#end
#if ($exception)
#set($failed=true) <-- this leaves the developer to decide by using
some IFs.
#end
#end
Yes, good points that we can review after using it for real in some code.
Point 1 is fixed now.
Thanks
-Vincent
considering the $failed variable>
Point 1. is a must IMO to avoid unwanted errors. The rest is just food for
thought, perhaps you consider them useful as well.
Thanks,
Eduard
On Fri, Sep 19, 2014 at 3:48 PM, vincent(a)massol.net
wrote:
>
>
>
>
> On 19 Sep 2014 at 10:24:51, Thomas Mortagne (thomas.mortagne(a)xwiki.com
> (mailto:thomas.mortagne@xwiki.com)) wrote:
>
> > #try
> >
> > was not possible ? (without parenthesis)
>
> AFAIK it’s not possible since BLOCK directives in Velocity need
> parenthesis.
>
> Thanks
> -Vincent
>
> > On Fri, Sep 19, 2014 at 12:04 AM, vincent(a)massol.net wrote:
> > >
> > >
> > > On 18 Sep 2014 at 19:41:56, vincent(a)massol.net (vincent(a)massol.net
> (mailto:vincent@massol.net)) wrote:
> > >
> > >> Hi Sergiu,
> > >>
> > >> On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu(a)xwiki.org(mailto:
> sergiu(a)xwiki.org)) wrote:
> > >>
> > >> > 1. I would rather add proper directives instead of macros and
> > >> > uberspector hacks: http://stackoverflow.com/q/159292/620249
> > >>
> > >> Argh… I wanted that and I asked Thomas and he told me it wasn’t
> possible and I didn’t double check. I should have since I’ve now finished
> my implementation and redoing it as a directive means dumping what I’ve
> done a third time in the end. Anyway I agree that a directive is better so
> I’ll redo it.
> > >
> > > I’ve now implemented a directive and it’s so cool :)
> > >
> > > It’s so simple compared to what I had, I love it.
> > >
> > > Here’s the usage code now:
> > >
> > > #try()
> > > #set($outputSyntax =
> $xwiki.getAvailableRendererSyntax($request.outputSyntax,
> $request.outputSyntaxVersion))
> > > #if ($outputSyntax)
> > > ## If the passed syntax is not an HTML-compatible syntax we need to
> HTML-escape it so that it can be
> > > ## displayed fine in HTML (since at the point this vm file is called
> we're already inside an HTML page with
> > > ## panels on the side, header, etc).
> > > #set($syntaxType = $outputSyntax.type.toIdString())
> > > #if (($syntaxType == "xhtml") || ($syntaxType ==
"html"))
> > > #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
> > > #else
> > > ## Make sure to print correctly the result when it's not HTML
> > > #set ($renderedContent = "
> $escapetool.html($tdoc.getRenderedContent($outputSyntax))
> ")
> > > #end
> > > #else
> > > #set ($renderedContent = $tdoc.getRenderedContent())
> > > #end
> > > #end
> > > …
> > >
>
> > > #if ("$!exception" != '')
> > > #displayException($exception)
> > > #else
> > > $renderedContent
> > > #end
> > >
>
> > > …
> > >
> > > I like it so much that I’m going to commit it and if someone doesn’t
> agree I’ll revert the commit :)
> > >
> > > Thanks Sergiu
> > > -Vincent
> > >
> > >> > 2. So far it has been the job of our scriptable wrappers to
catch
> > >> > exceptions, does this proposal mean that:
> > >> > a. We should stop doing that and write try-catch logic in our
> > >> > templates/scripts? IOW, catching exceptions in wrappers is a bad
> design
> > >> > pattern?
> > >>
> > >> I’ve not answered this question because I don’t know yet. I think
> we’ll discover that as we start using the new try/catch. ATM I’m only
> planning to use it in contentview.vm.
> > >>
> > >> > b. We didn't do that enough, and as a workaround we're
adding
> exception
> > >> > handling in Velocity?
> > >>
> > >> Yes, that’s true. We now have both cases in our code (code throwing
> exceptions, like a lot of methods in Document, XWiki) and others catching
> exceptions.
> > >>
> > >> This proposals offers a finer-grained control in vm while not
> breaking backward compatibility. Any change in our script APIs would break
> compatibility (this is what I started doing before realizing it would break
> too many things)…
> > >>
> > >> FTR here’s what I have so far in term of usage, in contentview.vm:
> > >>
> > >> #try()
> > >> #set($outputSyntax =
> $xwiki.getAvailableRendererSyntax($request.outputSyntax,
> $request.outputSyntaxVersion))
> > >> #if ($outputSyntax)
> > >> ## If the passed syntax is not an HTML-compatible syntax we need to
> HTML-escape it so that it can be
> > >> ## displayed fine in HTML (since at the point this vm file is called
> we're already inside an HTML page with
> > >> ## panels on the side, header, etc).
> > >> #set($syntaxType = $outputSyntax.type.toIdString())
> > >> #if (($syntaxType == "xhtml") || ($syntaxType ==
"html"))
> > >> #set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
> > >> #else
> > >> ## Make sure to print correctly the result when it's not HTML
> > >> #set ($renderedContent = "
> > > $escapetool.html($tdoc.getRenderedContent($outputSyntax))
> > > ")
> > >> #end
> > >> #else
> > >> #set ($renderedContent = $tdoc.getRenderedContent())
> > >> #end
> > >> #catch()
> > >>
> > >> …
> > >>
> > >
> > >> #if ("$!exception" != '')
> > >> #displayException($exception)
> > >> #else
> > >> $renderedContent
> > >> #end
> > >>
> > >
> > >> …
> > >>
> > >>
> > >> Thanks
> > >> -Vincent
> > >>
> > >> > On 09/18/2014 09:28 AM, vincent(a)massol.net wrote:
> > >> > > Hi Devs,
> > >> > >
> > >> > > Example of a use case:
> > >> > >
> > >> > > * In contentview.vm we need to be able to catch when
> getRenderedContent() throws an exception in order to be able to still
> display the page title, content menu and last modification line. We want
> only the content part to display an error.
> > >> > >
> > >> > > I’ve tried implementing the following:
> > >> > > * Modified Document.getRenderedContent() to not throw
exceptions
> any more
> > >> > > * I’ve added Document.setError() and getError()
> > >> > > * Then added a #displayRenderedContent($renderedContent)
> velocimacro to display an error, calling Document.getError()
> > >> > >
> > >> > > However that’s no good because we have lots of places in our
code
> (and in extensions we don’t control) that call getRenderedContent() and
> they expect the whole page rendering to fail in case of an error…
> > >> > >
> > >> > > Thus after discussing quickly on IRC and with Thomas we’d
like to
> propose instead:
> > >> > >
> > >> > > * Add a new #try() velocimacro that would push a new empty
List
> in the Execution Context to a Stack variable
> > >> > > * Add a new uberspector that would, based on this flag do a
> try/catch around the method called (can be done easily by returning a
> custom VelMethod)
> > >> > > * The uberspector catch would add the exception to the List
in
> the Execution Context
> > >> > > * Add a new #catch() velocimacro that would pop from the
Stack in
> the Execution Context and that would set 2 variables in the Velocity
> Context:
> > >> > > ** $exception: the first exception
> > >> > > ** $exceptions: the list of all exceptions that happened
between
> the #try() and #catch()
> > >> > >
> > >> > > The good part with this is:
> > >> > > * No change in behavior to all existing code
> > >> > > * Code that want to do something in case of error can do it
using
> #try()/#catch() for java methods throwing exceptions (like
> getRenderedContent())
> > >> > >
> > >> > > Of course for the future we would need to decide if script
> services should stop doing try/catch or not but that’s another debate I’d
> prefer to separate from this thread.
> > >> > >
> > >> > > WDYT?
> > >> > >
> > >> > > Thanks
> > >> > > -Vincent
19 Sep
19 Sep
10:21 a.m.
On Thu, Sep 18, 2014 at 7:41 PM, vincent(a)massol.net <vincent(a)massol.net> wrote:
--
Thomas Mortagne
Hi Sergiu,
On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu (sergiu@xwiki.org(mailto:sergiu@xwiki.org))
wrote:
I said I did not know, not that it was not possible for sure.
1. I would rather add proper directives instead
of macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
Argh… I wanted that and I asked Thomas and he told me it wasn’t possible and I didn’t
double check. I should have since I’ve now finished my implementation and redoing it as a
directive means dumping what I’ve done a third time in the end. Anyway I agree that a
directive is better so I’ll redo it. 2. So far it has been the job of our scriptable
wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
I’ve not answered this question because I don’t know yet. I think we’ll discover that as
we start using the new try/catch. ATM I’m only planning to use it in contentview.vm.
b. We didn't do that enough, and as a
workaround we're adding exception
handling in Velocity?
Yes, that’s true. We now have both cases in our code (code throwing exceptions, like a
lot of methods in Document, XWiki) and others catching exceptions.
This proposals offers a finer-grained control in vm while not breaking backward
compatibility. Any change in our script APIs would break compatibility (this is what I
started doing before realizing it would break too many things)…
FTR here’s what I have so far in term of usage, in contentview.vm:
#try()
#set($outputSyntax = $xwiki.getAvailableRendererSyntax($request.outputSyntax,
$request.outputSyntaxVersion))
#if ($outputSyntax)
## If the passed syntax is not an HTML-compatible syntax we need to HTML-escape it so
that it can be
## displayed fine in HTML (since at the point this vm file is called we're
already inside an HTML page with
## panels on the side, header, etc).
#set($syntaxType = $outputSyntax.type.toIdString())
#if (($syntaxType == "xhtml") || ($syntaxType == "html"))
#set ($renderedContent = $tdoc.getRenderedContent($outputSyntax))
#else
## Make sure to print correctly the result when it's not HTML
#set ($renderedContent =
"<pre>$escapetool.html($tdoc.getRenderedContent($outputSyntax))</pre>")
#end
#else
#set ($renderedContent = $tdoc.getRenderedContent())
#end
#catch()
…
<div id="xwikicontent">
#if ("$!exception" != '')
#displayException($exception)
#else
$renderedContent
#end
</div>
…
Thanks
-Vincent
On 09/18/2014 09:28 AM, vincent(a)massol.net
wrote:
> Hi Devs,
>
> Example of a use case:
>
> * In contentview.vm we need to be able to catch when getRenderedContent() throws an
exception in order to be able to still display the page title, content menu and last
modification line. We want only the content part to display an error.
>
> I’ve tried implementing the following:
> * Modified Document.getRenderedContent() to not throw exceptions any more
> * I’ve added Document.setError() and getError()
> * Then added a #displayRenderedContent($renderedContent) velocimacro to display an
error, calling Document.getError()
>
> However that’s no good because we have lots of places in our code (and in extensions
we don’t control) that call getRenderedContent() and they expect the whole page rendering
to fail in case of an error…
>
> Thus after discussing quickly on IRC and with Thomas we’d like to propose instead:
>
> * Add a new #try() velocimacro that would push a new empty List in the Execution
Context to a Stack variable
> * Add a new uberspector that would, based on this flag do a try/catch around the
method called (can be done easily by returning a custom VelMethod)
> * The uberspector catch would add the exception to the List in the Execution
Context
> * Add a new #catch() velocimacro that would pop from the Stack in the Execution
Context and that would set 2 variables in the Velocity Context:
> ** $exception: the first exception
> ** $exceptions: the list of all exceptions that happened between the #try() and
#catch()
>
> The good part with this is:
> * No change in behavior to all existing code
> * Code that want to do something in case of error can do it using #try()/#catch()
for java methods throwing exceptions (like getRenderedContent())
>
> Of course for the future we would need to decide if script services should stop
doing try/catch or not but that’s another debate I’d prefer to separate from this thread.
>
> WDYT?
>
> Thanks
> -Vincent
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
10:32 a.m.
New subject: [xwiki-devs] [Proposal] Adding new velocity methods to simulate try/catch in Velocity
It would be soooooo cool to be able to count on a test-run with velocity to display us (in
the face) all the exceptions velocity is silently swallowing!
Probably that's not entirely covered in this proposal, but something that diminishes
my frustration of a velocity that keeps swallowing way too many exceptions. If we have a
try/catch we can start making velocity less tolerant and, for example, stop the ridiculous
conversion null == true == undefined == compilation-error that currently happens inside an
#if.
paul
On 18 sept. 2014, at 19:23, Sergiu Dumitriu <sergiu(a)xwiki.org> wrote:
1. I would rather add proper directives instead of
macros and
uberspector hacks: http://stackoverflow.com/q/159292/620249
2. So far it has been the job of our scriptable wrappers to catch
exceptions, does this proposal mean that:
a. We should stop doing that and write try-catch logic in our
templates/scripts? IOW, catching exceptions in wrappers is a bad design
pattern?
b. We didn't do that enough, and as a workaround we're adding exception
handling in Velocity?
3735
days inactive
3748
days old
13 comments
6 participants
participants (6)
-
Eduard Moraru -
Marius Dumitru Florea -
Paul Libbrecht -
Sergiu Dumitriu -
Thomas Mortagne -
vincent＠massol.net