Skip to content

Commit 37d0a87

Browse files
NaorayNaoray
authored
Merge pull request #14 from Naoray/feat/improve-stack-trace-formatting
Feat/improve stack trace formatting
2 parents 6883948 + 5363e11 commit 37d0a87

File tree

8 files changed

+253
-19
lines changed

8 files changed

+253
-19
lines changed

‎resources/views/comment.md

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
## Error Details
21
**Type:** {level}
32
**Message:** {message}
43

‎resources/views/issue.md

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
## Error Details
21
**Log Level:** {level}
32
**Class:** {class}
43
**Message:** {message}

‎src/Issues/Formatters/ExceptionFormatter.php

+71-7
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,29 @@ public function __construct(
1818

1919
public function format(LogRecord $record): array
2020
{
21-
$exception = $record->context['exception'] ?? null;
22-
if (! $exception instanceof Throwable) {
21+
$exceptionData = $record->context['exception'] ?? null;
22+
23+
// Handle case where the exception is stored as a string instead of a Throwable object
24+
if (is_string($exceptionData) &&
25+
(str_contains($exceptionData, 'Stack trace:') || preg_match('/#\d+ \//', $exceptionData))) {
26+
27+
return $this->formatExceptionString($exceptionData);
28+
}
29+
30+
// Original code for Throwable objects
31+
if (! $exceptionData instanceof Throwable) {
2332
return [];
2433
}
2534

26-
$header = $this->formatHeader($exception);
27-
$stackTrace = $exception->getTraceAsString();
35+
$message = $this->formatMessage($exceptionData->getMessage());
36+
$stackTrace = $exceptionData->getTraceAsString();
37+
38+
$header = $this->formatHeader($exceptionData);
2839

2940
return [
30-
'message' => $exception->getMessage(),
31-
'simplified_stack_trace' => $header."\n[stacktrace]\n".$this->stackTraceFormatter->format($stackTrace),
32-
'full_stack_trace' => $header."\n[stacktrace]\n".$stackTrace,
41+
'message' => $message,
42+
'simplified_stack_trace' => $header."\n[stacktrace]\n".$this->stackTraceFormatter->format($stackTrace, true),
43+
'full_stack_trace' => $header."\n[stacktrace]\n".$this->stackTraceFormatter->format($stackTrace, false),
3344
];
3445
}
3546

@@ -52,6 +63,15 @@ public function formatTitle(Throwable $exception, string $level): string
5263
->toString();
5364
}
5465

66+
private function formatMessage(string $message): string
67+
{
68+
if (! str_contains($message, 'Stack trace:')) {
69+
return $message;
70+
}
71+
72+
return (string) preg_replace('/\s+in\s+\/[^\s]+\.php:\d+.*$/s', '', $message);
73+
}
74+
5575
private function formatHeader(Throwable $exception): string
5676
{
5777
return sprintf(
@@ -63,4 +83,48 @@ private function formatHeader(Throwable $exception): string
6383
$exception->getLine()
6484
);
6585
}
86+
87+
/**
88+
* Format an exception stored as a string.
89+
*/
90+
private function formatExceptionString(string $exceptionString): array
91+
{
92+
$message = $exceptionString;
93+
$stackTrace = '';
94+
95+
// Try to extract the message and stack trace
96+
if (preg_match('/^(.*?)(?:Stack trace:|#\d+ \/)/', $exceptionString, $matches)) {
97+
$message = trim($matches[1]);
98+
99+
// Remove file/line info if present
100+
if (preg_match('/^(.*) in \/[^\s]+(?:\.php)? on line \d+$/s', $message, $fileMatches)) {
101+
$message = trim($fileMatches[1]);
102+
}
103+
104+
// Extract stack trace
105+
$traceStart = strpos($exceptionString, 'Stack trace:');
106+
if ($traceStart === false) {
107+
// Find the first occurrence of a stack frame pattern
108+
if (preg_match('/#\d+ \//', $exceptionString, $matches, PREG_OFFSET_CAPTURE)) {
109+
$traceStart = $matches[0][1];
110+
}
111+
}
112+
113+
if ($traceStart !== false) {
114+
$stackTrace = substr($exceptionString, $traceStart);
115+
}
116+
}
117+
118+
$header = sprintf(
119+
'[%s] Exception: %s at unknown:0',
120+
now()->format('Y-m-d H:i:s'),
121+
$message
122+
);
123+
124+
return [
125+
'message' => $this->formatMessage($message),
126+
'simplified_stack_trace' => $header."\n[stacktrace]\n".$this->stackTraceFormatter->format($stackTrace, true),
127+
'full_stack_trace' => $header."\n[stacktrace]\n".$this->stackTraceFormatter->format($stackTrace, false),
128+
];
129+
}
66130
}

‎src/Issues/Formatters/StackTraceFormatter.php

+15-7
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ class StackTraceFormatter
99
{
1010
private const VENDOR_FRAME_PLACEHOLDER = '[Vendor frames]';
1111

12-
public function format(string $stackTrace): string
12+
public function format(string $stackTrace, bool $collapseVendorFrames = true): string
1313
{
1414
return collect(explode("\n", $stackTrace))
1515
->filter(fn ($line) => ! empty(trim($line)))
16-
->map(function ($line) {
16+
->map(function ($line) use ($collapseVendorFrames) {
1717
if (trim($line) === '"}') {
1818
return '';
1919
}
@@ -28,20 +28,27 @@ public function format(string $stackTrace): string
2828

2929
$line = str_replace(base_path(), '', $line);
3030

31-
// Stack trace lines start with #\d. Here we pad the numbers 0-9
32-
// with a preceding zero to keep everything in line visually.
33-
$line = preg_replace('/^(\e\[0m)#(\d)(?!\d)/', '$1#0$2', $line);
31+
$line = $this->padStackTraceLine($line);
3432

35-
if ($this->isVendorFrame($line)) {
33+
if ($collapseVendorFrames && $this->isVendorFrame($line)) {
3634
return self::VENDOR_FRAME_PLACEHOLDER;
3735
}
3836

3937
return $line;
4038
})
41-
->pipe($this->collapseVendorFrames(...))
39+
->pipe(fn ($lines) => $collapseVendorFrames ? $this->collapseVendorFrames($lines) : $lines)
4240
->join("\n");
4341
}
4442

43+
/**
44+
* Stack trace lines start with #\d. Here we pad the numbers 0-9
45+
* with a preceding zero to keep everything in line visually.
46+
*/
47+
public function padStackTraceLine(string $line): string
48+
{
49+
return (string) preg_replace('/^#(\d)(?!\d)/', '#0$1', $line);
50+
}
51+
4552
private function formatInitialException(string $line): array
4653
{
4754
[$message, $exception] = explode('{"exception":"[object] ', $line);
@@ -56,6 +63,7 @@ private function isVendorFrame($line): bool
5663
{
5764
return str_contains((string) $line, self::VENDOR_FRAME_PLACEHOLDER)
5865
|| str_contains((string) $line, '/vendor/') && ! Str::isMatch("/BoundMethod\.php\([0-9]+\): App/", $line)
66+
|| str_contains((string) $line, '/artisan')
5967
|| str_ends_with($line, '{main}');
6068
}
6169

‎tests/Issues/Formatters/ExceptionFormatterTest.php

+54-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22

33
use Naoray\LaravelGithubMonolog\Issues\Formatters\ExceptionFormatter;
4+
use Naoray\LaravelGithubMonolog\Issues\Formatters\StackTraceFormatter;
45

56
beforeEach(function () {
67
$this->formatter = resolve(ExceptionFormatter::class);
@@ -12,12 +13,17 @@
1213

1314
$result = $this->formatter->format($record);
1415

16+
// Pad the stack trace lines to not mess up the test assertions
17+
$exceptionTrace = collect(explode("\n", $exception->getTraceAsString()))
18+
->map(fn ($line) => (new StackTraceFormatter)->padStackTraceLine($line))
19+
->join("\n");
20+
1521
expect($result)
1622
->toBeArray()
1723
->toHaveKeys(['message', 'simplified_stack_trace', 'full_stack_trace'])
1824
->and($result['message'])->toBe('Test exception')
1925
->and($result['simplified_stack_trace'])->toContain('[Vendor frames]')
20-
->and($result['full_stack_trace'])->toContain($exception->getTraceAsString());
26+
->and($result['full_stack_trace'])->toContain($exceptionTrace);
2127
});
2228

2329
test('it returns empty array for non-exception records', function () {
@@ -50,3 +56,50 @@
5056
->toContain('RuntimeException')
5157
->toContain('...');
5258
});
59+
60+
test('it properly formats exception with stack trace in message', function () {
61+
// Create a custom exception class that mimics our problematic behavior
62+
$exception = new class('Error message') extends Exception
63+
{
64+
public function __construct()
65+
{
66+
parent::__construct('The calculation amount [123.45] does not match the expected total [456.78]. in /path/to/app/Calculations/Calculator.php:49
67+
Stack trace:
68+
#0 /path/to/app/Services/PaymentService.php(83): App\\Calculations\\Calculator->calculate()
69+
#1 /vendor/framework/src/Services/TransactionService.php(247): App\\Services\\PaymentService->process()');
70+
}
71+
};
72+
73+
$record = createLogRecord('Test message', exception: $exception);
74+
75+
$result = $this->formatter->format($record);
76+
77+
// The formatter should extract just the actual error message without the file path or stack trace
78+
expect($result)
79+
->toBeArray()
80+
->toHaveKeys(['message', 'simplified_stack_trace', 'full_stack_trace'])
81+
->and($result['message'])->toBe('The calculation amount [123.45] does not match the expected total [456.78].')
82+
->and($result['simplified_stack_trace'])->toContain('[stacktrace]')
83+
->and($result['simplified_stack_trace'])->toContain('App\\Calculations\\Calculator->calculate()');
84+
});
85+
86+
test('it handles exceptions with string in context', function () {
87+
// Create a generic exception string
88+
$exceptionString = 'The calculation amount [123.45] does not match the expected total [456.78]. in /path/to/app/Calculations/Calculator.php:49
89+
Stack trace:
90+
#0 /path/to/app/Services/PaymentService.php(83): App\\Calculations\\Calculator->calculate()
91+
#1 /vendor/framework/src/Services/TransactionService.php(247): App\\Services\\PaymentService->process()';
92+
93+
// Create a record with a string in the exception context
94+
$record = createLogRecord('Test message', ['exception' => $exceptionString]);
95+
96+
$result = $this->formatter->format($record);
97+
98+
// Should extract just the clean message
99+
expect($result)
100+
->toBeArray()
101+
->toHaveKeys(['message', 'simplified_stack_trace', 'full_stack_trace'])
102+
->and($result['message'])->toBe('The calculation amount [123.45] does not match the expected total [456.78].')
103+
->and($result['simplified_stack_trace'])->toContain('[stacktrace]')
104+
->and($result['simplified_stack_trace'])->toContain('App\\Calculations\\Calculator->calculate()');
105+
});

‎tests/Issues/Formatters/StackTraceFormatterTest.php

+109
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
namespace Naoray\LaravelGithubMonolog\Tests\Issues\Formatters;
4+
35
use Naoray\LaravelGithubMonolog\Issues\Formatters\StackTraceFormatter;
46

57
beforeEach(function () {
@@ -54,3 +56,110 @@
5456
->toContain('/app/Services/TestService.php')
5557
->not->toContain('[Vendor frames]');
5658
});
59+
60+
test('it replaces base path in stack traces', function () {
61+
$formatter = new StackTraceFormatter;
62+
$basePath = base_path();
63+
64+
$stackTrace = <<<TRACE
65+
#0 {$basePath}/app/Services/ImageService.php(25): Spatie\\Image\\Image->loadFile()
66+
#1 {$basePath}/vendor/laravel/framework/src/Foundation/Application.php(1235): App\\Services\\ImageService->process()
67+
#2 {$basePath}/artisan(13): Illuminate\\Foundation\\Application->run()
68+
TRACE;
69+
70+
// Test simplified trace (with vendor frame collapsing)
71+
expect($formatter->format($stackTrace, true))
72+
->toBe(<<<'TRACE'
73+
#00 /app/Services/ImageService.php(25): Spatie\Image\Image->loadFile()
74+
[Vendor frames]
75+
TRACE
76+
);
77+
78+
// Test full trace (without vendor frame collapsing)
79+
expect($formatter->format($stackTrace, false))
80+
->toBe(<<<'TRACE'
81+
#00 /app/Services/ImageService.php(25): Spatie\Image\Image->loadFile()
82+
#01 /vendor/laravel/framework/src/Foundation/Application.php(1235): App\Services\ImageService->process()
83+
#02 /artisan(13): Illuminate\Foundation\Application->run()
84+
TRACE
85+
);
86+
});
87+
88+
test('it handles empty base path correctly', function () {
89+
$formatter = new StackTraceFormatter;
90+
91+
$stackTrace = <<<'TRACE'
92+
#0 /absolute/path/to/app/Services/ImageService.php(25): someMethod()
93+
#1 /vendor/package/src/File.php(10): otherMethod()
94+
TRACE;
95+
96+
$result = $formatter->format($stackTrace, true);
97+
98+
expect($result)
99+
->toBe(<<<'TRACE'
100+
#00 /absolute/path/to/app/Services/ImageService.php(25): someMethod()
101+
[Vendor frames]
102+
TRACE
103+
);
104+
});
105+
106+
test('it preserves non-stack-trace lines', function () {
107+
$formatter = new StackTraceFormatter;
108+
109+
$stackTrace = <<<'TRACE'
110+
[2024-03-21 12:00:00] Error: Something went wrong
111+
#0 /app/Services/Service.php(25): someMethod()
112+
#1 /vendor/package/src/File.php(10): otherMethod()
113+
TRACE;
114+
115+
$result = $formatter->format($stackTrace, true);
116+
117+
expect($result)
118+
->toBe(<<<'TRACE'
119+
[2024-03-21 12:00:00] Error: Something went wrong
120+
#00 /app/Services/Service.php(25): someMethod()
121+
[Vendor frames]
122+
TRACE
123+
);
124+
});
125+
126+
test('it recognizes artisan lines as vendor frames', function () {
127+
$formatter = new StackTraceFormatter;
128+
$basePath = base_path();
129+
130+
$stackTrace = <<<TRACE
131+
#0 {$basePath}/app/Console/Commands/ImportCommand.php(25): handle()
132+
#1 {$basePath}/artisan(13): Illuminate\\Foundation\\Application->run()
133+
#2 {$basePath}/artisan(37): require()
134+
TRACE;
135+
136+
// Test simplified trace (with vendor frame collapsing)
137+
expect($formatter->format($stackTrace, true))
138+
->toBe(<<<'TRACE'
139+
#00 /app/Console/Commands/ImportCommand.php(25): handle()
140+
[Vendor frames]
141+
TRACE
142+
);
143+
144+
// Test that artisan lines are preserved in full trace
145+
expect($formatter->format($stackTrace, false))
146+
->toBe(<<<'TRACE'
147+
#00 /app/Console/Commands/ImportCommand.php(25): handle()
148+
#01 /artisan(13): Illuminate\Foundation\Application->run()
149+
#02 /artisan(37): require()
150+
TRACE
151+
);
152+
});
153+
154+
test('it collapses multiple artisan lines into single vendor frame', function () {
155+
$formatter = new StackTraceFormatter;
156+
157+
$stackTrace = <<<'TRACE'
158+
#0 /artisan(13): Illuminate\Foundation\Application->run()
159+
#1 /vendor/laravel/framework/src/Foundation/Application.php(1235): handle()
160+
#2 /artisan(37): require()
161+
TRACE;
162+
163+
expect($formatter->format($stackTrace, true))
164+
->toBe('[Vendor frames]');
165+
});

‎tests/Issues/TemplateRendererTest.php

-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@
9191
);
9292

9393
expect($rendered)
94-
->toContain('## Error Details')
9594
->toContain('**Type:** ERROR')
9695
->toContain('<!-- Signature: test -->');
9796
});

‎tests/Pest.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php
22

3+
use Illuminate\Support\Arr;
34
use Monolog\Level;
45
use Monolog\LogRecord;
56
use Naoray\LaravelGithubMonolog\Tests\TestCase;
@@ -14,12 +15,14 @@ function createLogRecord(
1415
?Throwable $exception = null,
1516
?string $signature = null,
1617
): LogRecord {
18+
$context = Arr::has($context, 'exception') ? $context : array_merge($context, ['exception' => $exception]);
19+
1720
return new LogRecord(
1821
datetime: new \DateTimeImmutable,
1922
channel: 'test',
2023
level: $level,
2124
message: $message,
22-
context: array_merge($context, ['exception' => $exception]),
25+
context: $context,
2326
extra: array_merge($extra, ['github_issue_signature' => $signature]),
2427
);
2528
}

0 commit comments

Comments
 (0)