Hi,
On Mon, May 22, 2017 at 5:14 PM, Uvindra Dias Jayasinha <uvin...@wso2.com>
wrote:
>
> Noticed a few inconsistencies when it comes to how we are doing exception
> handling in C5.
>
> *1. Logging the same exception in multiple places.*
>
> The practice of logging and throwing exception has been a long time
> practice at WSO2 but this is leading to cluttering of the logs.
>
> Quoting from the following Oracle blog[1] when it comes to logging and
> throwing,
>
> *"This is one of the most annoying error-handling antipatterns. Either log
> the exception, or throw it, but never do both. Logging and throwing results
> in multiple log messages for a single problem in the code, and makes life
> hell for the support engineer who is trying to dig through the logs."*
>
> Having experienced this first hand with our products this can lead to
> unwanted confusion. Since we are working with a single APIM code base it
> does not make sense to log and throw the same error up the call tree. I
> propose we only log the exception at the Microservice layer where we
> actually handle the exception in order to do the response mapping as
> follows.
>
I remeber there were some discussions about this before, few downsides are:
1. The Microservice layer becoming the only level of logging exceptions. If
someone forgots to log a particular exception, there are no backup
exception.
2. Even if we did this perfectly, someone else uses our components in osgi
level and does not log properly.
I understand that digging through duplicated logs are bit hard. But we know
if someone forgots logging the exception (since there is only one place,
the probability is high), it gives us a even more difficult situation.
Instead of completely avoiding it, shall we at least put a debug log with
the stack trace?
>
> catch (APIManagementException e) {
> String errorMessage = "Error while retrieving API : " + apiId;
> HashMap<String, String> paramList = new HashMap<String, String>();
> paramList.put(APIMgtConstants.ExceptionsConstants.API_ID, apiId);
> ErrorDTO errorDTO = RestApiUtil.getErrorDTO(e.getErrorHandler(),
> paramList);
> log.error(errorMessage, e);
> return Response.status(e.getErrorHandler().getHttpStatusCode()).
> entity(errorDTO).build();
> }
>
>
>
> *2. Destructive Exception Wrapping is done leading to the stack trace
> being swallowed*
>
> This also seems to be happening in our code base at certain places like
> shown below
>
> catch (APIMgtDAOException e) {
> String msg = "Error occurred while retrieving labels";
> log.error(msg, e);
> throw new LabelException(msg, ExceptionCodes.LABEL_EXCEPTION);
> }
>
>
> In this case the original stack trace which is part of the
> APIMgtDAOException is lost and not propagated along with the
> LabelException. I believe this incorrect practice has led to the adoption
> of the log and throw anti pattern. Correcting the above code as follows
> will allow for the complete stack trace to be printed when logging the
> exception at the Microservice layer
>
>
> catch (APIMgtDAOException e) {
> String msg = "Error occurred while retrieving labels";
> throw new LabelException(msg, e, ExceptionCodes.LABEL_EXCEPTION);
> }
>
> +1 we must propergate the original exception.
Thanks!
Malintha
>
> With the above proposal, anyone who uses the Impl layer of our Java code
> directly(instead of via the Microservice layer) will need to log exceptions
> as part of their exception handling logic. I don't see this as a problem
> since by throwing exceptions we are mandating that exception handling code
> has to be written.
>
>
> WDYT?
>
>
> [1] https://community.oracle.com/docs/DOC-983543#antipatterns
>
>
> --
> Regards,
> Uvindra
>
> Mobile: 777733962
>
--
Malintha Amarasinghe
Software Engineer
*WSO2, Inc. - lean | enterprise | middleware*
http://wso2.com/
Mobile : +94 712383306 <+94%2071%20238%203306>
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev
