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

Add Crashlytics to Chat #582

Merged
merged 21 commits into from
Oct 16, 2020
Merged

Add Crashlytics to Chat #582

merged 21 commits into from
Oct 16, 2020

Conversation

Jag96
Copy link
Contributor

@Jag96 Jag96 commented Oct 1, 2020

@timszot @AndrewGable will you please review this?

This PR adds Crashlytics to Chat so that we can get reports of crashes that users are experiencing as they happen. This is configured to only report crashes for release builds, dev builds will not report crashes. This PR also updates the fastlane file on iOS to upload the dsym from the build output to crashlytics automatically.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/142274

Tests

  1. Run npm install and cd ios && pod install to confirm all the dependencies are up to date
  2. Run the apps via npm run ios/npm run android (debug builds) and confirm that the apps build fine
  3. Update some JS to cause a crash. I updated submitForm in ReportActionCompose.js to do abc.test(); at the beginning of the function.
  4. Build the app again and cause the crash, then go to Crashlytics. Confirm that the crash isn't uploaded.
  5. Go into ~/Library/Developer/Xcode/DerivedData/ReactNativeChat-bkiuyhfpclvjiabklrsfbkppdtbh/Build/Products/Release-iphonesimulator/ (replace your simluator name) and delete the Chat.app.dSYM file if it exists. This will allow us to overwrite it on the next build, so we can upload it to Firebase
  6. Build a release version of the app to your device/emulator
    1. On Android, do this by running react-native run-android --variant=release
    2. On iOS, you can do this by following these instructions. If you have issues with profiles, you can also update the Scheme to run with the Release build (follow these instructions), just be sure to uncheck Debug Executable, then run the build normally to a simulator.
  7. For iOS, we need to upload the dsym so that Crashlytics can register the crashes. To do this, run the following from ReactNativeChat, be sure to update your simulator name (after DerivedData)
~/Expensidev/ReactNativeChat/iOS/pods/FirebaseCrashlytics/upload-symbols -gsp ios/GoogleService-Info.plist -p ios  ~/Library/Developer/Xcode/DerivedData/ReactNativeChat-bkiuyhfpclvjiabklrsfbkppdtbh/Build/Products/Release-iphonesimulator/Chat.app.dSYM
  1. Go back to the Crashlytics console and confirm that your crashes show up. Note that for iOS this seems to take a few minutes (up to 10), but on Android they were posted within 2 minutes.
  2. Confirm you can run the web app and desktop app without any issues

@Jag96 Jag96 self-assigned this Oct 1, 2020
@Jag96

This comment has been minimized.

@Jag96
Copy link
Contributor Author

Jag96 commented Oct 7, 2020

Ok, I messed around with this a bit more and I got the crash showing up on the dashboard. Going to update this PR w/ the working config and then test/confirm that is what works tomorrow.

@Jag96

This comment has been minimized.

@Jag96
Copy link
Contributor Author

Jag96 commented Oct 10, 2020

Got Android working (thanks to @timszot for helping me test) and to the point where only Release builds are producing crashes on the dashboard. Will clean this up next week and get it in review.

@Jag96 Jag96 changed the title [WIP] Add Crashlytics to Chat Add Crashlytics to Chat Oct 15, 2020
@Jag96 Jag96 requested review from AndrewGable and timszot October 15, 2020 00:41
@Jag96 Jag96 marked this pull request as ready for review October 15, 2020 00:43
Copy link
Contributor

@timszot timszot left a comment

Choose a reason for hiding this comment

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

Code looks good. Going to run through the tests this afternoon.

@timszot
Copy link
Contributor

timszot commented Oct 15, 2020

Ah, forgot to mention that there are some conflicts, most likely due to mis-matched versions in the build.gradle.

@timszot
Copy link
Contributor

timszot commented Oct 15, 2020

Ran into some dev environment problems, but I got through the first few steps of the tests now. Tomorrow morning I'll test the release builds portion.

timszot
timszot previously approved these changes Oct 16, 2020
Copy link
Contributor

@timszot timszot left a comment

Choose a reason for hiding this comment

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

👍

@timszot timszot merged commit d570a50 into master Oct 16, 2020
@timszot timszot deleted the joe-crashlytics branch October 16, 2020 19:41
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