Skip to content

Commit 2530dd2

Browse files
NaorayNaoray
committed
feat: enhance message handling for edge cases
1 parent ffa9dd2 commit 2530dd2

File tree

4 files changed

+141
-12
lines changed

4 files changed

+141
-12
lines changed

‎src/Issues/Formatters/ExceptionFormatter.php‎

Lines changed: 71 additions & 7 deletions
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".$this->stackTraceFormatter->format($stackTrace, collapseVendorFrames: false),
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‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ public function format(string $stackTrace, bool $collapseVendorFrames = true): s
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('/^#(\d)(?!\d)/', '#0$1', $line);
31+
$line = $this->padStackTraceLine($line);
3432

3533
if ($collapseVendorFrames && $this->isVendorFrame($line)) {
3634
return self::VENDOR_FRAME_PLACEHOLDER;
@@ -42,6 +40,18 @@ public function format(string $stackTrace, bool $collapseVendorFrames = true): s
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+
* @param string $line
48+
* @return string
49+
*/
50+
public function padStackTraceLine(string $line): string
51+
{
52+
return (string) preg_replace('/^#(\d)(?!\d)/', '#0$1', $line);
53+
}
54+
4555
private function formatInitialException(string $line): array
4656
{
4757
[$message, $exception] = explode('{"exception":"[object] ', $line);

‎tests/Issues/Formatters/ExceptionFormatterTest.php‎

Lines changed: 53 additions & 1 deletion
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,49 @@
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+
public function __construct()
64+
{
65+
parent::__construct("The calculation amount [123.45] does not match the expected total [456.78]. in /path/to/app/Calculations/Calculator.php:49
66+
Stack trace:
67+
#0 /path/to/app/Services/PaymentService.php(83): App\\Calculations\\Calculator->calculate()
68+
#1 /vendor/framework/src/Services/TransactionService.php(247): App\\Services\\PaymentService->process()");
69+
}
70+
};
71+
72+
$record = createLogRecord('Test message', exception: $exception);
73+
74+
$result = $this->formatter->format($record);
75+
76+
// The formatter should extract just the actual error message without the file path or stack trace
77+
expect($result)
78+
->toBeArray()
79+
->toHaveKeys(['message', 'simplified_stack_trace', 'full_stack_trace'])
80+
->and($result['message'])->toBe('The calculation amount [123.45] does not match the expected total [456.78].')
81+
->and($result['simplified_stack_trace'])->toContain('[stacktrace]')
82+
->and($result['simplified_stack_trace'])->toContain('App\\Calculations\\Calculator->calculate()');
83+
});
84+
85+
test('it handles exceptions with string in context', function () {
86+
// Create a generic exception string
87+
$exceptionString = "The calculation amount [123.45] does not match the expected total [456.78]. in /path/to/app/Calculations/Calculator.php:49
88+
Stack trace:
89+
#0 /path/to/app/Services/PaymentService.php(83): App\\Calculations\\Calculator->calculate()
90+
#1 /vendor/framework/src/Services/TransactionService.php(247): App\\Services\\PaymentService->process()";
91+
92+
// Create a record with a string in the exception context
93+
$record = createLogRecord('Test message', ['exception' => $exceptionString]);
94+
95+
$result = $this->formatter->format($record);
96+
97+
// Should extract just the clean message
98+
expect($result)
99+
->toBeArray()
100+
->toHaveKeys(['message', 'simplified_stack_trace', 'full_stack_trace'])
101+
->and($result['message'])->toBe('The calculation amount [123.45] does not match the expected total [456.78].')
102+
->and($result['simplified_stack_trace'])->toContain('[stacktrace]')
103+
->and($result['simplified_stack_trace'])->toContain('App\\Calculations\\Calculator->calculate()');
104+
});

‎tests/Pest.php‎

Lines changed: 4 additions & 1 deletion
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)