OFBiz
  1. OFBiz
  2. OFBIZ-1843

Suppress stack traces on front end due to errors in ftl files

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: ALL COMPONENTS
    • Labels:
      None

      Description

      We need to find a way to gracefully handle stack traces that appear within freemarker on front end due to errors in ftl files. Instead of displaying stack traces to the end user, we should have a simple, configurable way of controlling levels of error messages that should be displayed.

      1. OFBIZ-1843.patch
        7 kB
        Amit Shinde
      2. OFBIZ-1843.patch
        7 kB
        Ray Barlow

        Activity

        Hide
        Amit Shinde added a comment - - edited

        Attached patch for this enhancement.

        Description -

        I have created properties to control levels of display of exceptions on frontend due to ftl errors. The properties are in debug.properties -

        1. Catch Freemarker Exceptions
          #freemarker.exception.display.level property values can be IGNORE_ALL or IGNORE_OPTIMAL or IGNORE_NONE
          freemarker.exception.display.level=IGNORE_NONE

        #set 'freemarker.exception.suppress' property to Y only if we want to suppress exceptions containing specific words specified in #'freemarker.exception.contains' property
        freemarker.exception.suppress=N

        #'freemarker.exception.contains' property will suppress errors containing specific words in exceptions like com.yourcompanyname ONLY when #'freemarker.exception.suppress' is set to Y
        freemarker.exception.contains=com.yourcompanyname

        #'freemarker.exception.message' property is the replacement text we use to display the error stack trace ONLY when 'freemarker.exception.suppress' is #set to Y
        freemarker.exception.message=Error occurred on rendering

        'freemarker.exception.display.level' Property - This property controls the level of exception we want to show on frontend. Its values are IGNORE_ALL or IGNORE_OPTIMAL or IGNORE_NONE. The exceptions will only be written to logs in all cases.

        • IGNORE_NONE - When this value is set, whole stack trace will be displayed. This what we see right now by default. Snapshot - http://screencast.com/t/mn4urtKk7Y
        • IGNORE_OPTIMAL - When this value is set, only one line of error is displayed and the remaining page (section after error) is rendered as well. This is the best level for us and I have set it in debug.properties as well. Snapshot - http://screencast.com/t/lksaILn5Y
        • IGNORE_ALL - This value takes the highest precedence and ignores ALL exceptions and nothing will be displayed on frontend at all. Snapshot - http://screencast.com/t/uunS2fxRD

        Custom Exception Suppression - This is used ONLY when we want to suppress a certain exception that has specific string in it and replace that stack with a custom error message. This functionality will work only when 'freemarker.exception.display.level' is IGNORE_OPTIMAL or IGNORE_NONE. This requires all the remaining 3 properties -

        • freemarker.exception.suppress=Y (Enable suppression)
        • freemarker.exception.contains=com.yourcompanyname (if the exception contains "com.yourcompanyname" string, replace the stack with custom message. We can add more strings separated by pipes ('|').)
        • freemarker.exception.message=Error occurred on rendering (if suppression is enabled and stack contains all of the above strings, then replace stack with this message)
        Show
        Amit Shinde added a comment - - edited Attached patch for this enhancement. Description - I have created properties to control levels of display of exceptions on frontend due to ftl errors. The properties are in debug.properties - Catch Freemarker Exceptions #freemarker.exception.display.level property values can be IGNORE_ALL or IGNORE_OPTIMAL or IGNORE_NONE freemarker.exception.display.level=IGNORE_NONE #set 'freemarker.exception.suppress' property to Y only if we want to suppress exceptions containing specific words specified in #'freemarker.exception.contains' property freemarker.exception.suppress=N #'freemarker.exception.contains' property will suppress errors containing specific words in exceptions like com.yourcompanyname ONLY when #'freemarker.exception.suppress' is set to Y freemarker.exception.contains=com.yourcompanyname #'freemarker.exception.message' property is the replacement text we use to display the error stack trace ONLY when 'freemarker.exception.suppress' is #set to Y freemarker.exception.message=Error occurred on rendering 'freemarker.exception.display.level' Property - This property controls the level of exception we want to show on frontend. Its values are IGNORE_ALL or IGNORE_OPTIMAL or IGNORE_NONE. The exceptions will only be written to logs in all cases. IGNORE_NONE - When this value is set, whole stack trace will be displayed. This what we see right now by default. Snapshot - http://screencast.com/t/mn4urtKk7Y IGNORE_OPTIMAL - When this value is set, only one line of error is displayed and the remaining page (section after error) is rendered as well. This is the best level for us and I have set it in debug.properties as well. Snapshot - http://screencast.com/t/lksaILn5Y IGNORE_ALL - This value takes the highest precedence and ignores ALL exceptions and nothing will be displayed on frontend at all. Snapshot - http://screencast.com/t/uunS2fxRD Custom Exception Suppression - This is used ONLY when we want to suppress a certain exception that has specific string in it and replace that stack with a custom error message. This functionality will work only when 'freemarker.exception.display.level' is IGNORE_OPTIMAL or IGNORE_NONE. This requires all the remaining 3 properties - freemarker.exception.suppress=Y (Enable suppression) freemarker.exception.contains=com.yourcompanyname (if the exception contains "com.yourcompanyname" string, replace the stack with custom message. We can add more strings separated by pipes ('|').) freemarker.exception.message=Error occurred on rendering (if suppression is enabled and stack contains all of the above strings, then replace stack with this message) Snapshots - o When suppression is disabled and 'freemarker.exception.display.level' is IGNORE_NONE - http://screencast.com/t/ZfFNbxXKs o When suppression is enabled and 'freemarker.exception.display.level' is IGNORE_NONE or IGNORE_OPTIMAL - http://screencast.com/t/eyqM50IzCW
        Hide
        Jacques Le Roux added a comment -

        This is interesting, I hope to find some time to check it out. No need to say that if somebody beats me on it I will not complain

        Show
        Jacques Le Roux added a comment - This is interesting, I hope to find some time to check it out. No need to say that if somebody beats me on it I will not complain
        Hide
        Ray Barlow added a comment -

        Updated patch to latest trunk version due to changes in the FreeMarkerWorker that caused conflicts, fixed to relative paths, also did a little tidy of source in places.

        Testing wise it worked fine.

        Show
        Ray Barlow added a comment - Updated patch to latest trunk version due to changes in the FreeMarkerWorker that caused conflicts, fixed to relative paths, also did a little tidy of source in places. Testing wise it worked fine.
        Hide
        Adrian Crum added a comment -

        This looks good.

        One problem though - the patch assumes FreeMarker is being used for the UI only. FreeMarker is used in other places and for other purposes as well. It would be preferable to have the exception handling controlled by a setting in the Environment object - then we could have the exception controlled on a case-by-case basis. Maybe instead of using global settings in the debug.properties files, we could have those settings in a properties file in the framework/widget/config folder that is loaded into the screen rendering context.

        Show
        Adrian Crum added a comment - This looks good. One problem though - the patch assumes FreeMarker is being used for the UI only. FreeMarker is used in other places and for other purposes as well. It would be preferable to have the exception handling controlled by a setting in the Environment object - then we could have the exception controlled on a case-by-case basis. Maybe instead of using global settings in the debug.properties files, we could have those settings in a properties file in the framework/widget/config folder that is loaded into the screen rendering context.
        Hide
        Jacques Le Roux added a comment -

        Before this patch is too old, any takers ?

        Show
        Jacques Le Roux added a comment - Before this patch is too old, any takers ?
        Hide
        Amit Shinde added a comment -

        I am not clear here. "FreeMarker is used in other places and for other purposes as well." but the exceptions thrown will be the same kind. Can you provide an example where this is not required.

        I will make the required changes once clarified.

        Show
        Amit Shinde added a comment - I am not clear here. "FreeMarker is used in other places and for other purposes as well." but the exceptions thrown will be the same kind. Can you provide an example where this is not required. I will make the required changes once clarified.
        Hide
        Adrian Crum added a comment -

        The purpose of this issue is to modify how exceptions are displayed to the end user (in the UI I'm assuming). The patch modifies the behavior of ALL FreeMarker exceptions, not just the ones involving the UI.

        FreeMarker is also used to print reports and import data (via WebTools).

        So, let's say you've configured your installation to display little or no information when a FreeMarker exception occurs. Then you try to print a report or import some data and a FreeMarker exception occurs. With the current patch, you have no way of knowing what went wrong (since you turned off the display of exceptions). You would have to change the properties settings, clear the cache, and try again.

        That's why I suggested putting the exception handling properties in the context. The screen rendering context would have one set of properties, and report generation or XML import services contexts would have a different set of properties.

        Maybe I'm being too picky. It just seems to me that the current patch might cause confusion down the road.

        Show
        Adrian Crum added a comment - The purpose of this issue is to modify how exceptions are displayed to the end user (in the UI I'm assuming). The patch modifies the behavior of ALL FreeMarker exceptions, not just the ones involving the UI. FreeMarker is also used to print reports and import data (via WebTools). So, let's say you've configured your installation to display little or no information when a FreeMarker exception occurs. Then you try to print a report or import some data and a FreeMarker exception occurs. With the current patch, you have no way of knowing what went wrong (since you turned off the display of exceptions). You would have to change the properties settings, clear the cache, and try again. That's why I suggested putting the exception handling properties in the context. The screen rendering context would have one set of properties, and report generation or XML import services contexts would have a different set of properties. Maybe I'm being too picky. It just seems to me that the current patch might cause confusion down the road.

          People

          • Assignee:
            Unassigned
            Reporter:
            Amit Shinde
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development