Hi Sergiu, On 18 Sep 2014 at 19:29:52, Sergiu Dumitriu ([email protected](mailto:[email protected])) 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, [email protected] 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 [email protected] http://lists.xwiki.org/mailman/listinfo/devs
