Repository navigation
feat(flutter): emit native frames for obfuscated AOT errors - #677
Conversation
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 9358e8c: 3 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
| processedFrame = { | ||
| 'platform': 'native', | ||
| 'lang': 'dart', | ||
| 'instruction_addr': '0x${instructionAddress.toRadixString(16)}', |
There was a problem hiding this comment.
P2 · Compensate for the resolver's call-site adjustment
Dart AOT traces already report call addresses, but Cymbal's RawNativeFrame::resolve_impl subtracts one again. For future-listener frames, Dart reports the closure entry point, so this extra subtraction places the lookup before the closure and resolves the wrong function or no symbol. Bias instruction_addr by +1 to match the resolver.
There was a problem hiding this comment.
Good catch. Dart AOT reports the program counter directly. The companion server change in PostHog/posthog#124249 now skips the return-address adjustment for Dart frames, so the SDK keeps the actual instruction address. A regression test resolves a function-entry address from an invented Flutter artifact.
| if (instructionAddress != null && | ||
| imageBase != null && | ||
| instructionAddress >= imageBase) { | ||
| processedFrame = { | ||
| 'platform': 'native', | ||
| 'lang': 'dart', | ||
| 'instruction_addr': '0x${instructionAddress.toRadixString(16)}', | ||
| 'image_addr': aotImage!.imageAddr, |
There was a problem hiding this comment.
P2 · Associate deferred frames with their own loading-unit images
Android deferred-component traces contain loading_unit headers and unit N frames. This branch assigns deferred addresses above the root base to the root image, making Cymbal query the wrong symbol set. The SDK contract required by AGENTS.md:11 requires matching images; select each unit's build ID/base or preserve those frames as raw.
| 'lang': 'dart', | ||
| 'instruction_addr': '0x${instructionAddress.toRadixString(16)}', | ||
| 'image_addr': aotImage!.imageAddr, | ||
| 'in_app': true, |
There was a problem hiding this comment.
P2 · Honor the configured default for AOT frame classification
When inAppByDefault is false, every recognized AOT frame still receives in_app: true. The ordinary-frame path honors this setting for unknown origins, so enabling split debug information changes classification and makes external frames participate as application code. Use inAppByDefault for native frames whose package cannot be determined.
posthog-flutter Compliance ReportDate: 2026-10-28 10:32:18 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
dustinbyrne
left a comment
There was a problem hiding this comment.
No actionable defects found in the AOT frame conversion, fallback handling, cause-image collection, or native forwarding path. Coordinate the companion backend correction and complete running-build/symbol verification before release.
AI-assisted review.
| ) | ||
| : <Map<String, dynamic>>[]; | ||
| : null; | ||
| final frames = parsedTrace?.frames ?? <Map<String, dynamic>>[]; |
There was a problem hiding this comment.
always use Object or Object? instead of dynamic so we can be explicit about null safety
There was a problem hiding this comment.
Updated the new frame, image, and cause maps to Object? in 5dde65a. The existing processException return type stays unchanged for compatibility.
| List<String>? inAppIncludes, | ||
| List<String>? inAppExcludes, | ||
| bool inAppByDefault = true, | ||
| required List<Map<String, dynamic>> debugImages, |
There was a problem hiding this comment.
i'd avoid required since its not needed if strip debug symbols compiler flag isnt used
There was a problem hiding this comment.
required here applies only to a private helper's image accumulator. processException passes an empty list even when no AOT metadata exists, and it adds $debug_images to the event only if that list is nonempty. No SDK caller needs to provide debug images. I kept this internal argument required so cause frames can add images to the same list.
| List<String>? inAppIncludes, | ||
| List<String>? inAppExcludes, | ||
| bool inAppByDefault = true, | ||
| required List<Map<String, dynamic>> debugImages, |
There was a problem hiding this comment.
There was a problem hiding this comment.
Same for this recursive private helper: it always receives the internal accumulator, which can be empty. Events without native AOT frames omit $debug_images; required does not make it a public option or an event field.
💡 Motivation and Context
Flutter developers using obfuscated Android and iOS builds cannot inspect readable Dart frames in error tracking (request #433).
$debug_images.inAppByDefault. Cause stacks contribute their referenced debug images.Android AOT event-to-UI matching is verified in an isolated local project; iOS AOT runtime validation and coordinated release remain pending. The change does not remap obfuscated exception type names or change the app UI directly.
💚 How did you test it?
flutter test --no-pub,make checkFormatDart,make analyzeDart, andpnpm exec changeset status.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file. (A minor changeset was added directly and validated withpnpm exec changeset status.)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: pi, gpt-6-sol
f4faad3e-b1d7-4e72-90f1-eab640e344b9(no public session link available). Tools: read, edit, write, bash, codemode./pr,/writing-pr-descriptions,/writing-tests,/writing-code-comments,/writing-user-facing-copy,/announcing-behavior-changes.