Skip to content

Truncate exceptions in GitHub summary tables#5108

Merged
thomhurst merged 1 commit intomainfrom
truncate-github-summary-exceptions
Mar 9, 2026
Merged

Truncate exceptions in GitHub summary tables#5108
thomhurst merged 1 commit intomainfrom
truncate-github-summary-exceptions

Conversation

@thomhurst
Copy link
Owner

Summary

  • GitHub summary tables have limited data capacity, but previously full Exception.ToString() (type, message, and entire stack trace) was written to the details column
  • Now only the exception message and first line of the stack trace are included, keeping summaries concise while still providing useful diagnostic info

Test plan

  • Verify build passes
  • Run tests with intentional failures and check GitHub summary output contains truncated exceptions

… frame

GitHub summaries have limited data capacity. Previously, full exception
ToString() (type, message, and entire stack trace) was written to the
summary table. Now only the message and first line of the stack trace
are included, keeping summaries concise while still useful.
@thomhurst thomhurst merged commit da6644a into main Mar 9, 2026
13 of 14 checks passed
@thomhurst thomhurst deleted the truncate-github-summary-exceptions branch March 9, 2026 01:27
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The fix addresses a real problem — full with entire stack traces bloating GitHub summary tables. The implementation is clean and the intent is clear. A few observations:

Issue: Exception type name is dropped

alone loses the exception type. For example, would previously yield:

System.NullReferenceException: Object reference not set to an instance of an object.
   at SomeTest.MyTest() in ...

The new output only gives:

Object reference not set to an instance of an object.
   at SomeTest.MyTest() in ...

Without the type name, it's much harder to tell apart a NullReferenceException from an AssertionException or InvalidOperationException at a glance. A minimal fix:

var message = $"{exception.GetType().Name}: {exception.Message}";

Issue: Inner exceptions are silently dropped

For wrapped exceptions (e.g., AggregateException from async tests), the inner exception often contains the real failure message. These are completely invisible now. Consider including at least the innermost exception's type and message:

var innermost = exception;
while (innermost.InnerException != null) innermost = innermost.InnerException;

// Use innermost.Message (or both if they differ)

Or append (caused by: {inner.GetType().Name}: {inner.Message}) when an inner exception exists.

Consideration: Very long assertion messages

Some assertion libraries (like TUnit's own assertions) emit detailed diff messages that can themselves be hundreds of characters. The current change truncates the stack trace but not the message. If the goal is to prevent oversized table cells, a character cap on the full output would be more robust:

const int MaxLength = 500;
var result = $"{typeName}: {message}\n{firstStackTraceLine}";
return result.Length > MaxLength ? result[..MaxLength] + "…" : result;

Minor:

ο node/20.20.0

Use up/down arrow keys to select a version, return key to install, d to delete, q to quit copying : node/20.20.0
installed : v20.20.0 (with npm 10.8.2) vs.

The stack trace is split on
ο node/20.20.0

Use up/down arrow keys to select a version, return key to install, d to delete, q to quit copying : node/20.20.0
installed : v20.20.0 (with npm 10.8.2) which works fine on Linux/macOS and even on Windows (since strips the ). This is acceptable, but worth noting for Windows edge cases.


Summary: The PR solves the right problem and the implementation is straightforward. The main concern is losing the exception type name, which is important diagnostic context and cheap to preserve. Inner exception handling would be a bonus. Happy to see this merged with the type name fix at minimum.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The fix addresses a real problem - full Exception.ToString() with entire stack traces bloating GitHub summary tables. The implementation is clean and the intent is clear. A few observations:

Issue: Exception type name is dropped

exception.Message alone loses the exception type. Previously Exception.ToString() would yield something like:

System.NullReferenceException: Object reference not set to an instance of an object.
   at SomeTest.MyTest() in ...

The new output only gives:

Object reference not set to an instance of an object.
   at SomeTest.MyTest() in ...

Without the type name, it is much harder to distinguish a NullReferenceException from an AssertionException or InvalidOperationException at a glance. A minimal fix would be:

var message = $"{exception.GetType().Name}: {exception.Message}";

Issue: Inner exceptions are silently dropped

For wrapped exceptions (e.g. AggregateException from async tests), the inner exception often contains the real failure message. These are completely invisible with the current approach. Consider including at least the innermost exception type and message:

var innermost = exception;
while (innermost.InnerException != null) innermost = innermost.InnerException;
// use innermost.Message alongside the outer exception type

Or append something like (caused by: InnerExceptionType: message) when an inner exception exists.

Consideration: Very long assertion messages

Some assertion libraries (including TUnit's own) emit detailed diff messages that can themselves be hundreds of characters. This change truncates the stack trace but not the message body. If the goal is to prevent oversized table cells, a character cap on the combined output would be more robust:

const int MaxLength = 500;
var result = $"{typeName}: {message}\n{firstStackTraceLine}";
return result.Length > MaxLength ? result[..MaxLength] + "..." : result;

Minor: newline splitting

Splitting on \n and calling .Trim() works correctly on both Windows (\r\n) and Linux (\n). No issue here, just noting it is intentional.


Summary: The PR solves the right problem and the implementation is clean. The main concern is losing the exception type name - it is important diagnostic context and trivial to preserve. Inner exception handling would be a nice bonus. Happy to see this merged with the type name fix at minimum.

This was referenced Mar 9, 2026
This was referenced Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant