BIT-941: Set up Crashlytics for crash logs and non-fatal error reporting (#118)

This commit is contained in:
Matt Czech 2023-11-02 14:59:33 -05:00 committed by GitHub
parent 6d2f5e99f3
commit e0a384787f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 205 additions and 15 deletions

View File

@ -7,6 +7,9 @@ import UIKit
protocol AppDelegateType: AnyObject {
/// The processor that manages application level logic.
var appProcessor: AppProcessor? { get }
/// Whether the app is running for unit tests.
var isTesting: Bool { get }
}
/// The app's `UIApplicationDelegate` which serves as the entry point into the app.
@ -17,13 +20,27 @@ class AppDelegate: UIResponder, UIApplicationDelegate, AppDelegateType {
/// The processor that manages application level logic.
var appProcessor: AppProcessor?
/// Whether the app is running for unit tests.
var isTesting: Bool {
ProcessInfo.processInfo.arguments.contains("-testing")
}
// MARK: Methods
func application(
_ application: UIApplication,
didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? = nil
) -> Bool {
let services = ServiceContainer()
// Exit early if testing to avoid running any app functionality.
guard !isTesting else { return true }
#if DEBUG
let errorReporter = OSLogErrorReporter()
#else
let errorReporter = CrashlyticsErrorReporter()
#endif
let services = ServiceContainer(errorReporter: errorReporter)
let appModule = DefaultAppModule(services: services)
appProcessor = AppProcessor(appModule: appModule, services: services)
return true

View File

@ -0,0 +1,30 @@
import BitwardenShared
import FirebaseCore
import FirebaseCrashlytics
/// An `ErrorReporter` that logs non-fatal errors to Crashlytics for investigation.
///
final class CrashlyticsErrorReporter: ErrorReporter {
// MARK: ErrorReporter Properties
var isEnabled: Bool {
get { Crashlytics.crashlytics().isCrashlyticsCollectionEnabled() }
set {
Crashlytics.crashlytics().setCrashlyticsCollectionEnabled(newValue)
}
}
// MARK: Initialization
/// Initialize the `CrashlyticsErrorReporter`.
///
init() {
FirebaseApp.configure()
}
// MARK: ErrorReporter
func log(error: Error) {
Crashlytics.crashlytics().record(error: error)
}
}

View File

@ -0,0 +1,26 @@
import BitwardenShared
import OSLog
/// An `ErrorReporter` that logs non-fatal errors to the console via OSLog.
///
final class OSLogErrorReporter: ErrorReporter {
// MARK: Properties
/// The logger instance to log local messages.
let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "ErrorReporter")
// MARK: ErrorReporter Properties
/// This exists here satisfy the `ErrorReporter` protocol, but doesn't do anything since we
/// don't report these errors to an external service.
var isEnabled = true
// MARK: ErrorReporter
func log(error: Error) {
logger.error("Error: \(error)")
// Crash in debug builds to make the error more visible during development.
assertionFailure("Unexpected error: \(error)")
}
}

View File

@ -1,4 +1,5 @@
import BitwardenShared
import SwiftUI
import UIKit
class SceneDelegate: UIResponder, UIWindowSceneDelegate {
@ -14,9 +15,21 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
willConnectTo session: UISceneSession,
options connectionOptions: UIScene.ConnectionOptions
) {
guard let appProcessor = (UIApplication.shared.delegate as? AppDelegateType)?.appProcessor,
let windowScene = scene as? UIWindowScene
else {
guard let windowScene = scene as? UIWindowScene else { return }
guard let appProcessor = (UIApplication.shared.delegate as? AppDelegateType)?.appProcessor else {
if (UIApplication.shared.delegate as? AppDelegateType)?.isTesting == true {
// If the app is running tests, show a testing view.
window = UIWindow(windowScene: windowScene)
window?.makeKeyAndVisible()
window?.rootViewController = UIHostingController(rootView: ZStack {
Color("backgroundSplash").ignoresSafeArea()
Image("logoBitwarden")
.resizable()
.scaledToFit()
.frame(width: 238)
})
}
return
}

View File

@ -29,7 +29,10 @@ class SceneDelegateTests: BitwardenTestCase {
/// `scene(_:willConnectTo:options:)` with a `UIWindowScene` creates the app's UI.
func test_sceneWillConnectTo_withWindowScene() throws {
let appProcessor = AppProcessor(appModule: appModule, services: ServiceContainer())
let appProcessor = AppProcessor(
appModule: appModule,
services: ServiceContainer(errorReporter: MockErrorReporter())
)
(UIApplication.shared.delegate as? TestingAppDelegate)?.appProcessor = appProcessor
let session = TestInstanceFactory.create(UISceneSession.self)
@ -46,7 +49,10 @@ class SceneDelegateTests: BitwardenTestCase {
/// `scene(_:willConnectTo:options:)` without a `UIWindowScene` fails to create the app's UI.
func test_sceneWillConnectTo_withNonWindowScene() throws {
let appProcessor = AppProcessor(appModule: appModule, services: ServiceContainer())
let appProcessor = AppProcessor(
appModule: appModule,
services: ServiceContainer(errorReporter: MockErrorReporter())
)
(UIApplication.shared.delegate as? TestingAppDelegate)?.appProcessor = appProcessor
let session = TestInstanceFactory.create(UISceneSession.self)

View File

@ -8,4 +8,5 @@ import UIKit
///
class TestingAppDelegate: NSObject, UIApplicationDelegate, AppDelegateType {
var appProcessor: AppProcessor?
var isTesting = false
}

View File

@ -0,0 +1,32 @@
import Foundation
/// A type used to construct errors that are reported to the error reporter.
///
/// Each type of error should have a unique `code`. Non-fatal errors in Crashlytics are grouped by
/// the `domain` and `code`.
///
enum BitwardenError {
// MARK: Types
/// An error code for the error.
///
enum Code: Int {
case logoutError = 1000
}
// MARK: Errors
/// An error that occurred during logout.
///
/// - Parameter error: The underlying error that caused the logout error.
///
static func logoutError(error: Error) -> NSError {
NSError(
domain: "Logout Error",
code: Code.logoutError.rawValue,
userInfo: [
NSUnderlyingErrorKey: error,
]
)
}
}

View File

@ -0,0 +1,16 @@
/// A protocol for a service that can report non-fatal errors for investigation.
///
public protocol ErrorReporter: AnyObject {
// MARK: Properties
/// Whether collecting non-fatal errors and crash reports is enabled.
var isEnabled: Bool { get set }
// MARK: Methods
/// Logs an error to be reported.
///
/// - Parameter error: The error to log.
///
func log(error: Error)
}

View File

@ -36,6 +36,9 @@ public class ServiceContainer: Services {
/// The service used by the application to handle encryption and decryption tasks.
let clientService: ClientService
/// The service used by the application to report non-fatal errors.
let errorReporter: ErrorReporter
/// The repository used by the application to manage generator data for the UI layer.
let generatorRepository: GeneratorRepository
@ -65,6 +68,7 @@ public class ServiceContainer: Services {
/// - baseUrlService: The service used by the application to retrieve the current base url for API requests.
/// - captchaService: The service used by the application to create captcha related artifacts.
/// - clientService: The service used by the application to handle encryption and decryption tasks.
/// - errorReporter: The service used by the application to report non-fatal errors.
/// - generatorRepository: The repository used by the application to manage generator data for the UI layer.
/// - settingsRepository: The repository used by the application to manage data for the UI layer.
/// - stateService: The service used by the application to manage account state.
@ -79,6 +83,7 @@ public class ServiceContainer: Services {
baseUrlService: BaseUrlService,
captchaService: CaptchaService,
clientService: ClientService,
errorReporter: ErrorReporter,
generatorRepository: GeneratorRepository,
settingsRepository: SettingsRepository,
stateService: StateService,
@ -92,6 +97,7 @@ public class ServiceContainer: Services {
self.baseUrlService = baseUrlService
self.captchaService = captchaService
self.clientService = clientService
self.errorReporter = errorReporter
self.generatorRepository = generatorRepository
self.settingsRepository = settingsRepository
self.stateService = stateService
@ -104,7 +110,9 @@ public class ServiceContainer: Services {
/// A convenience initializer to initialize the `ServiceContainer` with the default services.
///
public convenience init() {
/// - Parameter errorReporter: The service used by the application to report non-fatal errors.
///
public convenience init(errorReporter: ErrorReporter) {
let appSettingsStore = DefaultAppSettingsStore(
userDefaults: UserDefaults(suiteName: Bundle.main.groupIdentifier)!
)
@ -136,6 +144,7 @@ public class ServiceContainer: Services {
baseUrlService: baseUrlService,
captchaService: DefaultCaptchaService(baseUrlService: baseUrlService),
clientService: clientService,
errorReporter: errorReporter,
generatorRepository: generatorRepository,
settingsRepository: settingsRepository,
stateService: stateService,

View File

@ -10,6 +10,7 @@ typealias Services = HasAccountAPIService
& HasCaptchaService
& HasClientAuth
& HasDeviceAPIService
& HasErrorReporter
& HasGeneratorRepository
& HasSettingsRepository
& HasStateService
@ -84,6 +85,13 @@ protocol HasDeviceAPIService {
var deviceAPIService: DeviceAPIService { get }
}
/// Protocol for an object that provides an `ErrorReporter`.
///
protocol HasErrorReporter {
/// The service used by the application to report non-fatal errors.
var errorReporter: ErrorReporter { get }
}
/// Protocol for an object that provides a `GeneratorRepository`.
///
protocol HasGeneratorRepository {

View File

@ -10,6 +10,7 @@ extension ServiceContainer {
baseUrlService: BaseUrlService = DefaultBaseUrlService(baseUrl: .example),
captchaService: CaptchaService = MockCaptchaService(),
clientService: ClientService = MockClientService(),
errorReporter: ErrorReporter = MockErrorReporter(),
generatorRepository: GeneratorRepository = MockGeneratorRepository(),
httpClient: HTTPClient = MockHTTPClient(),
settingsRepository: SettingsRepository = MockSettingsRepository(),
@ -28,6 +29,7 @@ extension ServiceContainer {
baseUrlService: baseUrlService,
captchaService: captchaService,
clientService: clientService,
errorReporter: errorReporter,
generatorRepository: generatorRepository,
settingsRepository: settingsRepository,
stateService: stateService,

View File

@ -28,6 +28,7 @@ internal final class AuthCoordinator: NSObject, Coordinator, HasStackNavigator {
& HasCaptchaService
& HasClientAuth
& HasDeviceAPIService
& HasErrorReporter
& HasStateService
& HasSystemDevice

View File

@ -6,6 +6,7 @@ class VaultUnlockProcessor: StateProcessor<VaultUnlockState, VaultUnlockAction,
// MARK: Types
typealias Services = HasAuthRepository
& HasErrorReporter
// MARK: Private Properties
@ -74,9 +75,7 @@ class VaultUnlockProcessor: StateProcessor<VaultUnlockState, VaultUnlockAction,
do {
try await self.services.authRepository.logout()
} catch {
// TODO: BIT-941 Log error to Crashlytics.
Logger.processor.error("Error logging out: \(error)")
assertionFailure("Error logging out: \(error)")
self.services.errorReporter.log(error: BitwardenError.logoutError(error: error))
}
self.coordinator.navigate(to: .landing)
}

View File

@ -5,7 +5,8 @@ import OSLog
final class SettingsProcessor: StateProcessor<SettingsState, SettingsAction, Void> {
// MARK: Types
typealias Services = HasSettingsRepository
typealias Services = HasErrorReporter
& HasSettingsRepository
// MARK: Private Properties
@ -52,9 +53,7 @@ final class SettingsProcessor: StateProcessor<SettingsState, SettingsAction, Voi
do {
try await self.services.settingsRepository.logout()
} catch {
// TODO: BIT-941 Log error to Crashlytics.
Logger.processor.error("Error logging out: \(error)")
assertionFailure("Error logging out: \(error)")
self.services.errorReporter.log(error: BitwardenError.logoutError(error: error))
}
self.coordinator.navigate(to: .logout)
}

View File

@ -16,7 +16,8 @@ public protocol SettingsCoordinatorDelegate: AnyObject {
final class SettingsCoordinator: Coordinator, HasStackNavigator {
// MARK: Types
typealias Services = HasSettingsRepository
typealias Services = HasErrorReporter
& HasSettingsRepository
// MARK: Properties

View File

@ -0,0 +1,10 @@
@testable import BitwardenShared
class MockErrorReporter: ErrorReporter {
var errors = [Error]()
var isEnabled = false
func log(error: Error) {
errors.append(error)
}
}

View File

@ -0,0 +1,10 @@
#!/bin/bash
set -euo pipefail
if [ "$CONFIGURATION" != "Debug" ]; then
find "${DWARF_DSYM_FOLDER_PATH}" -name "*.dSYM" \
-exec "${BUILD_DIR%Build/*}/SourcePackages/checkouts/firebase-ios-sdk/Crashlytics/upload-symbols" \
-gsp "${PROJECT_DIR}/Bitwarden/Application/Support/GoogleService-Info.plist" \
-p ios -- {} +
fi

View File

@ -36,6 +36,8 @@ schemes:
Bitwarden: all
BitwardenTests: [test]
test:
commandLineArguments:
"-testing": true
environmentVariables:
TZ: UTC
gatherCoverageData: true
@ -85,6 +87,8 @@ schemes:
BitwardenShared: all
BitwardenSharedTests: [test]
test:
commandLineArguments:
"-testing": true
environmentVariables:
TZ: UTC
gatherCoverageData: true
@ -145,6 +149,12 @@ targets:
- script: Scripts/update_settings_version_number.sh
name: "Settings.bundle: Update Version Number"
basedOnDependencyAnalysis: false
- path: Scripts/firebase_crashlytics_run.sh
name: "Run Firebase Crashlytics"
basedOnDependencyAnalysis: false
inputFiles:
- ${DWARF_DSYM_FOLDER_PATH}/${DWARF_DSYM_FILE_NAME}/Contents/Resources/DWARF/${TARGET_NAME}
- ${BUILT_PRODUCTS_DIR}/${INFOPLIST_PATH}
BitwardenTests:
type: bundle.unit-test
platform: iOS