OFBiz
  1. OFBiz
  2. OFBIZ-1169

Eliminate redundant log messages in screen widgets

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: SVN trunk
    • Fix Version/s: SVN trunk
    • Component/s: framework
    • Labels:
      None

      Description

      Currently, if you have a typo or minor bug in a bsh or ftl file that is part of a screen widget definition, the resulting error message in the log and on screen can be overwhelming. This causes a serious usability issue with developers and new users.

      I decided to spend some personal time to see what the source of the log "explosion" is and how it can be fixed.

      1. screenwidget-log-fix.patch
        5 kB
        Leon Torres
      2. screen-render-exception.patch
        2 kB
        Leon Torres

        Activity

        Hide
        Leon Torres added a comment -

        This patch provides two major improvements to the readability of screen widget logs.

        1. The render screen function is recursive in nature, and each render step will log every error. Most of these errors get thrown up to the previous render function, which causes them to get logged again. This is the primary reason that the log file explodes with messages.

        To fix it, I introduced a special exception called ScreenRenderException which tells the rendering function that the particular error was already caught and logged, so it can just pass it up the chain.

        2. HtmlWidget.java was throwing a RuntimeException, which pollutes the user's screen with a lot of error information. Instead of throwing RuntimeException, just render the error message directly to the screen. This is much more useful as it prints exactly what the error was and nothing more, allowing you to fix typos in the FTL much faster.

        Show
        Leon Torres added a comment - This patch provides two major improvements to the readability of screen widget logs. 1. The render screen function is recursive in nature, and each render step will log every error. Most of these errors get thrown up to the previous render function, which causes them to get logged again. This is the primary reason that the log file explodes with messages. To fix it, I introduced a special exception called ScreenRenderException which tells the rendering function that the particular error was already caught and logged, so it can just pass it up the chain. 2. HtmlWidget.java was throwing a RuntimeException, which pollutes the user's screen with a lot of error information. Instead of throwing RuntimeException, just render the error message directly to the screen. This is much more useful as it prints exactly what the error was and nothing more, allowing you to fix typos in the FTL much faster.
        Hide
        Leon Torres added a comment -

        Oops, forgot to svn add the ScreenRenderException so I can diff it. The second patch contains the new exception file, which is simply a subclass of GeneralException.

        Show
        Leon Torres added a comment - Oops, forgot to svn add the ScreenRenderException so I can diff it. The second patch contains the new exception file, which is simply a subclass of GeneralException.
        Hide
        Si Chen added a comment -

        If there are no objections I will commit this.

        Show
        Si Chen added a comment - If there are no objections I will commit this.
        Hide
        Adrian Crum added a comment -

        I don't have time to review the patch, but I like the idea.

        Show
        Adrian Crum added a comment - I don't have time to review the patch, but I like the idea.
        Hide
        David E. Jones added a comment -

        For anything like this please allow at least a couple of days for review before committing. If it was an urgent bug fix or something that would be different, but with a little time more of the general group using OFBiz might be able to review, comment on, and help improve this.

        Show
        David E. Jones added a comment - For anything like this please allow at least a couple of days for review before committing. If it was an urgent bug fix or something that would be different, but with a little time more of the general group using OFBiz might be able to review, comment on, and help improve this.
        Hide
        Leon Torres added a comment -

        Yes, please test. It's pretty easy to: just introduce typos into the FTL and bsh files and see what shows up in the log and on screen.

        Show
        Leon Torres added a comment - Yes, please test. It's pretty easy to: just introduce typos into the FTL and bsh files and see what shows up in the log and on screen.
        Hide
        Si Chen added a comment -

        I won't commit this until Friday so anybody who has a comment or suggestion please chime in.

        Show
        Si Chen added a comment - I won't commit this until Friday so anybody who has a comment or suggestion please chime in.
        Hide
        Jacques Le Roux added a comment -

        I can't see any differences for .bsh (tried just to remove a semi-colon) but for .ftl yes it's really smarter. The code seems sound : I vote for

        Show
        Jacques Le Roux added a comment - I can't see any differences for .bsh (tried just to remove a semi-colon) but for .ftl yes it's really smarter. The code seems sound : I vote for
        Hide
        Leon Torres added a comment -

        Oh yeah, I forgot to mention that BshUtil.java is a little noisy, because it both logs the exception and throws it, causing redundant messages. I just shut it off in debug.properties:

        log4r.logger.org.ofbiz.base.util=OFF

        Try this patch with this logger setting and it should be more concise.

        Show
        Leon Torres added a comment - Oh yeah, I forgot to mention that BshUtil.java is a little noisy, because it both logs the exception and throws it, causing redundant messages. I just shut it off in debug.properties: log4r.logger.org.ofbiz.base.util=OFF Try this patch with this logger setting and it should be more concise.
        Hide
        Jacques Le Roux added a comment -

        Sorry Leon,

        I added "log4r.logger.org.ofbiz.base.util=OFF" in debug.properties. I still can't see any differences

        Show
        Jacques Le Roux added a comment - Sorry Leon, I added "log4r.logger.org.ofbiz.base.util=OFF" in debug.properties. I still can't see any differences
        Hide
        Si Chen added a comment -

        Leon,

        Generally this is a great helper and has significantly cut down on the error messages thrown. However, one problem is that the exception message no longer tells you the full location of the file. For example, I get this:

        2007-07-25 16:26:15,598 (http-0.0.0.0-8433-Processor4) [ Log4JLoggerFactory.java:96 :ERROR]
        Expression haha is undefined on line 64, column 31 in viewprofile.ftl.
        The problematic instruction:
        ----------
        ==> $

        {haha}

        [on line 64, column 29 in viewprofile.ftl]
        ----------

        Java backtrace for programmers:
        ----------
        freemarker.core.InvalidReferenceException: Expression haha is undefined on line 64, column 31 in viewprofile.ftl.

        But which viewprofile.ftl? It would be helpful if it could say component://.../viewprofile.ftl

        Show
        Si Chen added a comment - Leon, Generally this is a great helper and has significantly cut down on the error messages thrown. However, one problem is that the exception message no longer tells you the full location of the file. For example, I get this: 2007-07-25 16:26:15,598 (http-0.0.0.0-8433-Processor4) [ Log4JLoggerFactory.java:96 :ERROR] Expression haha is undefined on line 64, column 31 in viewprofile.ftl. The problematic instruction: ---------- ==> $ {haha} [on line 64, column 29 in viewprofile.ftl] ---------- Java backtrace for programmers: ---------- freemarker.core.InvalidReferenceException: Expression haha is undefined on line 64, column 31 in viewprofile.ftl. But which viewprofile.ftl? It would be helpful if it could say component://.../viewprofile.ftl
        Hide
        Si Chen added a comment -

        Oops I found it below:
        2007-07-25 16:48:17,079 (http-0.0.0.0-8433-Processor4) [ HtmlWidget.java:87 :ERROR]
        ---- exception report ----------------------------------------------------------
        Error rendering included template at location [component://crmsfa/webapp/crmsfa/contactmech/viewprofile.ftl]: freemarker.core.InvalidReferenceException: Expression haha is undefined on line 64, column 3 in viewprofile.ftl.
        Exception: freemarker.core.InvalidReferenceException
        Message: Expression haha is undefined on line 64, column 3 in viewprofile.ftl.
        ---- stack trace ---------------------------------------------------------------

        Show
        Si Chen added a comment - Oops I found it below: 2007-07-25 16:48:17,079 (http-0.0.0.0-8433-Processor4) [ HtmlWidget.java:87 :ERROR] ---- exception report ---------------------------------------------------------- Error rendering included template at location [component://crmsfa/webapp/crmsfa/contactmech/viewprofile.ftl] : freemarker.core.InvalidReferenceException: Expression haha is undefined on line 64, column 3 in viewprofile.ftl. Exception: freemarker.core.InvalidReferenceException Message: Expression haha is undefined on line 64, column 3 in viewprofile.ftl. ---- stack trace ---------------------------------------------------------------

          People

          • Assignee:
            Si Chen
            Reporter:
            Leon Torres
          • Votes:
            2 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development