Product Development

Page tree
Skip to end of metadata
  • Created on 2015-01-19, last edited by Jan Haderka on 2015-02-16
Go to start of metadata

Current Status

Implemented for Magnolia 5.4

The final/chosen solution (not really described below)  was to extend current exception handler to work also without context, but still be based in existing rendering module.

 

Render Exception Handler

We currently have RenderExceptionHandler which is able to render errors (e.g. from Freemarker). It's not possible to use it outside of rendering since RenderingContext is not set.

    <component>
      <type>info.magnolia.rendering.engine.RenderExceptionHandler</type>
      <implementation>info.magnolia.rendering.engine.ModeDependentRenderExceptionHandler</implementation>
    </component>

Suggested generic implementation

green=current implementation, black = suggested implementation

ClassMethod
 
Interfaces
info.magnolia.exception.ExceptionHandler<E extends Exception>

void handleException(T exception, Appendable out);

info.magnolia.rendering.engine.RenderExceptionHandler extends ExceptionHandler<RenderException>

void handleException(RenderException renderException, RenderingContext renderingContext) @deprecated

void handleException(RenderException renderException, Appendable out);

Implementations
info.magnolia.exception.ModeDependentExceptionHandler implements ExceptionHandler

void handleException(Exception exception, Appendable out);

info.magnolia.rendering.engine.ModeDependentRenderExceptionHandler extends ModeDependentExceptionHandler implements RenderExceptionHandler

void handleException(RenderException renderException, RenderingContext renderingContext) @deprecated

void handleException(RenderException renderException, Appendable out);

Suggested RenderingContext-independent method

green=current implementation, black = suggested implementation

ClassMethod
 
Interfaces
info.magnolia.rendering.engine.RenderExceptionHandler

void handleException(RenderException renderException, RenderingContext renderingContext)

void handleException(RenderException renderException, Appendable out);

Implementations
info.magnolia.rendering.engine.ModeDependentRenderExceptionHandler implements RenderExceptionHandler

void handleException(RenderException renderException, RenderingContext renderingContext)

void handleException(RenderException renderException, Appendable out);

JIRA

MAGNOLIA-6047 - Getting issue details... STATUS

  • No labels

10 Comments

  1. Magnolia International

    there is either (or both) another concept page and jira issue(s) that describe the same thing, can you find and link them ?

    I think it's a good idea, but a) do we _need_ it for 5.4, or is this sthg you just thought would be nice ? (i agree it would) b) i think we still need the freemarker-specific one, to let freemarker know what to do with exceptions (and using it provides the "nice" (i.e ugly but detailed) error messages for templaters) I see we've already somewhat attempted to make it freemarker-agnostic, but at the same time it still shows the freemarker exceptions ? not sure how it's been done:)

    1. Roman Kovařík

      Possibly related concepts: The only one I found is Concept Filter exception handler. But it's about handling of filter exceptions. 

      a) I had to copy most of the code from RenderExceptionHandler to provide same functionality for SiteMesh, see MSITEMESH-15.

      b) Yes, it's already freemarker-agnostic, FreemarkerRenderer just wraps TemplateException as RenderException. The goal is to make something rendering-agnostic.

      1. Magnolia International

        Ha, found the JIRA issue:  MAGNOLIA-3595 - Getting issue details... STATUS .

        I'm not sure why you make a distinction; it'll always happen in a filter, no ? Or did you want to explicitly call the exception handler from within the SiteMesh filter ? The other concept is indeed more about treating "any" exception (based on the type, it'd be pick a different handler).

        The current impl (rendering-specific) is probably still useful; the RenderingContext can provide info to handler that Appendable obviously can't.

        1. Roman Kovařík

          Yes, I want to explicitly call the exception handler from within the SiteMesh filter (per every failed tag).

          I would keep RenderExceptionHandler (I believe that RenderingContext can be injected), I would just extend it from a more generic one.

          1. Magnolia International

            Is it because you also want to do "mode dependent" stuff ? (e.g treat sitemesh exception different in author vs public)

            1. Roman Kovařík

              Exacly, e.g render an error when a fragment can't be requested. If I'd want to do the same in e.g. ModifyStream than I'd need to copy it again...

              1. Magnolia International

                Hmmm random thought without looking at current code; couldn't you just inject the (existing impl of) exception handler ?

                1. Roman Kovařík

                  It's not possible to use it outside of rendering since informations from RenderingContext  are not relevant and also I don't want to be directly dependent on Rendering...

                   

  2. Magnolia International

    To summarize what we discussed about on 2015-01-22, and after another coffee, here's my current thinking. Feel free to try your proposal, then we can compare ? 

    • The need is a) distinguish between author and public b) treat each exception without interrupting the whole process.
    • (a) is a simple if statement, but the existing mode-dependent exception handler in rendering provides pre-built exception message that work "well" within html pages
    • (b) seems very specific to a small number of cases; mainly tho, i see it be pertaining to "rendering" (not necessarily the module itself, but the idea/concept), where we want to continue rendering a page instead of interrupting at the first problem, so that authors can fix the problems they see.
    • As such, I'd consider maybe keeping the existing ModeDependentEH in rendering module, but changing its interface to what you propose. We could move the RenderingContext specific bit out of it and into RenderingEngine/Filter, and you could then probably use it as-is in the sitemesh filter ?

    1. Roman Kovařík

      I agree it's still tied to rendering idea. Let's keep the old method there to be compatible at least on the implementation level, see  MAGNOLIA-6047 - Getting issue details... STATUS