Skip to content
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.

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

Native crash handling #4517

Merged
merged 1 commit into from
May 10, 2024
Merged

Native crash handling #4517

merged 1 commit into from
May 10, 2024

Conversation

aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented May 6, 2024

Task/Issue URL: https://app.asana.com/0/488551667048375/1207205378744694/f

Description

Add native crash handler and basic reporting

Steps to test this PR

Test native crash handing

  • apply the patch from this Asana task to this PR
  • build and install and launch app
  • verify Native crash handler init pixel sent appears in logcat
  • verify Native crash handler successfully initialized. apperas in logcat
  • verify app crashes and Native crash pixel sent appears in logcat
  • In kibana, verify pixels appear when applying this filter pixel:m.app*.native.crash.*
  • pixels have the right params, main process name, right appVersion and right customTab true/false value

Copy link
Collaborator Author

aitorvs commented May 6, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @aitorvs and the rest of your teammates on Graphite Graphite

@aitorvs aitorvs force-pushed the feature/aitor/ndk_crash branch from 5eb6f4d to 2f0e4de Compare May 6, 2024 12:11
@aitorvs aitorvs marked this pull request as ready for review May 6, 2024 12:45
@aitorvs aitorvs mentioned this pull request May 6, 2024
11 tasks
@aitorvs aitorvs force-pushed the feature/aitor/ndk_crash branch from 2f0e4de to db2487f Compare May 7, 2024 10:53
@aitorvs aitorvs marked this pull request as draft May 7, 2024 22:50
@aitorvs aitorvs force-pushed the feature/aitor/ndk_crash branch 4 times, most recently from 1bd7855 to 3c64082 Compare May 8, 2024 09:33
@aitorvs aitorvs marked this pull request as ready for review May 8, 2024 09:33
@aitorvs aitorvs force-pushed the feature/aitor/ndk_crash branch from 3c64082 to 318f7ec Compare May 8, 2024 09:38
@aitorvs aitorvs force-pushed the feature/aitor/ndk_crash branch from 318f7ec to ed2641e Compare May 8, 2024 10:03
@CDRussell CDRussell self-assigned this May 8, 2024
@CDRussell
Copy link
Member

Tried testing with the patch; it crashes but i'm not seeing any pixels (not in logcat nor kibana). Will wait until we get a chance to pair to look more into it as I might be doing something wrong in the testing, but heads up in case you know why this might be the case, @aitorvs

@aitorvs
Copy link
Collaborator Author

aitorvs commented May 8, 2024

Tried testing with the patch; it crashes but i'm not seeing any pixels (not in logcat nor kibana). Will wait until we get a chance to pair to look more into it as I might be doing something wrong in the testing, but heads up in case you know why this might be the case, @aitorvs

We can take a look tomorrow. I used the patches and could crash and see the pixels in kibana.
Admittedly the test steps were not very accurate (fixed that) so maybe that was the issue.

Gonna run this on emulator as well to see if there's any difference (been running it in pixel 4XL)

private val nativeCrashFeature: NativeCrashFeature,
) : MainProcessLifecycleObserver {

private val isCustomTab: Boolean by lazy { customTabDetector.isCustomTab() }
Copy link
Member

Choose a reason for hiding this comment

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

this is lazily-init'd but the class is a singleton; won't it pick up whether it was a custom tab first time and cache that value thereafter regardless of whether custom tab state changes?

Copy link
Member

@CDRussell CDRussell left a comment

Choose a reason for hiding this comment

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

Still unsure about custom tab param (see comment), but otherwise, LGTM

@aitorvs aitorvs merged commit 9f565e7 into develop May 10, 2024
7 checks passed
@aitorvs aitorvs deleted the feature/aitor/ndk_crash branch May 10, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants