-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix Investigate App Crash MainActivity.onCreate #36465
Conversation
@hayata-suenaga Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Assigning @akinwale as the reviewer since he was the one who approved the proposal. |
@akinwale It pasted 4 days. Is there any problem to approve this PR? |
I am trying to test the solution. I will have an update soon. |
I created an adhoc release build locally, and tried to test on browserstack, but a OnePlus 8 Pro running Android v11 is not available, so I tested on the closest possible configurations:
The release build does not crash on launch with the changes applied. cc @mountiny |
Reviewer Checklist
Screenshots/VideosAndroid: Native36465-android-browserstack.mp4Android: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Sorry for chiming in. Did you get crash on release build before this PR? |
I don't have the exact build to test this with. |
I think you can download and install any recent adhoc build. |
Thanks. I'd need the build for the |
Not sure why the automatic flow is not working here, assigning @marcaaron since he was part of the original issue |
@akinwale did you also test the |
@akinwale It seems like we can still see this crash happening but it seems specific to that one device |
If then I think we can use 'com.facebook.soloader:soloader:0.11.0+' or 'com.facebook.soloader:soloader:0.10.5+' instead of 'com.facebook.soloader:soloader:0.10.4+'. |
@yoyumiracle I think what we want to achieve here is to confirm to verify this crash happens on some device we can test on in main and then check if its resolved on this PR as now its a bit of guesswork |
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
Asked QA to see if they have this device to test with https://expensify.slack.com/archives/C9YU7BX5M/p1708380774682179?thread_ts=1708378569.586009&cid=C9YU7BX5M |
Still no luck I believe |
We started a exploratory round with community testers to look for a crash in One Plus 8 Pro devices. |
@mountiny Sorry for asking like this. Was I hired in Upwork for this issue? I can't see any order on my upwork account. |
It might be the C+ here who helps to test and review the PR @akinwale who you see hired in Upwork Do not worry you are hired too, however, we need a way to verify the crash and fix. At the moment, you did not include any videos of the app working as expected compared to main. To completely verify we have fixed this, we are looking for reproduction steps first @yoyumiracle can you also help with that? |
Hello @mountiny. Thanks for your kind answer. |
Can anyone suggest at least the area where the crash is happening? |
The crash log indicates that it crashes on launch, i.e. immediately a user tries to open the app from the app drawer. |
@akinwale How is it going? Is there any probable to approve this PR? I'm tired to wait for result. |
@yoyumiracle Based on #36465 (comment), we're still waiting to be able to test on a OnePlus 8 Pro specifically. |
I'm guessing this is still holding on testing? Can we do anything to move it forward? What are we blocked on? Thanks! |
Does it move forward? I can't believe this situation. |
@marcaaron Hello. |
@yoyumiracleI will ask QA to look into this. I can assure you that nobody is making fun of you. We are a busy company and things get lost in translation at times. Thanks for your patience. |
QA results are that no crash was found. @yoyumiracle there is a |
Hi @marcaaron. |
I fixed the bug related on crash log.
Details
Fixed Issues
$ #35655
PROPOSAL: #35655 (comment)
Tests
Offline tests
N/A
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
It doesn't need any Screenshot/Videos.