adamsaghy commented on code in PR #3471:
URL: https://github.com/apache/fineract/pull/3471#discussion_r1336813058


##########
fineract-core/src/main/java/org/apache/fineract/batch/exception/ErrorHandler.java:
##########
@@ -71,24 +88,68 @@ private <T extends RuntimeException> ExceptionMapper<T> 
findMostSpecificExceptio
         return (ExceptionMapper<T>) defaultExceptionMapper;
     }
 
-    private <T> Set<T> createSet(T[] array) {
-        if (array == null) {
-            return Set.of();
-        } else {
-            return Set.of(array);
-        }
-    }
-
     /**
      * Returns an object of ErrorInfo type containing the information 
regarding the raised error.
      *
      * @param exception
      * @return ErrorInfo
      */
-    public ErrorInfo handle(final RuntimeException exception) {
+    public ErrorInfo handle(@NotNull RuntimeException exception) {
         ExceptionMapper<RuntimeException> exceptionMapper = 
findMostSpecificExceptionHandler(exception);
         Response response = exceptionMapper.toResponse(exception);
+        MultivaluedMap<String, Object> headers = response.getHeaders();
+        Set<Header> batchHeaders = headers == null ? null
+                : headers.keySet().stream().map(e -> new Header(e, 
response.getHeaderString(e))).collect(Collectors.toSet());
         return new ErrorInfo(response.getStatus(), ((FineractExceptionMapper) 
exceptionMapper).errorCode(),
-                JSON_HELPER.toJson(response.getEntity()));
+                JSON_HELPER.toJson(response.getEntity()), batchHeaders);
+    }
+
+    public RuntimeException getMappable(@NotNull Throwable thr) {
+        return getMappable(thr, null, null, null);
+    }
+
+    public RuntimeException getMappable(@NotNull Throwable t, String msgCode, 
String defaultMsg, String param,

Review Comment:
   The `findMostSpecificExceptionHandler` was introduced to dynamically couple 
exception handlers to exceptions and move away from the "hardcoded" exception 
handling. I reckon here it is not coupling which exception to be handled by 
which exception handler, but rather hardcoding which exception to be translated 
into a "new" exception. I was wondering rather having this `getMappable` 
method, it would be better to simply map the proper exception handler for this 
type of exceptions?
   
   That way we dont have hardcoded logic and easier to work with / overwrite 
them (if needed).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to