Skip to content

Indent overridden Throwable#printStackTrace output#25969

Open
Suvrat1629 wants to merge 1 commit intojenkinsci:masterfrom
Suvrat1629:jenkins-73770
Open

Indent overridden Throwable#printStackTrace output#25969
Suvrat1629 wants to merge 1 commit intojenkinsci:masterfrom
Suvrat1629:jenkins-73770

Conversation

@Suvrat1629
Copy link

@Suvrat1629 Suvrat1629 commented Dec 20, 2025

Fixes #16544

Summary

When a Throwable overrides printStackTrace(PrintWriter) its multi-line output was appended raw by Functions.printThrowable, which could break indentation and interleave confusing "Caused"/"Caused by" lines and produce an extra blank line. The change captures the overridden output, splits it by line, and prefixes each line with the existing prefix so custom implementations are formatted consistently. The split avoids preserving a trailing empty element to prevent an extra blank line.

Testing done

  • Built and ran the affected unit test locally:
    • mvn -pl core -am -Dtest=FunctionsTest#printThrowable test
    • FunctionsTest#printThrowable passes locally after the change.
  • Built core to ensure no regressions:
    • mvn -pl core -am clean install -DskipTests -Dspotbugs.skip=true -Dcheckstyle.skip=true

Manual validation:

  • Verified that custom Throwable#printStackTrace output is indented with the same prefix used by doPrintStackTrace and that no extra blank line is appended.

Proposed changelog entries

  • human-readable text

Proposed changelog category

/label

Proposed upgrade guidelines

N/A

Submitter checklist

  • The issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
  • There is automated testing or an explanation as to why this change has no tests.
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
  • New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
  • UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin. In particular, new or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
  • For dependency updates, there are links to external changelogs and, if possible, full differentials.
  • For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@daniel-beck

Before the changes are marked as ready-for-merge:

Maintainer checklist

  • There are at least two (2) approvals for the pull request and no outstanding requests for change.
  • Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
  • Proper changelog labels are set so that the changelog can be generated automatically.
  • If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
  • If it would make sense to backport the change to LTS, be a Bug or Improvement, and either the issue or pull request must be labeled as lts-candidate to be considered.

@Suvrat1629
Copy link
Author

/label bug

@comment-ops-bot comment-ops-bot bot added the bug For changelog: Minor bug. Will be listed after features label Dec 22, 2025
@daniel-beck
Copy link
Member

This needs to have test coverage. Also, please include before/after screenshots of the output to ensure that it's better, rather than worse, wrt. alternating Caused/Caused by lines. (Screenshots of text are usually inappropriate, but since this is about formatting more than actual content, I'd consider it preferable to text.)

@MarkEWaite
Copy link
Contributor

This needs to have a changelog entry that describes the issue in a way that a Jenkins user reading the changelog will understand what changed and how they might have seen the issue before the change. The comments in the pull request template provide more details on the phrasing of the changelog entry.

This needs to have more description of the interactive testing that was performed. What actions in Jenkins did you take in order to see the badly formatted stack trace before this change? The original issue says that it happens in rare circumstances. Describe at least one of those rare circumstances.

@daniel-beck
Copy link
Member

The original issue says that it happens in rare circumstances. Describe at least one of those rare circumstances.

Sorry about that being vague. This just refers to it being unusual for exceptions to override these methods. I removed the only common occurrence in the ecosystem that I'm aware of in jenkinsci/jelly#125. active-directory is the only original Jenkins code doing that that's left I think, and otherwise it could only come from random libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For changelog: Minor bug. Will be listed after features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[JENKINS-73770] Badly formatted Functions#printThrowable/printStackTrace in rare circumstances

3 participants