Catch LinkageError in ThrowableExtendedStackTraceRenderer#loadClass (#4028)#4049
Catch LinkageError in ThrowableExtendedStackTraceRenderer#loadClass (#4028)#4049chanani wants to merge 2 commits intoapache:2.xfrom
LinkageError in ThrowableExtendedStackTraceRenderer#loadClass (#4028)#4049Conversation
…s` (apache#4028) Signed-off-by: CHANHAN <130114269+chanani@users.noreply.github.com>
|
Hi @vy, thanks for the encouragement! Here's what this PR includes:
Let me know if any changes are needed! |
| https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" | ||
| type="fixed"> | ||
| <issue id="4028" link="https://github.com/apache/logging-log4j2/issues/4028"/> | ||
| <description format="asciidoc">Catch `LinkageError` in `ThrowableExtendedStackTraceRenderer#loadClass` to prevent `NoClassDefFoundError` from breaking logging</description> |
There was a problem hiding this comment.
| <description format="asciidoc">Catch `LinkageError` in `ThrowableExtendedStackTraceRenderer#loadClass` to prevent `NoClassDefFoundError` from breaking logging</description> | |
| <description format="asciidoc">Ensure all `Throwable`s are handled while rendering stack traces</description> |
| xsi:schemaLocation="https://logging.apache.org/xml/ns | ||
| https://logging.apache.org/xml/ns/log4j-changelog-0.xsd" | ||
| type="fixed"> | ||
| <issue id="4028" link="https://github.com/apache/logging-log4j2/issues/4028"/> |
There was a problem hiding this comment.
| <issue id="4028" link="https://github.com/apache/logging-log4j2/issues/4028"/> | |
| <issue id="4049" link="https://github.com/apache/logging-log4j2/pull/4049"/> |
There was a problem hiding this comment.
Instead of adding a new test file, would you mind extending ThrowablePatternConverterTest.StackTraceTest, please?
| return clazz; | ||
| } | ||
| } catch (final Exception ignored) { | ||
| } catch (final Exception | LinkageError ignored) { |
There was a problem hiding this comment.
| } catch (final Exception | LinkageError ignored) { | |
| } catch (final Throwable ignored) { |
…che#4028) - Changed catch (Exception) to catch (Throwable) in loadClass method - Moved tests into ThrowablePatternConverterTest.StackTraceTest - Removed standalone ThrowableExtendedStackTraceRendererTest - Updated changelog description and issue link per review feedback Signed-off-by: CHANHAN <130114269+chanani@users.noreply.github.com>
|
Hi @vy, thank you for the review! I've addressed all the feedback.
Please let me know if any further changes are needed! |
|
@chanani, |
|
Hi @vy, The reason
So the existing test with Please let me know if any further changes are needed ! |
Summary
The
loadClassmethod inThrowableExtendedStackTraceRendereronly catchesException,but
ClassLoadercan throwNoClassDefFoundError, which extendsLinkageError(anErrorsubclass). This causes the entire log event to fail with
AppenderLoggingExceptioninsteadof gracefully degrading when a class in the stack trace cannot be resolved.
Changes
ThrowableExtendedStackTraceRenderer.java: Changedcatch (Exception)tocatch (Exception | LinkageError)in theloadClassmethodThrowableExtendedStackTraceRendererTest.java: Added two test cases:ClassLoaderthrowsNoClassDefFoundError4028_fix_catch_LinkageError_in_ThrowableExtendedStackTraceRenderer.xmlRoot Cause
Java's
Throwablehierarchy:Fixes #4028