XComp commented on a change in pull request #13111: URL: https://github.com/apache/flink/pull/13111#discussion_r469865932
########## File path: flink-core/src/main/java/org/apache/flink/util/ExceptionUtils.java ########## @@ -110,51 +112,76 @@ public static boolean isJvmFatalOrOutOfMemoryError(Throwable t) { } /** - * Tries to enrich the passed exception with additional information. - * - * <p>This method improves error message for direct and metaspace {@link OutOfMemoryError}. - * It adds description of possible causes and ways of resolution. - * - * @param exception exception to enrich if not {@code null} - * @return the enriched exception or the original if no additional information could be added; - * {@code null} if the argument was {@code null} + * Tries to enrich OutOfMemoryErrors being part of the passed root Throwable's cause tree. + * + * <p>This method improves error messages for direct and metaspace {@link OutOfMemoryError}. + * It adds description about the possible causes and ways of resolution. + * + * @param root The Throwable of which the cause tree shall be traversed. + * @param jvmMetaspaceOomNewErrorMessage The message being used for JVM metaspace-related OutOfMemoryErrors. Passing + * <code>null</code> will disable handling this class of error. + * @param jvmDirectOomNewErrorMessage The message being used for direct memory-related OutOfMemoryErrors. Passing + * <code>null</code> will disable handling this class of error. + * @param jvmHeapSpaceOomNewErrorMessage The message being used for Heap space-related OutOfMemoryErrors. Passing + * <code>null</code> will disable handling this class of error. */ - @Nullable - public static Throwable tryEnrichOutOfMemoryError( - @Nullable Throwable exception, - String jvmMetaspaceOomNewErrorMessage, - String jvmDirectOomNewErrorMessage) { - boolean isOom = exception instanceof OutOfMemoryError; - if (!isOom) { - return exception; - } - - OutOfMemoryError oom = (OutOfMemoryError) exception; - if (isMetaspaceOutOfMemoryError(oom)) { - return changeOutOfMemoryErrorMessage(oom, jvmMetaspaceOomNewErrorMessage); - } else if (isDirectOutOfMemoryError(oom)) { - return changeOutOfMemoryErrorMessage(oom, jvmDirectOomNewErrorMessage); - } + public static void tryEnrichOutOfMemoryError( + @Nullable Throwable root, + @Nullable String jvmMetaspaceOomNewErrorMessage, + @Nullable String jvmDirectOomNewErrorMessage, + @Nullable String jvmHeapSpaceOomNewErrorMessage) { + updateDetailMessage(root, t -> { + if (isMetaspaceOutOfMemoryError(t)) { + return jvmMetaspaceOomNewErrorMessage; + } else if (isDirectOutOfMemoryError(t)) { + return jvmDirectOomNewErrorMessage; + } else if (isHeapSpaceOutOfMemoryError(t)) { + return jvmHeapSpaceOomNewErrorMessage; + } - return oom; + return null; + }); } /** - * Rewrites the error message of a {@link OutOfMemoryError}. + * Updates the error message of the first Throwable appearing in the cause tree of the passed root Throwable and + * matching the passed Predicate by traversing the cause tree top-to-bottom. * - * @param oom original {@link OutOfMemoryError} - * @param newMessage new error message - * @return the origianl {@link OutOfMemoryError} if it already has the new error message or - * a new {@link OutOfMemoryError} with the new error message + * @param root The Throwable whose cause tree shall be traversed. + * @param throwableToMessage The Function based on which the new messages are generated. The function implementation + * should return the new message. Returning <code>null</code>, in contrast, will result in + * not updating the message for the corresponding Throwable. */ - private static OutOfMemoryError changeOutOfMemoryErrorMessage(OutOfMemoryError oom, String newMessage) { - if (oom.getMessage().equals(newMessage)) { - return oom; + public static void updateDetailMessage(Throwable root, Function<Throwable, String> throwableToMessage) { + if (throwableToMessage == null) { + return; + } + + Throwable it = root; + while (it != null) { + String newMessage = throwableToMessage.apply(it); + if (newMessage != null) { + updateDetailMessageOfThrowable(it, newMessage); + } + + it = it.getCause(); + } + } + + private static void updateDetailMessageOfThrowable(Throwable throwable, String newDetailMessage) { + Field field; + try { + field = Throwable.class.getDeclaredField("detailMessage"); + } catch (NoSuchFieldException e) { + throw new IllegalStateException("The JDK Throwable does provide a detailMessage.", e); Review comment: I updated both exception messages to be more descriptive. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org