Skip to content

Commit b686091

Browse files
NaorayNaoray
authored
Merge pull request #27 from Naoray/fix/stack-trace-formatting-and-section-mapping
fix(issues): correct stack trace formatting and section mapping
2 parents 1810a04 + 9976fda commit b686091

File tree

9 files changed

+415
-14
lines changed

9 files changed

+415
-14
lines changed

‎resources/views/issue.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
```shell
1515
{simplified_stack_trace}
1616
```
17+
</details
1718
1819
<details>
1920
<summary>📋 View Complete Stack Trace</summary>

‎src/Deduplication/DefaultSignatureGenerator.php‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ private function generateFromException(Throwable $exception, LogRecord $record):
6868
{
6969
$frames = $this->inAppFrames($exception, $this->maxFrames);
7070

71+
// Include the first vendor frame before the first in-app frame (normalized to class name only)
72+
// This helps group errors from the same vendor class that occur through different methods
73+
$firstVendorFrame = $this->firstVendorFrameBeforeInApp($exception);
74+
7175
// If we can't determine any in-app frames, fall back to the first frame in the trace
7276
if ($frames === []) {
7377
$first = $this->firstFrame($exception);
@@ -78,6 +82,13 @@ private function generateFromException(Throwable $exception, LogRecord $record):
7882

7983
$context = $this->contextExtractor->extract($record);
8084

85+
// Build frames array: include normalized vendor frame first if found, then in-app frames
86+
$allFrames = [];
87+
if ($firstVendorFrame !== null) {
88+
$allFrames[] = $firstVendorFrame;
89+
}
90+
$allFrames = array_merge($allFrames, $frames);
91+
8192
$payload = [
8293
'v' => 2,
8394
'kind' => $context['kind'],
@@ -86,9 +97,9 @@ private function generateFromException(Throwable $exception, LogRecord $record):
8697
'ex_chain' => $this->exceptionChainSignature($exception, $this->maxExceptionChainDepth),
8798
'frames' => array_map(
8899
fn (array $frame) => $this->frameSignature($frame),
89-
$frames
100+
$allFrames
90101
),
91-
'culprit' => $frames[0] ? $this->frameSignature($frames[0]) : null,
102+
'culprit' => $allFrames[0] ? $this->frameSignature($allFrames[0]) : null,
92103
],
93104
'variant' => [
94105
'msg_tpl' => $this->messageTemplate->template($exception->getMessage()),
@@ -138,6 +149,24 @@ private function inAppFrames(Throwable $e, int $limit): array
138149
return $frames;
139150
}
140151

152+
/**
153+
* Find the first vendor frame before the first in-app frame.
154+
* This helps group errors from the same vendor class that occur through different methods.
155+
*
156+
* @return array<string, mixed>|null
157+
*/
158+
private function firstVendorFrameBeforeInApp(Throwable $e): ?array
159+
{
160+
foreach ($e->getTrace() as $frame) {
161+
if ($this->vendorFrameDetector->isVendorFrame($frame)) {
162+
// Return the first vendor frame we encounter (before any in-app frames)
163+
return $frame;
164+
}
165+
}
166+
167+
return null;
168+
}
169+
141170
/**
142171
* Reduce a frame to stable, high-signal identifiers.
143172
*
@@ -147,7 +176,17 @@ private function inAppFrames(Throwable $e, int $limit): array
147176
private function frameSignature(array $frame): array
148177
{
149178
$file = $this->normalizePath((string) ($frame['file'] ?? ''));
150-
$func = (string) (($frame['class'] ?? '').($frame['type'] ?? '').($frame['function'] ?? ''));
179+
$class = (string) ($frame['class'] ?? '');
180+
$type = (string) ($frame['type'] ?? '');
181+
$function = (string) ($frame['function'] ?? '');
182+
183+
// Normalize vendor frames to class name only (without method) to group errors
184+
// from the same vendor class that occur through different methods
185+
if ($this->vendorFrameDetector->isVendorFrame($frame) && $class !== '') {
186+
$func = $class;
187+
} else {
188+
$func = $class.$type.$function;
189+
}
151190

152191
// Intentionally avoid line numbers; they change frequently and cause under-grouping.
153192
return [

‎src/Issues/Formatters/ExceptionFormatter.php‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ private function formatExceptionString(string $exceptionString): array
9898
$stackTrace = '';
9999

100100
// Try to extract the message and stack trace
101-
if (! preg_match('/^(.*?)(?:Stack trace:|#\d+ \/)/', $exceptionString, $matches)) {
101+
if (! preg_match('/^(.*?)(?:Stack trace:|#\d+ \/)/s', $exceptionString, $matches)) {
102102
$header = sprintf(
103103
'[%s] Exception: %s at unknown:0',
104104
now()->format('Y-m-d H:i:s'),
@@ -119,19 +119,23 @@ private function formatExceptionString(string $exceptionString): array
119119
$message = trim($classMatches[1]);
120120
}
121121

122-
// Remove file/line info if present
123-
if (preg_match('/^(.*) in \/[^\s]+(?:\.php)? on line \d+$/s', $message, $fileMatches)) {
122+
// Remove file/line info if present (handles both " on line X" and ":X" formats)
123+
if (preg_match('/^(.*) in \/[^\s]+(?:\.php)?(?::\d+| on line \d+)$/s', $message, $fileMatches)) {
124124
$message = trim($fileMatches[1]);
125125
}
126126

127127
// Extract stack trace
128128
$traceStart = strpos($exceptionString, 'Stack trace:');
129-
if ($traceStart === false && preg_match('/#\d+ \//', $exceptionString, $traceMatches, PREG_OFFSET_CAPTURE)) {
130-
$traceStart = $traceMatches[0][1];
131-
}
132-
133129
if ($traceStart !== false) {
134-
$stackTrace = substr($exceptionString, $traceStart);
130+
// Find the position after "Stack trace:" and any whitespace/newlines
131+
$stackTraceStart = $traceStart + strlen('Stack trace:');
132+
$stackTrace = substr($exceptionString, $stackTraceStart);
133+
// Remove leading whitespace and newlines
134+
$stackTrace = ltrim($stackTrace, " \t\n\r\0\x0B");
135+
} elseif (preg_match('/#\d+ \//', $exceptionString, $traceMatches, PREG_OFFSET_CAPTURE)) {
136+
$stackTrace = substr($exceptionString, $traceMatches[0][1]);
137+
} else {
138+
$stackTrace = '';
135139
}
136140

137141
$header = sprintf(

‎src/Issues/SectionMapping.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class SectionMapping
99
private const SECTION_MAPPINGS = [
1010
'{simplified_stack_trace}' => 'stacktrace',
1111
'{full_stack_trace}' => 'stacktrace',
12-
'{previous_exceptions}' => 'prev-exception',
12+
'{previous_exceptions}' => 'prev-stacktrace',
1313
'{environment}' => 'environment',
1414
'{request}' => 'request',
1515
'{route}' => 'route',

‎tests/Deduplication/SignatureGeneratorTest.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,3 +398,131 @@
398398
// Different UUIDs in exception message should produce same signature after templating
399399
expect($signature2)->toBe($signature1);
400400
});
401+
402+
test('groups errors from same vendor class with different methods', function () {
403+
// Simulate errors from the same vendor class (DefaultFileRemover) but different methods
404+
// This tests that vendor frames are normalized to class name only
405+
406+
$exception1 = new \Exception('Disk [tracks] does not have a configured driver.');
407+
$reflection1 = new \ReflectionClass($exception1);
408+
$traceProperty1 = $reflection1->getProperty('trace');
409+
$traceProperty1->setAccessible(true);
410+
$fileProperty1 = $reflection1->getProperty('file');
411+
$fileProperty1->setAccessible(true);
412+
$lineProperty1 = $reflection1->getProperty('line');
413+
$lineProperty1->setAccessible(true);
414+
415+
// Set exception file and line
416+
$fileProperty1->setValue($exception1, base_path('vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemManager.php'));
417+
$lineProperty1->setValue($exception1, 138);
418+
419+
// First error: through removeFromMediaDirectory
420+
$traceProperty1->setValue($exception1, [
421+
[
422+
'file' => base_path('vendor/spatie/laravel-medialibrary/src/Support/FileRemover/DefaultFileRemover.php'),
423+
'line' => 38,
424+
'function' => 'removeFromMediaDirectory',
425+
'class' => 'Spatie\\MediaLibrary\\Support\\FileRemover\\DefaultFileRemover',
426+
'type' => '->',
427+
],
428+
[
429+
'file' => base_path('app/Nova/Track.php'),
430+
'line' => 128,
431+
'function' => 'fieldsForUpdate',
432+
'class' => 'App\\Nova\\Track',
433+
'type' => '->',
434+
],
435+
]);
436+
437+
$exception2 = new \Exception('Disk [tracks] does not have a configured driver.');
438+
$reflection2 = new \ReflectionClass($exception2);
439+
$traceProperty2 = $reflection2->getProperty('trace');
440+
$traceProperty2->setAccessible(true);
441+
$fileProperty2 = $reflection2->getProperty('file');
442+
$fileProperty2->setAccessible(true);
443+
$lineProperty2 = $reflection2->getProperty('line');
444+
$lineProperty2->setAccessible(true);
445+
446+
// Set exception file and line
447+
$fileProperty2->setValue($exception2, base_path('vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemManager.php'));
448+
$lineProperty2->setValue($exception2, 138);
449+
450+
// Second error: through removeFromConversionsDirectory
451+
$traceProperty2->setValue($exception2, [
452+
[
453+
'file' => base_path('vendor/spatie/laravel-medialibrary/src/Support/FileRemover/DefaultFileRemover.php'),
454+
'line' => 64,
455+
'function' => 'removeFromConversionsDirectory',
456+
'class' => 'Spatie\\MediaLibrary\\Support\\FileRemover\\DefaultFileRemover',
457+
'type' => '->',
458+
],
459+
[
460+
'file' => base_path('app/Nova/Track.php'),
461+
'line' => 128,
462+
'function' => 'fieldsForUpdate',
463+
'class' => 'App\\Nova\\Track',
464+
'type' => '->',
465+
],
466+
]);
467+
468+
$exception3 = new \Exception('Disk [tracks] does not have a configured driver.');
469+
$reflection3 = new \ReflectionClass($exception3);
470+
$traceProperty3 = $reflection3->getProperty('trace');
471+
$traceProperty3->setAccessible(true);
472+
$fileProperty3 = $reflection3->getProperty('file');
473+
$fileProperty3->setAccessible(true);
474+
$lineProperty3 = $reflection3->getProperty('line');
475+
$lineProperty3->setAccessible(true);
476+
477+
// Set exception file and line
478+
$fileProperty3->setValue($exception3, base_path('vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemManager.php'));
479+
$lineProperty3->setValue($exception3, 138);
480+
481+
// Third error: through removeFromResponsiveImagesDirectory
482+
$traceProperty3->setValue($exception3, [
483+
[
484+
'file' => base_path('vendor/spatie/laravel-medialibrary/src/Support/FileRemover/DefaultFileRemover.php'),
485+
'line' => 96,
486+
'function' => 'removeFromResponsiveImagesDirectory',
487+
'class' => 'Spatie\\MediaLibrary\\Support\\FileRemover\\DefaultFileRemover',
488+
'type' => '->',
489+
],
490+
[
491+
'file' => base_path('app/Nova/Track.php'),
492+
'line' => 128,
493+
'function' => 'fieldsForUpdate',
494+
'class' => 'App\\Nova\\Track',
495+
'type' => '->',
496+
],
497+
]);
498+
499+
$record1 = createLogRecord('Test', [
500+
'request' => [
501+
'route' => ['name' => 'nova.api.generated::t9axneyGDK4KlTRT'],
502+
'method' => 'PUT',
503+
],
504+
], exception: $exception1);
505+
506+
$record2 = createLogRecord('Test', [
507+
'request' => [
508+
'route' => ['name' => 'nova.api.generated::t9axneyGDK4KlTRT'],
509+
'method' => 'PUT',
510+
],
511+
], exception: $exception2);
512+
513+
$record3 = createLogRecord('Test', [
514+
'request' => [
515+
'route' => ['name' => 'nova.api.generated::t9axneyGDK4KlTRT'],
516+
'method' => 'PUT',
517+
],
518+
], exception: $exception3);
519+
520+
$signature1 = $this->generator->generate($record1);
521+
$signature2 = $this->generator->generate($record2);
522+
$signature3 = $this->generator->generate($record3);
523+
524+
// All three errors should produce the same signature despite different vendor methods
525+
// because they're from the same vendor class and have the same in-app frames
526+
expect($signature2)->toBe($signature1);
527+
expect($signature3)->toBe($signature1);
528+
});

‎tests/Deduplication/VendorFrameDetectorTest.php‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
<?php
2+
3+
namespace Naoray\LaravelGithubMonolog\Tests\Deduplication;
4+
5+
use Naoray\LaravelGithubMonolog\Deduplication\VendorFrameDetector;
6+
7+
beforeEach(function () {
8+
$this->detector = new VendorFrameDetector;
9+
});
10+
11+
test('it detects vendor frames from file path', function () {
12+
$frame = [
13+
'file' => '/path/to/vendor/laravel/framework/src/File.php',
14+
'line' => 123,
15+
'function' => 'someMethod',
16+
];
17+
18+
expect($this->detector->isVendorFrame($frame))->toBeTrue();
19+
});
20+
21+
test('it detects vendor frames from artisan path', function () {
22+
$frame = [
23+
'file' => '/path/to/artisan',
24+
'line' => 13,
25+
'function' => 'run',
26+
];
27+
28+
expect($this->detector->isVendorFrame($frame))->toBeTrue();
29+
});
30+
31+
test('it detects vendor frames from main function', function () {
32+
$frame = [
33+
'file' => '/path/to/index.php',
34+
'line' => 1,
35+
'function' => '{main}',
36+
];
37+
38+
expect($this->detector->isVendorFrame($frame))->toBeTrue();
39+
});
40+
41+
test('it does not detect app frames as vendor frames', function () {
42+
$frame = [
43+
'file' => '/path/to/app/Services/Service.php',
44+
'line' => 25,
45+
'function' => 'handle',
46+
];
47+
48+
expect($this->detector->isVendorFrame($frame))->toBeFalse();
49+
});
50+
51+
test('it does not detect BoundMethod calling app code as vendor frame', function () {
52+
$frame = [
53+
'file' => '/path/to/vendor/laravel/framework/src/Support/BoundMethod.php',
54+
'line' => 123,
55+
'class' => 'Illuminate\\Support\\BoundMethod',
56+
'type' => '::',
57+
'function' => 'App\\Services\\Service::handle',
58+
];
59+
60+
expect($this->detector->isVendorFrame($frame))->toBeFalse();
61+
});
62+
63+
test('it detects BoundMethod not calling app code as vendor frame', function () {
64+
$frame = [
65+
'file' => '/path/to/vendor/laravel/framework/src/Support/BoundMethod.php',
66+
'line' => 123,
67+
'class' => 'Illuminate\\Support\\BoundMethod',
68+
'type' => '::',
69+
'function' => 'Vendor\\Class::method',
70+
];
71+
72+
expect($this->detector->isVendorFrame($frame))->toBeTrue();
73+
});
74+
75+
test('it handles frames without file', function () {
76+
$frame = [
77+
'line' => 123,
78+
'function' => 'someMethod',
79+
];
80+
81+
expect($this->detector->isVendorFrame($frame))->toBeFalse();
82+
});
83+
84+
test('it detects vendor frames from stack trace line', function () {
85+
$line = '#0 /path/to/vendor/laravel/framework/src/File.php(123): SomeClass->method()';
86+
87+
expect($this->detector->isVendorFrameLine($line))->toBeTrue();
88+
});
89+
90+
test('it detects artisan frames from stack trace line', function () {
91+
$line = '#0 /path/to/artisan(13): Illuminate\\Foundation\\Application->run()';
92+
93+
expect($this->detector->isVendorFrameLine($line))->toBeTrue();
94+
});
95+
96+
test('it detects main function from stack trace line', function () {
97+
$line = '#105 {main}';
98+
99+
expect($this->detector->isVendorFrameLine($line))->toBeTrue();
100+
});
101+
102+
test('it does not detect app frames from stack trace line', function () {
103+
$line = '#0 /path/to/app/Services/Service.php(25): App\\Services\\Service->handle()';
104+
105+
expect($this->detector->isVendorFrameLine($line))->toBeFalse();
106+
});
107+
108+
test('it does not detect BoundMethod calling app code from stack trace line', function () {
109+
$line = '#0 /path/to/vendor/laravel/framework/src/Support/BoundMethod.php(123): App\\Services\\Service::handle()';
110+
111+
expect($this->detector->isVendorFrameLine($line))->toBeFalse();
112+
});
113+
114+
test('it detects BoundMethod not calling app code from stack trace line', function () {
115+
$line = '#0 /path/to/vendor/laravel/framework/src/Support/BoundMethod.php(123): Vendor\\Class::method()';
116+
117+
expect($this->detector->isVendorFrameLine($line))->toBeTrue();
118+
});

0 commit comments

Comments
 (0)