MyFaces Core
  1. MyFaces Core
  2. MYFACES-3053 Improve error reporting and logging
  3. MYFACES-3191

Handing exception (in exception handler) from render response phase with forward/redirect is inconsistent

    Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.1.2-SNAPSHOT
    • Fix Version/s: None
    • Component/s: General
    • Labels:
      None
    • Environment:
      myfaces trunk

      Description

      View.xhtml snippet:
      <h:commandButton value="Submit">
      <f:setPropertyActionListener value="#

      {true}

      " target="#

      {bean.rendered}" />
      <f:ajax render="outputText" />
      </h:commandButton>
      <h:outputText rendered="#{bean.rendered}

      " id="outputText" value="#

      {bean.value}

      " />

      Bean.java snippet:
      public Object getValue()

      { throw new NullPointerException(); }

      Custom exception handler code:
      if (PhaseId.RENDER_RESPONSE.equals(currentPhaseId)) {
      try

      { nav.handleNavigation(facesContext, null, "/ErrorPage.xhtml?faces-redirect=true"); }

      finally

      { i.remove(); }

      }
      Modify view and exception handler to achieve following combinations (HTML request = comment out f:ajax, forward = remove ?faces-redirect=true)

      1) HTML request + forward: nothing in log, and malformed output is produced (no closing tags for </body> etc.)

      2) HTML request + redirect: OK , redirect to ErrorPage.xhtml is done

      3) AJAX request + forward: exception in log, bud no forward to ErrorPage performed

      4) AJAX request + redirect: exception in log, malformed XML produced (<?xml and <partial-response 2x in output) , no redirect performed

        Activity

        Hide
        Martin Kočí added a comment -

        attached first draft alpha1 patch for discussion; allows redirect in render response with partial request/response

        Show
        Martin Kočí added a comment - attached first draft alpha1 patch for discussion; allows redirect in render response with partial request/response
        Hide
        Leonardo Uribe added a comment -

        MyFaces org.apache.myfaces.renderkit.ErrorHandler has this line:

        if (!facesContext.getExternalContext().isResponseCommitted())

        { facesContext.getExternalContext().responseReset(); }
        Show
        Leonardo Uribe added a comment - MyFaces org.apache.myfaces.renderkit.ErrorHandler has this line: if (!facesContext.getExternalContext().isResponseCommitted()) { facesContext.getExternalContext().responseReset(); }
        Hide
        Leonardo Uribe added a comment -

        The problem here is to do that we need to buffer the response before send it to the client, so if an error occur, be able to reset the response and render the error page. This has a significant impact in performance, specially in memory usage. In client side state saving a part of the response is buffered to render the state token, but in server side it does not.

        The question is simple: it is really wanted to buffer all requests into memory before sent it to the server?

        I think it is possible to use StreamCharBuffer to do that and handle memory usage in a efficient way. There exists a method called ServletResponse.resetBuffer() but it is not useable because the buffer size should be bigger than the response size otherwise, the buffer is flushed automatically. Unfortunately there is no ExternalContext.responseResetBuffer() but there is ExternalContext.responseReset().

        Maybe this should be considered a flaw of servlet spec. It is possible to set the buffer in FaceletsViewDeclarationLanguage.renderView and change ExternalContext.responseReset() to reset that buffer too if it is called. Note that is something outside the spec, but is reasonable behavior, because if the user set the buffer size (with javax.faces.FACELETS_BUFFER_SIZE) to a enough big value, it will work anyway. I think the buffer stuff could be handled better by the servlet container, but in this case we don't have choice.

        Show
        Leonardo Uribe added a comment - The problem here is to do that we need to buffer the response before send it to the client, so if an error occur, be able to reset the response and render the error page. This has a significant impact in performance, specially in memory usage. In client side state saving a part of the response is buffered to render the state token, but in server side it does not. The question is simple: it is really wanted to buffer all requests into memory before sent it to the server? I think it is possible to use StreamCharBuffer to do that and handle memory usage in a efficient way. There exists a method called ServletResponse.resetBuffer() but it is not useable because the buffer size should be bigger than the response size otherwise, the buffer is flushed automatically. Unfortunately there is no ExternalContext.responseResetBuffer() but there is ExternalContext.responseReset(). Maybe this should be considered a flaw of servlet spec. It is possible to set the buffer in FaceletsViewDeclarationLanguage.renderView and change ExternalContext.responseReset() to reset that buffer too if it is called. Note that is something outside the spec, but is reasonable behavior, because if the user set the buffer size (with javax.faces.FACELETS_BUFFER_SIZE) to a enough big value, it will work anyway. I think the buffer stuff could be handled better by the servlet container, but in this case we don't have choice.
        Hide
        Martin Kočí added a comment -

        forward vs. redirect

        The nature of forward does not allow to perform it in render response phase:
        1) view.xhtml is rendered (lifecycle.render)
        2) an exception occurs
        3) exception handler forwards to ErrorPage.xhtml
        4) ErrorPage.xhtml is rendered

        step 1) can produce some output (html) elements and send it to client (depends on size of server buffer). With this combination it is obtainable combination of both views in client: output from view.xhtml and from ErrorPage.xhtml

        The goal is:
        1) provide reliable redirect from render response phase (works ok for HTML request, does not work for ajax)
        2) an attempt to forward in render response phase log as error (and ignore it?)

        Show
        Martin Kočí added a comment - forward vs. redirect The nature of forward does not allow to perform it in render response phase: 1) view.xhtml is rendered (lifecycle.render) 2) an exception occurs 3) exception handler forwards to ErrorPage.xhtml 4) ErrorPage.xhtml is rendered step 1) can produce some output (html) elements and send it to client (depends on size of server buffer). With this combination it is obtainable combination of both views in client: output from view.xhtml and from ErrorPage.xhtml The goal is: 1) provide reliable redirect from render response phase (works ok for HTML request, does not work for ajax) 2) an attempt to forward in render response phase log as error (and ignore it?)
        Hide
        Martin Kočí added a comment -

        from spec 2.1:
        ViewDeclarationLanguage.renderView() : All implementations must: Call endDocument() on the ResponseWriter.

        Order of invocations in render_response Lifecycle.render:
        0) before phase listeners
        1) ViewDeclarationLanguage.renderView() (calls responseWriter.endDocument())
        2) after phase listeners
        3) facesContext.getExceptionHandler().handle()

        spec mandates endDocument() in 1) but that is semantically incorrect: AJAX response = XML and it's procedural representation on server is Lifecycle.render, not ViewDeclarationLanguage.renderView. With point

        4) call responseWriter.endDocument() as last statement in Lifecycle.render

        can redirect work.

        Show
        Martin Kočí added a comment - from spec 2.1: ViewDeclarationLanguage.renderView() : All implementations must: Call endDocument() on the ResponseWriter. Order of invocations in render_response Lifecycle.render: 0) before phase listeners 1) ViewDeclarationLanguage.renderView() (calls responseWriter.endDocument()) 2) after phase listeners 3) facesContext.getExceptionHandler().handle() spec mandates endDocument() in 1) but that is semantically incorrect: AJAX response = XML and it's procedural representation on server is Lifecycle.render, not ViewDeclarationLanguage.renderView. With point 4) call responseWriter.endDocument() as last statement in Lifecycle.render can redirect work.
        Hide
        Thomas Andraschko added a comment -

        I agree.
        if a exception occurs in the render_respone phase, the user will never notice any error. A redirect via ExceptionHandler should work in ALL cases!

        If the specification does not allow this, could this be fixed via 2.2?

        Show
        Thomas Andraschko added a comment - I agree. if a exception occurs in the render_respone phase, the user will never notice any error. A redirect via ExceptionHandler should work in ALL cases! If the specification does not allow this, could this be fixed via 2.2?
        Hide
        Martin Kočí added a comment -

        a user found it too: http://www.mail-archive.com/users@myfaces.apache.org/msg58387.html

        this problem is bigger, because user can have buggy render_respose phase for following reasons:
        1) getters with unexpected exceptions like NPE
        2) lazy loading errors
        3) buggy f:event type="preRender" listener - with JSF 2. 0 it is possible to register listeners for render response phase
        4) ...

        Implementation:
        myfaces close XML document in PartialViewContextImpl after (successful or unsuccessful) rendering.

        But in case of a exception is necessary to add other elements in outputed XML:
        1) <error> elements from AjaxExceptionHandlerImpl
        2) <redirect> from org.apache.myfaces.context.servlet.ServletExternalContextImpl.redirect(String)
        3) custom elemetns from custom exception handler

        I think this is doable in code, but what specification says?

        Show
        Martin Kočí added a comment - a user found it too: http://www.mail-archive.com/users@myfaces.apache.org/msg58387.html this problem is bigger, because user can have buggy render_respose phase for following reasons: 1) getters with unexpected exceptions like NPE 2) lazy loading errors 3) buggy f:event type="preRender" listener - with JSF 2. 0 it is possible to register listeners for render response phase 4) ... Implementation: myfaces close XML document in PartialViewContextImpl after (successful or unsuccessful) rendering. But in case of a exception is necessary to add other elements in outputed XML: 1) <error> elements from AjaxExceptionHandlerImpl 2) <redirect> from org.apache.myfaces.context.servlet.ServletExternalContextImpl.redirect(String) 3) custom elemetns from custom exception handler I think this is doable in code, but what specification says?

          People

          • Assignee:
            Unassigned
            Reporter:
            Martin Kočí
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development