[HOLD for payment 2024-08-19] [HOLD for payment 2024-08-09] [$250] Associate crashes with specific logins

[roryabraham]

Problem

When users report that their staging or production app crashed, it can be difficult to find the stacktrace for that crash after-the-fact. This makes it harder to identify and fix real-world crashes, leading to a poorer UX in terms of app quality and the level of support we can provide to users who do experience crashes.

Solution

Upon login, associate the user login with crashlytics user ID. That way, if the app crashes, the crash report will be logged along with the login for the user. Then we'll be able to search for crashes by user in the Crashlytics console, and maybe find out more.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019268795ed9fe7be1
• Upwork Job ID: 1795144470205255680
• Last Price Increase: 2024-05-27
• Automatic offers:
• getusha | Reviewer | 102519677
• ShridharGoel | Contributor | 102519678

Issue Owner

Current Issue Owner: @dylanexpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019268795ed9fe7be1

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @getusha (External)

[melvin-bot]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[roryabraham]

FWIW I think the code for this is the easy part - testing is the harder part. Not sure what's needed to test crashlytics locally

[dragnoir]

I can see some ideas here for crashing the app https://stackoverflow.com/questions/11807354/how-to-crash-an-android-app-programmatically

[ShridharGoel]

Proposal

Please re-state the problem that we are trying to solve in this issue.

We need a way to associate crashes with IDs.

What is the root cause of that problem?

New feature.

What changes do you think we should make in order to solve the problem?

We can set the userId in crashlytics as soon as the login is done.

Add the below in Expensify.tsx:

useEffect(() => {
if (isAuthenticated) {
    crashlytics().setUserId(Number(session?.accountID).toString()) // We can also use login by getting personal details via Onyx
}
}, [isAuthenticated, session?.accountID])

Note:

We can also add email info using crashlytics().setAttribute('userEmail', userEmail);.
Also, details like device ID etc. can be added in the same way. If logged out, then instead of login we can pass something else like device ID in setUserId.

For testing, we can use crashlytics().crash(); to mock it, and then an internal engineer can check the logs on Firebase.
We'll also need to enable SEND_CRASH_REPORTS in debug mode.

To test native crashes:

Add a new option in TestToolMenu:

<TestToolRow title="Test crash">
<Button
    small
    text="Test crash"
    onPress={() => {
        crashlytics().crash();
    }}
/>
</TestToolRow>

To test JS crashes:

Option 1:

Just add <View>Test</View> anywhere, like in Expensify.tsx or ReportScreen.tsx.

Option 2:

Add a new item in TestToolMenu:

<TestToolRow title="JS crash">
    <Button
        small
        text="JS crash"
        onPress={() => {
            Check
        }}
    />
</TestToolRow>

Since "Check" doesn't exist, this will lead to an exception.

Update firebase.json to set crashlytics_debug_enabled to true.

Note that local logs will show what data is being sent to Firebase, we can use that to verify the changes. Then, once it is on staging, then someone can check on Firebase (using the troubleshoot option).

Native crash would not work on iOS debug build because we are on an old version. See invertase/react-native-firebase#6226

We can also plan to bump the Firebase version in another task.

[getusha]

@ShridharGoel anyway to test it locally?

[ShridharGoel]

If we have crashlytics().setCrashlyticsCollectionEnabled(true); then we can see the data being sent in the local logs.

[roryabraham]

I think we'd definitely want to set the userID as soon as we login, because the error boundary will only catch JS-level errors, not native crashes

[roryabraham]

thanks for the proposal @ShridharGoel, but I think I'd like to see a bit more detail on:

1. how you'd set the userID in Crashlytics as soon as the login is complete
2. how you'd test a crash (JS and native-level)
3. how you'd validate that the data is being sent to Firebase when the crash happens

if we can't test the 3rd part locally, maybe we can just test on staging. But for that we'd need a safe way to temporarily introduce a crash and verify that the report is sent. Maybe a button in the troubleshooting menu?

[ShridharGoel]

Proposal

Updated

[dylanexpensify]

@getusha to review

[roryabraham]

ok, I think @ShridharGoel's proposal has enough detail and LGTM 👍🏼

[melvin-bot]

📣 @getusha 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[getusha]

@roryabraham does this need a c+?

[roryabraham]

I think a C+ could be helpful for the review. Feel free to drop if you're not interested though and we can find another volunteer

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.15-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Associate crashes with user info #43063

If no regressions arise, payment will be issued on 2024-08-09. 🎊

For reference, here are some details about the assignees on this issue:

• @ShridharGoel requires payment automatic offer (Contributor)
• @getusha requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@getusha] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[dylanexpensify]

Coming up this week!

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.18-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [InternalQA] Show test crash option in staging builds #46643

If no regressions arise, payment will be issued on 2024-08-19. 🎊

For reference, here are some details about the assignees on this issue:

• @ShridharGoel requires payment automatic offer (Contributor)
• @getusha requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@getusha] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[dylanexpensify]

Payment coming up!

[dylanexpensify]

Payment summary:

Contributor: @ShridharGoe $250 via Upwork
Contributor+: @getusha $250 via Upwork

Please apply/request!

[dylanexpensify]

Confirming this is done

[JmillsExpensify]

$250 approved for @getusha