Implement Crashlytics Support #59
Conversation
WalkthroughThe changes introduced in this pull request enhance the iOS application by adding a new shell script build phase for uploading Crashlytics symbols to Firebase. This is accompanied by modifications in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
app/lib/main.dart (1)
26-30: Consider selective fatal error flaggingThe current implementation marks all async errors as fatal, which might be too aggressive. Consider adding logic to determine error severity.
PlatformDispatcher.instance.onError = (error, stack) { - FirebaseCrashlytics.instance.recordError(error, stack, fatal: true); + // Only mark certain errors as fatal based on type/severity + final isFatal = error is Error; // or other criteria + FirebaseCrashlytics.instance.recordError(error, stack, fatal: isFatal); return true; };app/macos/Flutter/GeneratedPluginRegistrant.swift (1)
Missing required Firebase configuration files for Crashlytics
The verification reveals that essential Firebase configuration files are missing:
google-services.jsonis missing in the Android configurationGoogleService-Info.plistis missing in the iOS configuration- Crashlytics symbol upload script is not configured in the iOS build phase
Please add these required configuration files and setup steps to ensure Crashlytics functions properly on both platforms.
🔗 Analysis chain
Line range hint
1-1: Verify complete Crashlytics setupLet's ensure all necessary configuration files are present for both platforms.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for required Firebase configuration files echo "Checking Android config..." if [ -f "app/android/app/google-services.json" ]; then echo "✓ google-services.json found" else echo "✗ google-services.json missing" fi echo "Checking iOS config..." if [ -f "app/ios/Runner/GoogleService-Info.plist" ]; then echo "✓ GoogleService-Info.plist found" else echo "✗ GoogleService-Info.plist missing" fi # Check for Crashlytics build phase script in iOS echo "Checking iOS Crashlytics build phase..." if grep -q "firebase_crashlytics/upload-symbols" "app/ios/Runner.xcodeproj/project.pbxproj"; then echo "✓ Crashlytics upload-symbols script found" else echo "✗ Crashlytics upload-symbols script missing" fiLength of output: 643
app/ios/Runner.xcodeproj/project.pbxproj (1)
405-405: Consider adding error handling to the upload scriptThe script should handle potential failures in the symbol upload process to prevent silent failures.
Add error handling by modifying the script:
-flutterfire upload-crashlytics-symbols --upload-symbols-script-path=$PODS_ROOT/FirebaseCrashlytics/upload-symbols --platform=ios --apple-project-path=${SRCROOT} --env-platform-name=${PLATFORM_NAME} --env-configuration=${CONFIGURATION} --env-project-dir=${PROJECT_DIR} --env-built-products-dir=${BUILT_PRODUCTS_DIR} --env-dwarf-dsym-folder-path=${DWARF_DSYM_FOLDER_PATH} --env-dwarf-dsym-file-name=${DWARF_DSYM_FILE_NAME} --env-infoplist-path=${INFOPLIST_PATH} --default-config=default +if ! flutterfire upload-crashlytics-symbols --upload-symbols-script-path=$PODS_ROOT/FirebaseCrashlytics/upload-symbols --platform=ios --apple-project-path=${SRCROOT} --env-platform-name=${PLATFORM_NAME} --env-configuration=${CONFIGURATION} --env-project-dir=${PROJECT_DIR} --env-built-products-dir=${BUILT_PRODUCTS_DIR} --env-dwarf-dsym-folder-path=${DWARF_DSYM_FOLDER_PATH} --env-dwarf-dsym-file-name=${DWARF_DSYM_FILE_NAME} --env-infoplist-path=${INFOPLIST_PATH} --default-config=default; then + echo "error: Failed to upload Crashlytics symbols" >&2 + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
app/ios/Runner.xcodeproj/project.pbxproj(2 hunks)app/lib/main.dart(2 hunks)app/macos/Flutter/GeneratedPluginRegistrant.swift(2 hunks)app/pubspec.yaml(1 hunks)
🔇 Additional comments (4)
app/pubspec.yaml (1)
70-71: Verify Firebase package versions and compatibility
The Firebase packages look good, but let's verify we're using the latest stable versions that are compatible with the existing firebase_core: ^3.7.0.
app/lib/main.dart (1)
23-24: LGTM! Well-placed Flutter error handler
The synchronous error handler is correctly placed after Firebase initialization and uses the recommended recordFlutterFatalError method.
app/macos/Flutter/GeneratedPluginRegistrant.swift (1)
9-9: LGTM! Auto-generated plugin registrations
The Firebase plugin registrations look correct. Note that this file is auto-generated, so any changes should be made through Flutter's build process rather than manual edits.
Also applies to: 11-11, 26-26, 28-28
app/ios/Runner.xcodeproj/project.pbxproj (1)
207-207: LGTM: Build phase properly integrated
The Crashlytics symbols upload phase is correctly added to the Runner target's build phases after the main build steps, ensuring debug symbols are available when the upload occurs.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (1)
.github/workflows/ios_deploy.yml (1)
Line range hint
1-41: Consider adding a cleanup step for sensitive files.To enhance security, consider adding a cleanup step at the end of the workflow to remove sensitive configuration files:
Add this step at the end of the workflow:
- name: Cleanup sensitive files if: always() run: | cd app rm -f firebase.json rm -f ios/Runner/GoogleService-Info.plist rm -f ios/firebase_app_id_file.json rm -f lib/firebase_options.dart cd ../data rm -f lib/apis/network/secrets.dart🧰 Tools
🪛 actionlint (1.7.4)
39-39: shellcheck reported issue in this script: SC2086:info:2:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:3:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:5:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:7:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
[error] 41-41: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.github/workflows/analyze.yml(2 hunks).github/workflows/android_build.yml(1 hunks).github/workflows/android_deploy.yml(1 hunks).github/workflows/ios_deploy.yml(1 hunks).idea/libraries/Dart_Packages.xml(23 hunks).idea/libraries/Flutter_Plugins.xml(1 hunks)app/.gitignore(1 hunks)data/.flutter-plugins-dependencies(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- app/.gitignore
- data/.flutter-plugins-dependencies
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/android_deploy.yml
43-43: shellcheck reported issue in this script: SC2086:info:2:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:3:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:6:6: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/android_build.yml
38-38: shellcheck reported issue in this script: SC2086:info:2:6: Double quote to prevent globbing and word splitting
(shellcheck)
38-38: shellcheck reported issue in this script: SC2086:info:3:6: Double quote to prevent globbing and word splitting
(shellcheck)
38-38: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
38-38: shellcheck reported issue in this script: SC2086:info:6:6: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ios_deploy.yml
39-39: shellcheck reported issue in this script: SC2086:info:2:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:3:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:5:6: Double quote to prevent globbing and word splitting
(shellcheck)
39-39: shellcheck reported issue in this script: SC2086:info:7:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/ios_deploy.yml
[error] 41-41: trailing spaces
(trailing-spaces)
🔇 Additional comments (9)
.idea/libraries/Flutter_Plugins.xml (2)
46-47: LGTM: Firebase integration looks correct.
The addition of Firebase plugins aligns well with the PR objective of implementing Crashlytics support. The necessary dependencies are included:
- firebase_core
- firebase_analytics
- firebase_crashlytics
Also applies to: 51-53
27-28: Verify if video player integration is intended.
New video player plugins have been added, but this seems unrelated to the PR's objective of implementing Crashlytics. Please confirm if this is an intentional change.
Also applies to: 56-56
.idea/libraries/Dart_Packages.xml (3)
19-25: LGTM: Firebase packages added correctly
The required Firebase packages for Crashlytics implementation have been properly added:
- _flutterfire_internals
- firebase_analytics
- firebase_crashlytics and its platform interface
Also applies to: 355-407
Line range hint 1481-1535: Verify Firebase initialization
The addition of Firebase packages will require proper initialization in the app. Ensure that Firebase is initialized before using Crashlytics or Analytics.
✅ Verification successful
Firebase is properly initialized and configured ✅
The codebase shows correct Firebase initialization with proper error handling:
- Firebase is initialized with platform-specific options
- Crashlytics is configured to capture both Flutter fatal errors and uncaught asynchronous errors
- Error handlers are properly set up for comprehensive crash reporting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Firebase initialization code
# Look for Firebase initialization in main.dart or similar entry points
echo "Checking for Firebase initialization..."
rg -l "Firebase.initializeApp|await Firebase.initializeApp" --type dart
# Check for Crashlytics initialization
echo "Checking for Crashlytics initialization..."
rg -l "FirebaseCrashlytics.instance" --type dart
Length of output: 342
Script:
#!/bin/bash
# Let's examine the actual initialization code in main.dart
echo "Firebase initialization code in main.dart:"
rg -A 5 "Firebase.initializeApp" app/lib/main.dart
echo -e "\nCrashlytics initialization code in main.dart:"
rg -A 5 "FirebaseCrashlytics.instance" app/lib/main.dart
Length of output: 985
379-379: Verify version compatibility
Several package versions have been updated. While the updates themselves look good, it's important to ensure compatibility across the dependency tree, especially with the newly added Firebase packages.
Also applies to: 386-386, 393-393, 499-499, 555-555, 910-910, 938-938, 1089-1089, 1146-1146, 1266-1266, 1281-1281, 1317-1317, 1331-1331, 1359-1359
✅ Verification successful
Let me gather more information about the dependency versions from the pubspec files to ensure compatibility.
Let me check the actual Firebase usage in the main.dart file to understand the integration better.
Firebase packages are compatible and properly integrated
The Firebase packages are being used correctly in the application:
firebase_core3.8.1 matches the latest stable version- Firebase is properly initialized in
main.dartwith crash reporting configured - All required Firebase plugins are correctly registered in the GeneratedPluginRegistrant
- The integration follows Firebase best practices with proper error handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential version conflicts and compatibility issues
# Check pubspec.lock for any dependency conflicts
if [ -f "pubspec.lock" ]; then
echo "Checking dependency constraints in pubspec.lock..."
cat pubspec.lock | grep -A 2 "firebase\|flutter_svg\|go_router\|permission_handler\|photo_manager\|shelf_web_socket\|sqflite\|url_launcher\|vector_graphics\|video_player"
fi
# Check if firebase packages are on latest stable versions
echo "Checking latest stable versions of firebase packages..."
for package in firebase_analytics firebase_crashlytics firebase_core; do
curl -s "https://pub.dev/api/packages/$package" | jq -r '.latest.version'
done
Length of output: 621
Script:
#!/bin/bash
# Check both pubspec.yaml and pubspec.lock for dependency versions
echo "Checking pubspec.yaml dependencies..."
if [ -f "pubspec.yaml" ]; then
cat pubspec.yaml | grep -A 1 "firebase_core:\|firebase_analytics:\|firebase_crashlytics:"
fi
echo -e "\nChecking pubspec.lock dependencies..."
if [ -f "pubspec.lock" ]; then
cat pubspec.lock | grep -A 2 "firebase_core:\|firebase_analytics:\|firebase_crashlytics:"
fi
# Check for any firebase-related imports in the codebase
echo -e "\nChecking Firebase usage in the codebase..."
rg "import.*firebase" -l
Length of output: 423
Script:
#!/bin/bash
# Check Firebase initialization and usage in main.dart
echo "Checking Firebase usage in main.dart..."
cat app/lib/main.dart
# Also check the plugin registrant to see Firebase plugin registration
echo -e "\nChecking Firebase plugin registration..."
cat app/macos/Flutter/GeneratedPluginRegistrant.swift
Length of output: 4072
.github/workflows/analyze.yml (1)
34-34: Verify firebase.json location
The firebase.json is being created in the app directory while other Firebase-related files are placed in platform-specific directories. Please verify if this location is correct for Firebase Crashlytics configuration.
✅ Verification successful
Firebase.json location is correctly placed in the app directory
The evidence shows that all workflow files (analyze.yml, android_build.yml, android_deploy.yml, and ios_deploy.yml) consistently place the firebase.json file in the app directory after changing to it with cd app. This is the standard location as shown by the pattern across all Firebase-related file placements:
- Platform-specific files go to their respective directories:
- Android:
android/app/google-services.json - iOS:
ios/Runner/GoogleService-Info.plist,ios/firebase_app_id_file.json
- Android:
- Common Firebase files stay in the app root:
firebase.jsonlib/firebase_options.dart
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other workflow files place firebase.json in the same location
rg -l "firebase.json" .github/workflows/
# Check if any Firebase-related build or configuration files reference this location
fd -t f -e json -e yaml -e xml . | xargs rg "firebase.json"
Length of output: 236
Script:
#!/bin/bash
# Check the content of other workflow files to see how they handle firebase.json
rg -A 5 -B 5 "firebase\.json" .github/workflows/
# Look for Firebase configuration patterns in platform-specific directories
fd -t d "(android|ios)" . | xargs rg -l "firebase"
Length of output: 3821
.github/workflows/android_deploy.yml (1)
42-47: Consider environment-specific Firebase configurations
Since this is the production deployment workflow, verify if you need different Firebase configurations for different environments (development, staging, production).
✅ Verification successful
Let me analyze the workflow files and Firebase-related configurations to verify if there are environment-specific setups.
Let me analyze the workflow file and check for any Firebase-related configuration files in the repository.
Environment-specific Firebase configurations are already properly handled
The workflow is correctly configured for production deployment with dedicated Firebase configuration secrets:
FIREBASE_OPTIONS_BASE64for Flutter Firebase optionsGOOGLE_SERVICES_JSON_BASE64for Android Firebase configurationFIREBASE_JSON_BASE64for general Firebase settings
These secrets are specifically used in the production workflow (triggered on main branch), which is the correct approach for environment separation. Different values can be configured for these secrets without requiring code changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for environment-specific Firebase configurations
gh api repos/{owner}/{repo}/actions/secrets/public | jq -r '.secrets[] | select(.name | contains("FIREBASE"))'
Length of output: 226
Script:
#!/bin/bash
# Look for environment-specific Firebase configuration files or setup
fd -e json -e dart -e yaml -e yml . | rg -i "firebase|google.*services"
# Check workflow files for environment-specific Firebase configurations
fd -e yml -e yaml . .github/workflows/ | xargs cat | rg -i "firebase.*_(dev|staging|prod|test)"
Length of output: 171
Script:
#!/bin/bash
# Check the workflow file content directly
cat .github/workflows/android_deploy.yml
# Look for any Firebase configuration files
fd firebase
fd google-services
Length of output: 3016
🧰 Tools
🪛 actionlint (1.7.4)
43-43: shellcheck reported issue in this script: SC2086:info:2:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:3:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
43-43: shellcheck reported issue in this script: SC2086:info:6:6: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/ios_deploy.yml (2)
37-37: LGTM: Environment variable addition follows established patterns.
The new FIREBASE_JSON_BASE64 environment variable is correctly integrated with the existing configuration.
44-44: Verify Firebase configuration security measures.
While the decoding follows the established pattern, please ensure:
- The
firebase.jsonfile is added to.gitignoreto prevent accidental commits - The workflow cleans up sensitive files after deployment
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/app_links-6.3.2" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/url_launcher_macos-3.2.1" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_android-2.2.12" /> |
There was a problem hiding this comment.
Remove duplicate entry for path_provider_android plugin.
There are two different versions of the same plugin:
- Line 41: path_provider_android-2.2.12
- Line 49: path_provider_android-2.2.14
This could lead to version conflicts. Keep only the latest version (2.2.14).
Also applies to: 49-49
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_android-2.2.12" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/photo_manager-3.6.2" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_foundation-2.4.0" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/shared_preferences_android-2.3.3" /> |
There was a problem hiding this comment.
Remove duplicate entry for shared_preferences_android plugin.
There are two different versions of the same plugin:
- Line 44: shared_preferences_android-2.3.3
- Line 58: shared_preferences_android-2.3.4
This could lead to version conflicts. Keep only the latest version (2.3.4).
Also applies to: 58-58
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/url_launcher_macos-3.2.1" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_android-2.2.12" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/photo_manager-3.6.2" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_foundation-2.4.0" /> |
There was a problem hiding this comment.
Remove duplicate entry for path_provider_foundation plugin.
There are two different versions of the same plugin:
- Line 43: path_provider_foundation-2.4.0
- Line 55: path_provider_foundation-2.4.1
This could lead to version conflicts. Keep only the latest version (2.4.1).
Also applies to: 55-55
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/photo_manager-3.6.2" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/path_provider_foundation-2.4.0" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/shared_preferences_android-2.3.3" /> | ||
| <root url="file://$USER_HOME$/.pub-cache/hosted/pub.dev/url_launcher_ios-6.3.1" /> |
There was a problem hiding this comment.
Remove duplicate entry for url_launcher_ios plugin.
There are two different versions of the same plugin:
- Line 45: url_launcher_ios-6.3.1
- Line 59: url_launcher_ios-6.3.2
This could lead to version conflicts. Keep only the latest version (6.3.2).
Also applies to: 59-59
Summary by CodeRabbit
New Features
Bug Fixes
Chores
pubspec.yamlto include Firebase Crashlytics and Firebase Analytics..gitignoreto exclude Firebase configuration files.