From 8ce3815b51b25209cdef74eddc5e99d7e7b73dc3 Mon Sep 17 00:00:00 2001 From: Jonathan Jackson Date: Thu, 19 Dec 2024 16:25:37 -0500 Subject: [PATCH] Crash report cohort ID support for iOS (#3692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1208592102886666/1208759541597499/f Tech Design URL: https://app.asana.com/0/1208592102886666/1208660326715650/f **Description**: DO NOT MERGE - this is a draft for input, not ready to go live yet. iOS client support for CRCID send/receive (primarily supported in BSK, with changes under review in [BSK #1116](https://github.com/duckduckgo/BrowserServicesKit/pull/1116)). This is pretty straightforward, just conforming to CrashCollection’s new init signature, and clearing CRCIDs when the user opts out of crash reporting. BSK handles everything else. **Steps to test this PR**: Note: Must be tested on a physical device, as the simulator does not produce crash logs (and thus doesn’t find and upload them either). To cause and report a crash: 1. Launch the app and force a crash, which can be done from Settings → All Debug Options → Crash (fatal error) or similar. Note that Crash (CPU/Memory) does not appear to produce a crash log, and thus won’t trigger crash uploading. 2. Launch the app again (easiest with a debugger) 1. For the first crash of an app install: You will be prompted to opt in or out of crash reporting when the app is launched. Opt in and watch logs for “crcid” and you should see logs from CrashReportSender:56, and CrashCollection:95-109. 2. On subsequent crashes, when opted in, you should see statements confirming the received crcid was sent, and that the server returned either the same matching one, or a new one (in which case the new one should be stored and used on subsequent crash reports) To test clearing of the crcid when opting out: 1. Navigate to Settings → About and switch “Send Crash Reports” off, then back on again (this step should clear the crcid) 2. Follow steps from “To cause and report a crash” above, and confirm that the crash is submitted without an initial crcid, and that the server assigns one and it is stored (causing and uploading a second crash should confirm this new value is used on send). --- Core/PixelEvent.swift | 5 +++ DuckDuckGo.xcodeproj/project.pbxproj | 6 ++- .../xcshareddata/swiftpm/Package.resolved | 4 +- DuckDuckGo/AppDelegate.swift | 4 +- DuckDuckGo/AppUserDefaults.swift | 2 +- DuckDuckGo/CrashCollectionOnboarding.swift | 3 ++ .../CrashCollectionOnboardingViewModel.swift | 6 +++ DuckDuckGo/CrashReportSenderExtensions.swift | 40 +++++++++++++++++++ DuckDuckGo/SettingsViewModel.swift | 5 +++ DuckDuckGoTests/AppSettingsMock.swift | 1 - 10 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 DuckDuckGo/CrashReportSenderExtensions.swift diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 18c6b0b761..0adf763dec 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -526,6 +526,9 @@ extension Pixel { case dbCrashDetectedDaily case crashOnCrashHandlersSetUp + case crashReportCRCIDMissing + case crashReportingSubmissionFailed + case dbMigrationError case dbRemovalError case dbDestroyError @@ -1456,6 +1459,8 @@ extension Pixel.Event { case .dbCrashDetected: return "m_d_crash" case .dbCrashDetectedDaily: return "m_d_crash_daily" + case .crashReportCRCIDMissing: return "m_crashreporting_crcid-missing" + case .crashReportingSubmissionFailed: return "m_crashreporting_submission-failed" case .crashOnCrashHandlersSetUp: return "m_d_crash_on_handlers_setup" case .dbMigrationError: return "m_d_dbme" case .dbRemovalError: return "m_d_dbre" diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index ed776495b7..91b6c160d0 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -225,6 +225,7 @@ 37FCAABC2992F592000E420A /* MultilineScrollableTextFix.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FCAABB2992F592000E420A /* MultilineScrollableTextFix.swift */; }; 37FCAAC029930E26000E420A /* FailedAssertionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FCAABF29930E26000E420A /* FailedAssertionView.swift */; }; 37FD780F2A29E28B00B36DB1 /* SyncErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FD780E2A29E28B00B36DB1 /* SyncErrorHandler.swift */; }; + 46DD3D5A2D0A29F600F33D49 /* CrashReportSenderExtensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 46DD3D592D0A29F400F33D49 /* CrashReportSenderExtensions.swift */; }; 4B0295192537BC6700E00CEF /* ConfigurationDebugViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B0295182537BC6700E00CEF /* ConfigurationDebugViewController.swift */; }; 4B0F3F502B9BFF2100392892 /* NetworkProtectionFAQView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B0F3F4F2B9BFF2100392892 /* NetworkProtectionFAQView.swift */; }; 4B274F602AFEAECC003F0745 /* NetworkProtectionWidgetRefreshModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B274F5F2AFEAECC003F0745 /* NetworkProtectionWidgetRefreshModel.swift */; }; @@ -1597,6 +1598,7 @@ 37FCAABF29930E26000E420A /* FailedAssertionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FailedAssertionView.swift; sourceTree = ""; }; 37FCAACB2993149A000E420A /* Waitlist */ = {isa = PBXFileReference; lastKnownFileType = wrapper; path = Waitlist; sourceTree = ""; }; 37FD780E2A29E28B00B36DB1 /* SyncErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncErrorHandler.swift; sourceTree = ""; }; + 46DD3D592D0A29F400F33D49 /* CrashReportSenderExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReportSenderExtensions.swift; sourceTree = ""; }; 4B0295182537BC6700E00CEF /* ConfigurationDebugViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationDebugViewController.swift; sourceTree = ""; }; 4B0F3F4F2B9BFF2100392892 /* NetworkProtectionFAQView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionFAQView.swift; sourceTree = ""; }; 4B274F5F2AFEAECC003F0745 /* NetworkProtectionWidgetRefreshModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionWidgetRefreshModel.swift; sourceTree = ""; }; @@ -3888,6 +3890,7 @@ 37CF915E2BB4735F00BADCAE /* Crashes */ = { isa = PBXGroup; children = ( + 46DD3D592D0A29F400F33D49 /* CrashReportSenderExtensions.swift */, 37CF915F2BB4737300BADCAE /* CrashCollectionOnboarding.swift */, 37CF91612BB474AA00BADCAE /* CrashCollectionOnboardingView.swift */, 37CF91632BB4A82A00BADCAE /* CrashCollectionOnboardingViewModel.swift */, @@ -8235,6 +8238,7 @@ BDF8D0022C1B87F4003E3B27 /* NetworkProtectionDNSSettingsViewModel.swift in Sources */, 9838059F2228208E00385F1A /* PositiveFeedbackViewController.swift in Sources */, 8590CB67268A2E520089F6BF /* RootDebugViewController.swift in Sources */, + 46DD3D5A2D0A29F600F33D49 /* CrashReportSenderExtensions.swift in Sources */, 1DEAADEA2BA4539800E25A97 /* SettingsAppearanceView.swift in Sources */, B623C1C22862CA9E0043013E /* DownloadSession.swift in Sources */, 317CA3432CFF82E100F88848 /* SettingsAIChatView.swift in Sources */, @@ -11719,7 +11723,7 @@ repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 221.3.0; + version = 222.1.0; }; }; 9F8FE9472BAE50E50071E372 /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 4eaad818db..5f7e2bbb19 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/DuckDuckGo/BrowserServicesKit", "state" : { - "revision" : "b71ed70ce9b0ef3ce51d4f96da0193ab70493944", - "version" : "221.3.0" + "revision" : "5704d77e3b4c77c7387518d796d31a35f7a1ffcf", + "version" : "222.1.0" } }, { diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 38ff685038..c176b05ae0 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -83,7 +83,9 @@ import os.log private var syncStateCancellable: AnyCancellable? private var isSyncInProgressCancellable: AnyCancellable? - private let crashCollection = CrashCollection(platform: .iOS) + private let crashCollection = CrashCollection(crashReportSender: CrashReportSender(platform: .iOS, + pixelEvents: CrashReportSender.pixelEvents), + crashCollectionStorage: UserDefaults()) private var crashReportUploaderOnboarding: CrashCollectionOnboarding? private var autofillPixelReporter: AutofillPixelReporter? diff --git a/DuckDuckGo/AppUserDefaults.swift b/DuckDuckGo/AppUserDefaults.swift index 2c17e2ac1e..df880d42d6 100644 --- a/DuckDuckGo/AppUserDefaults.swift +++ b/DuckDuckGo/AppUserDefaults.swift @@ -76,7 +76,7 @@ public class AppUserDefaults: AppSettings { static let crashCollectionOptInStatus = "com.duckduckgo.ios.crashCollectionOptInStatus" static let crashCollectionShouldRevertOptedInStatusTrigger = "com.duckduckgo.ios.crashCollectionShouldRevertOptedInStatusTrigger" - + static let duckPlayerMode = "com.duckduckgo.ios.duckPlayerMode" static let duckPlayerAskModeOverlayHidden = "com.duckduckgo.ios.duckPlayerAskModeOverlayHidden" static let duckPlayerOpenInNewTab = "com.duckduckgo.ios.duckPlayerOpenInNewTab" diff --git a/DuckDuckGo/CrashCollectionOnboarding.swift b/DuckDuckGo/CrashCollectionOnboarding.swift index dd9e54af7c..7dac3c6265 100644 --- a/DuckDuckGo/CrashCollectionOnboarding.swift +++ b/DuckDuckGo/CrashCollectionOnboarding.swift @@ -55,9 +55,12 @@ final class CrashCollectionOnboarding: NSObject { func presentOnboardingIfNeeded(for payloads: [Data], from viewController: UIViewController, sendReport: @escaping () -> Void) { let isCurrentlyPresenting = viewController.presentedViewController != nil + // Note: DO NOT TURN THIS ON until updated screens for the opt-in prompt and screen for reviewing the kinds of data + // we collect are updated (project coming soon) if featureFlagger.isFeatureOn(.crashReportOptInStatusResetting) { if appSettings.crashCollectionOptInStatus == .optedIn && appSettings.crashCollectionShouldRevertOptedInStatusTrigger < crashCollectionShouldRevertOptedInStatusTriggerTargetValue { + appSettings.crashCollectionOptInStatus = .undetermined appSettings.crashCollectionShouldRevertOptedInStatusTrigger = crashCollectionShouldRevertOptedInStatusTriggerTargetValue } diff --git a/DuckDuckGo/CrashCollectionOnboardingViewModel.swift b/DuckDuckGo/CrashCollectionOnboardingViewModel.swift index bd641f257b..5badf39bee 100644 --- a/DuckDuckGo/CrashCollectionOnboardingViewModel.swift +++ b/DuckDuckGo/CrashCollectionOnboardingViewModel.swift @@ -19,6 +19,7 @@ import Foundation import SwiftUI +import Crashes final class CrashCollectionOnboardingViewModel: ObservableObject { @@ -106,6 +107,11 @@ final class CrashCollectionOnboardingViewModel: ObservableObject { } set { appSettings.crashCollectionOptInStatus = newValue + if appSettings.crashCollectionOptInStatus == .optedOut { + let crashCollection = CrashCollection.init(crashReportSender: CrashReportSender(platform: .iOS, + pixelEvents: CrashReportSender.pixelEvents)) + crashCollection.clearCRCID() + } } } } diff --git a/DuckDuckGo/CrashReportSenderExtensions.swift b/DuckDuckGo/CrashReportSenderExtensions.swift new file mode 100644 index 0000000000..a90e8d54da --- /dev/null +++ b/DuckDuckGo/CrashReportSenderExtensions.swift @@ -0,0 +1,40 @@ +// +// CrashReportSenderExtensions.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Crashes +import Common +import Core + +extension CrashReportSender { + + static let pixelEvents: EventMapping = .init { event, _, _, _ in + switch event { + case CrashReportSenderError.crcidMissing: + Pixel.fire(pixel: .crashReportCRCIDMissing) + + case CrashReportSenderError.submissionFailed(let error): + if let error { + Pixel.fire(pixel: .crashReportingSubmissionFailed, + withAdditionalParameters: ["HTTPStatusCode": "\(error.statusCode)"]) + } else { + Pixel.fire(pixel: .crashReportingSubmissionFailed) + } + } + } +} diff --git a/DuckDuckGo/SettingsViewModel.swift b/DuckDuckGo/SettingsViewModel.swift index cfdc9ae43c..224f880c2c 100644 --- a/DuckDuckGo/SettingsViewModel.swift +++ b/DuckDuckGo/SettingsViewModel.swift @@ -25,6 +25,7 @@ import Common import Combine import SyncUI import DuckPlayer +import Crashes import Subscription import NetworkProtection @@ -377,6 +378,10 @@ final class SettingsViewModel: ObservableObject { Binding( get: { self.state.crashCollectionOptInStatus == .optedIn }, set: { + if self.appSettings.crashCollectionOptInStatus == .optedIn && $0 == false { + let crashCollection = CrashCollection(crashReportSender: CrashReportSender(platform: .iOS, pixelEvents: CrashReportSender.pixelEvents)) + crashCollection.clearCRCID() + } self.appSettings.crashCollectionOptInStatus = $0 ? .optedIn : .optedOut self.state.crashCollectionOptInStatus = $0 ? .optedIn : .optedOut } diff --git a/DuckDuckGoTests/AppSettingsMock.swift b/DuckDuckGoTests/AppSettingsMock.swift index 13ced3eb65..bfd5fff474 100644 --- a/DuckDuckGoTests/AppSettingsMock.swift +++ b/DuckDuckGoTests/AppSettingsMock.swift @@ -82,7 +82,6 @@ class AppSettingsMock: AppSettings { var autoconsentEnabled = true var crashCollectionOptInStatus: CrashCollectionOptInStatus = .undetermined - var crashCollectionShouldRevertOptedInStatusTrigger: Int = 0 var newTabPageSectionsEnabled: Bool = false