Skip to content

feat: canonical bottom-up stack frame order for error tracking #200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cat-ph wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: canonical bottom-up stack frame order for error tracking #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/canonical-frame-order.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog-php": minor

@greptile-apps greptile-apps Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Breaking Change Bumps Minor

This changes the wire meaning of stacktrace.frames[0] from crash site to entry point. Consumers that follow normal version ranges can upgrade from 4.8.x to 4.9.0 and silently read the wrong crash location, so the changeset should match the repo policy for breaking changes unless the release policy is also being changed.

Suggested change
"posthog-php": minor
"posthog-php": major

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

Emit error tracking stack frames in the canonical bottom-up order: `stacktrace.frames[0]` is now the outermost/entry-point call and the last frame is the crash site. This aligns the PHP SDK with the cross-SDK stack frame ordering standard.
10 changes: 8 additions & 2 deletions lib/ExceptionPayloadBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ private static function normalizeThrowableTrace(\Throwable $exception): array
&& !self::isDeclarationLineForFirstFrame($exception, $trace[0])
) {
// Many PHP exceptions report the throw site in getFile()/getLine() but omit it
// from getTrace()[0]. Prepending a synthetic top frame keeps the first frame aligned
// with the highlighted source location in PostHog.
// from getTrace()[0]. Prepending a synthetic frame at the innermost end keeps the
// crash site aligned with the highlighted source location in PostHog (it becomes the
// last frame once buildStacktrace reverses the trace into canonical bottom-up order).
array_unshift($trace, array_filter([
'file' => $exception->getFile(),
'line' => $exception->getLine(),
Expand Down Expand Up @@ -232,6 +233,11 @@ private static function buildStacktrace(?array $trace, int $maxFrames): ?array

$frames = array_values(array_filter($frames));

// PHP traces are innermost-first (crash site at index 0). PostHog's canonical wire order is
// bottom-up: frames[0] is the outermost/entry-point call and the last frame is the crash
// site. Slicing before reversing keeps the maxFrames most-relevant (crash-site) frames.
$frames = array_reverse($frames);

return [
'type' => 'raw',
'frames' => $frames,
Expand Down
16 changes: 10 additions & 6 deletions test/ExceptionCaptureTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,16 @@ public function testErrorHandlerCapturesNonFatalErrorsWithoutCaptureFrames(): vo
$event['properties']['$exception_list'][0]['mechanism']
);
$this->assertSame('ErrorException', $event['properties']['$exception_list'][0]['type']);
$this->assertSame('trigger_error', $frames[0]['function']);
$this->assertSame(__FILE__, $frames[0]['abs_path']);
$this->assertSame($triggerLine, $frames[0]['lineno']);
$this->assertSame(__CLASS__ . '->triggerWarningHelper', $frames[1]['function']);
$this->assertSame(__FILE__, $frames[1]['abs_path']);
$this->assertSame($callSiteLine, $frames[1]['lineno']);
// Canonical bottom-up order: the crash site (trigger_error) is the last frame and its
// caller sits just before it.
$crashFrame = $frames[count($frames) - 1];
$callerFrame = $frames[count($frames) - 2];
$this->assertSame('trigger_error', $crashFrame['function']);
$this->assertSame(__FILE__, $crashFrame['abs_path']);
$this->assertSame($triggerLine, $crashFrame['lineno']);
$this->assertSame(__CLASS__ . '->triggerWarningHelper', $callerFrame['function']);
$this->assertSame(__FILE__, $callerFrame['abs_path']);
$this->assertSame($callSiteLine, $callerFrame['lineno']);
$this->assertFalse($this->framesContainFunction($frames, ExceptionCapture::class . '::handleError'));
} finally {
error_reporting($previousReporting);
Expand Down
88 changes: 74 additions & 14 deletions test/ExceptionPayloadBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ public function testStacktraceUsesThrowableFileAndLineForMostRecentFrame(): void
[$exception, $throwLine] = $this->throwHelperWithKnownLine();
$result = ExceptionPayloadBuilder::buildExceptionList($exception);

// Canonical bottom-up order: the crash site (most recent frame) is the last frame.
$frames = $result[0]['stacktrace']['frames'];
$frame = $frames[0];
$frame = $frames[count($frames) - 1];

$this->assertEquals(__FILE__, $frame['abs_path']);
$this->assertEquals($throwLine, $frame['lineno']);
Expand All @@ -165,9 +166,10 @@ public function testStacktracePreservesOriginalCallerFrame(): void
[$exception, $throwLine, $callerLine] = $this->nestedThrowHelperWithKnownLines();
$result = ExceptionPayloadBuilder::buildExceptionList($exception);

// Canonical bottom-up order: the innermost (crash) frame is last, its caller sits just before it.
$frames = array_values($result[0]['stacktrace']['frames']);
$innermostFrame = $frames[0];
$callerFrame = $frames[1];
$innermostFrame = $frames[count($frames) - 1];
$callerFrame = $frames[count($frames) - 2];

$this->assertEquals(__FILE__, $innermostFrame['abs_path']);
$this->assertEquals($throwLine, $innermostFrame['lineno']);
Expand All @@ -181,15 +183,18 @@ public function testInternalFunctionErrorDoesNotDuplicateTopFrame(): void
[$exception, $arraySumLine, $callerLine] = $this->internalErrorHelperWithKnownLines();
$result = ExceptionPayloadBuilder::buildExceptionList($exception);

// Canonical bottom-up order: the internal function (crash site) is last, its caller before it.
$frames = array_values($result[0]['stacktrace']['frames']);
$crashFrame = $frames[count($frames) - 1];
$callerFrame = $frames[count($frames) - 2];

$this->assertEquals('array_sum', $frames[0]['function']);
$this->assertEquals(__FILE__, $frames[0]['abs_path']);
$this->assertEquals($arraySumLine, $frames[0]['lineno']);
$this->assertEquals(__CLASS__ . '->internalErrorLeaf', $frames[1]['function']);
$this->assertEquals(__FILE__, $frames[1]['abs_path']);
$this->assertEquals($callerLine, $frames[1]['lineno']);
$this->assertNotEquals($frames[0], $frames[1]);
$this->assertEquals('array_sum', $crashFrame['function']);
$this->assertEquals(__FILE__, $crashFrame['abs_path']);
$this->assertEquals($arraySumLine, $crashFrame['lineno']);
$this->assertEquals(__CLASS__ . '->internalErrorLeaf', $callerFrame['function']);
$this->assertEquals(__FILE__, $callerFrame['abs_path']);
$this->assertEquals($callerLine, $callerFrame['lineno']);
$this->assertNotEquals($crashFrame, $callerFrame);
}

public function testStrictTypeErrorUsesCallsiteBeforeDeclaration(): void
Expand Down Expand Up @@ -226,12 +231,14 @@ function requiresIntForTrace(int $value): int
$this->assertInstanceOf(\Throwable::class, $exception);

$result = ExceptionPayloadBuilder::buildExceptionList($exception);
// Canonical bottom-up order: the real callsite (crash site) is the last frame.
$frames = array_values($result[0]['stacktrace']['frames']);
$crashFrame = $frames[count($frames) - 1];

$this->assertSame($scriptPath, $frames[0]['abs_path']);
$this->assertSame('requiresIntForTrace', $frames[0]['function']);
$this->assertSame($callLine, $frames[0]['lineno']);
$this->assertNotSame($declarationLine, $frames[0]['lineno']);
$this->assertSame($scriptPath, $crashFrame['abs_path']);
$this->assertSame('requiresIntForTrace', $crashFrame['function']);
$this->assertSame($callLine, $crashFrame['lineno']);
$this->assertNotSame($declarationLine, $crashFrame['lineno']);
} finally {
unlink($scriptPath);
}
Expand All @@ -251,6 +258,59 @@ public function testFrameLimitCanBeConfigured(): void
$this->assertArrayHasKey('context_line', $frames[1]);
}

public function testStacktraceFramesUseCanonicalBottomUpOrder(): void
{
// Regression guard for the canonical wire order. PHP's getTrace() is innermost-first
// (crash site at index 0); PostHog expects bottom-up frames where frames[0] is the
// outermost/entry-point call and the last frame is the crash site.
$exception = $this->orderedNestedThrowHelper();
$result = ExceptionPayloadBuilder::buildExceptionList($exception);

$frames = array_values($result[0]['stacktrace']['frames']);
$this->assertGreaterThanOrEqual(3, count($frames), 'nested throw should yield multiple frames');

$functions = array_map(static fn(array $frame) => $frame['function'] ?? null, $frames);

// The three helpers unwind inward: leaf (crash) is called by middle, called by the entry
// point. Canonical order therefore reads entry -> middle -> leaf from first to last.
$entryIndex = array_search(__CLASS__ . '->orderedNestedThrowHelper', $functions, true);
$middleIndex = array_search(__CLASS__ . '->orderedThrowMiddle', $functions, true);
$leafIndex = array_search(__CLASS__ . '->orderedThrowLeaf', $functions, true);

$this->assertNotFalse($entryIndex);
$this->assertNotFalse($middleIndex);
$this->assertNotFalse($leafIndex);
$this->assertTrue(
$entryIndex < $middleIndex && $middleIndex < $leafIndex,
'frames must read bottom-up: entry point first, crash site last'
);
Comment on lines +274 to +286

@greptile-apps greptile-apps Bot Jul 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Throwing Frame Is Missing

When orderedThrowLeaf() throws, PHP omits that throwing function from Throwable::getTrace(). The synthetic crash-site frame preserves the throw file and line, but it does not add orderedThrowLeaf as a function name, so $leafIndex stays false and this test fails in CI.

Suggested change
// The three helpers unwind inward: leaf (crash) is called by middle, called by the entry
// point. Canonical order therefore reads entry -> middle -> leaf from first to last.
$entryIndex = array_search(__CLASS__ . '->orderedNestedThrowHelper', $functions, true);
$middleIndex = array_search(__CLASS__ . '->orderedThrowMiddle', $functions, true);
$leafIndex = array_search(__CLASS__ . '->orderedThrowLeaf', $functions, true);
$this->assertNotFalse($entryIndex);
$this->assertNotFalse($middleIndex);
$this->assertNotFalse($leafIndex);
$this->assertTrue(
$entryIndex < $middleIndex && $middleIndex < $leafIndex,
'frames must read bottom-up: entry point first, crash site last'
);
// The reachable PHP trace records caller frames; the throwing function itself is
// represented by the synthetic crash-site frame's file and line below.
$entryIndex = array_search(__CLASS__ . '->orderedNestedThrowHelper', $functions, true);
$middleIndex = array_search(__CLASS__ . '->orderedThrowMiddle', $functions, true);
$this->assertNotFalse($entryIndex);
$this->assertNotFalse($middleIndex);
$this->assertTrue(
$entryIndex < $middleIndex,
'frames must read bottom-up: entry point first, crash-site caller last'
);


// The very last frame is the crash site: it carries the throw location (file + line) and
// is the synthetic throw-site frame appended for the leaf.
$crashFrame = $frames[count($frames) - 1];
$this->assertSame(__FILE__, $crashFrame['abs_path']);
$this->assertSame($exception->getLine(), $crashFrame['lineno']);
}

private function orderedNestedThrowHelper(): \RuntimeException
{
try {
$this->orderedThrowMiddle();
} catch (\RuntimeException $e) {
return $e;
}
}

private function orderedThrowMiddle(): void
{
$this->orderedThrowLeaf();
}

private function orderedThrowLeaf(): never
{
throw new \RuntimeException('canonical order leaf');
}

private function throwHelper(): \RuntimeException
{
try {
Expand Down
Loading