Catch LinkageError in ThrowableExtendedStackTraceRenderer#loadClass (#4028)

[chanani]

Summary

The loadClass method in ThrowableExtendedStackTraceRenderer only catches Exception,
but ClassLoader can throw NoClassDefFoundError, which extends LinkageError (an Error
subclass). This causes the entire log event to fail with AppenderLoggingException instead
of gracefully degrading when a class in the stack trace cannot be resolved.

Changes

• ThrowableExtendedStackTraceRenderer.java: Changed catch (Exception) to
  catch (Exception | LinkageError) in the loadClass method
• ThrowableExtendedStackTraceRendererTest.java: Added two test cases:
  • Verifies rendering succeeds when a stack trace references a non-existent class
  • Verifies rendering succeeds when a custom ClassLoader throws NoClassDefFoundError
• Changelog entry: Added 4028_fix_catch_LinkageError_in_ThrowableExtendedStackTraceRenderer.xml

Root Cause

// Before: Only catches Exception — NoClassDefFoundError (Error) escapes
catch (final Exception ignored)

// After: Also catches LinkageError, which is the parent of NoClassDefFoundError
catch (final Exception | LinkageError ignored)

Java's Throwable hierarchy:

Throwable
├── Exception        ← previously caught
└── Error
       └── LinkageError ← now also caught
                  └── NoClassDefFoundError

Fixes #4028

[chanani]

Hi @vy, thanks for the encouragement!

Here's what this PR includes:

• Fix: Changed catch (Exception) to catch (Exception | LinkageError) in ThrowableExtendedStackTraceRenderer#loadClass
• Tests: Two test cases covering NoClassDefFoundError scenarios
• Changelog: Added entry for this fix

Let me know if any changes are needed!

[vy]

@chanani, thanks so much for the (AI-generated?) fix. Please make sure ./mvnw -pl :log4j-core install && ./mvnw -pl :log4j-core-test verify succeeds when wrapping up your changes.

[vy]

Suggested change

	<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>

[chanani]

Hi @vy, thank you for the review! I've addressed all the feedback.

1. catch (Throwable) — Applied the suggested change to catch all Throwables instead of Exception | LinkageError
2. Changelog description — Updated to "Ensure all Throwables are handled while rendering stack traces"
3. Changelog issue link — Changed from #4028 to #4049
4. Tests moved — Removed ThrowableExtendedStackTraceRendererTest and added the test cases into ThrowablePatternConverterTest.StackTraceTest as requested

Please let me know if any further changes are needed!

[vy]

@chanani, TestFriendlyException is already containing a stack trace element of a non-existent class: ORG_APACHE_REPLACEMENT_STACK_TRACE_ELEMENT. Any ideas on why does this NoClassDefFoundError not surface with the existing test suite?

[chanani]

Hi @vy,

The reason NoClassDefFoundError doesn't surface with the existing test suite is the difference between two exception paths:

1. ClassNotFoundException (extends Exception) — Thrown by ClassLoader.loadClass() when a class simply doesn't exist on the classpath. That's the case for bar.OrgApacheReplacement — it was never on the classpath, so the class loader search fails cleanly with ClassNotFoundException, which was already caught by the previous catch (Exception)

2. NoClassDefFoundError (extends LinkageError → Error) — Thrown when the class is found but fails during linking/initialization (e.g., a missing dependency at link time). This is the scenario from issue ThrowableExtendedStackTraceRenderer catches Exception but not Error #4028, and it wasn't caught by catch (Exception)

So the existing test with ORG_APACHE_REPLACEMENT_STACK_TRACE_ELEMENT exercises the ClassNotFoundException path, not the NoClassDefFoundError path

Please let me know if any further changes are needed !

[vy]

I've put some more thought into this issue, and this particular test feels like an after-thought. Would you mind removing this particular test method, extending TestFriendlyException with this patch to contain a stack trace element of a non-existent class, and adapt ThrowablePatternConverterTest and its subclasses to make sure they pass, please?

[ppkarwasz]

Catching Throwable might be excessive, let's limit ourselves to LinkageError.

Suggested change

	} catch (final Throwable ignored) {
	} catch (final Exception | LinkageError ignored) {

[ppkarwasz]

Sorry, I just realized this was @vy's suggestion (#4049 (comment)), so this merits a longer discussion.

I think that catching Throwable should always be used sparingly. We don't want to catch critical exception, such as anOutOfMemoryError, especially since we ignore the exception.

[vy]

Catching Throwable might be excessive

This is precisely what we do in util.Loader, and I'm inclined to keep similar functionalities aligned. If you think they need to change, maybe we should carry that out in a separate PR focusing on that change.