+1 for using the GenericEntityException, we did similar effort in
OFBIZ-7539.

Also agree on Jacques concern that, we should also fix the NPE for
delegator coming from HtmlFormMacroLibrary.ftl. So I think for now the fix
provided in the mentioned ticket is fine OFBIZ-9230 and we should also use
the GenericEntityException. Later we should try to figure out why and when
system lose the delegator which causes the NPE. Once we would fix the
reason we should remove the fix added in the EntityUtilProperties class
method.

New fix proposal:

Change method WidgetWorker.getDelegator with following code;

    public static Delegator getDelegator(Map<String, Object> context) {
        Delegator delegator = (Delegator) context.get("delegator");
        if (delegator == null) {
            delegator = DelegatorFactory.getDelegator(delegatorName);
        }
        return delegator;
    }


Please see if this proposal looks fine then we could try and see if it
works to fix the original issue reported in the ticket.

Thanks!


Rishi Solanki
Sr. Manager, Enterprise Software Development
HotWax Systems Pvt. Ltd.
Direct: +91-9893287847
http://www.hotwaxsystems.com

On Tue, Feb 28, 2017 at 1:49 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

> Thanks Wei,
>
> Please All, refer to OFBIZ-9230 to see how I temporarily handled the issue.
>
> I also agree that using GenericEntityException is better than just a
> permissive Exception
>
> But I believe we should rather fix the underlying issue I reported there.
> So I was hesitant to agree with Wei though it's maybe a better way to get
> to really fix the issue because it shows it at the UI level and non only in
> logs.
>
> Jacques
>
>
>
> Le 28/02/2017 à 03:35, Wei Zhang a écrit :
>
>> Hi,
>>
>> I found there is a try/catch block to catch Exception in
>> EntityUtilProperties.getSystemPropertyValue(). Which will catche a NPE
>> (delegator is null) when this method is called in
>> framework/widget/templates/HtmlFormMacroLibrary.ftl but we only get a
>> waring
>> message in the log file.
>>
>> So I think we should catch GenericEntityException rather than Exception
>> here
>> to expose NPE or other runtime exceptions.
>>
>> Thanks,
>>
>> Wei
>>
>>
>>
>> -----
>> 程序羊
>> --
>> View this message in context: http://ofbiz.135035.n4.nabble.
>> com/Should-not-catch-Exception-in-EntityUtilProperties-getSy
>> stemPropertyValue-tp4702833.html
>> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>>
>>
>>
>

Reply via email to