Skip to content

Commit

Permalink
Crash report cohort ID support for iOS (#3692)
Browse files Browse the repository at this point in the history
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](duckduckgo/BrowserServicesKit#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).
  • Loading branch information
jdjackson authored Dec 19, 2024
1 parent df948c9 commit 8ce3815
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 6 deletions.
5 changes: 5 additions & 0 deletions Core/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,9 @@ extension Pixel {
case dbCrashDetectedDaily
case crashOnCrashHandlersSetUp

case crashReportCRCIDMissing
case crashReportingSubmissionFailed

case dbMigrationError
case dbRemovalError
case dbDestroyError
Expand Down Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -1597,6 +1598,7 @@
37FCAABF29930E26000E420A /* FailedAssertionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FailedAssertionView.swift; sourceTree = "<group>"; };
37FCAACB2993149A000E420A /* Waitlist */ = {isa = PBXFileReference; lastKnownFileType = wrapper; path = Waitlist; sourceTree = "<group>"; };
37FD780E2A29E28B00B36DB1 /* SyncErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncErrorHandler.swift; sourceTree = "<group>"; };
46DD3D592D0A29F400F33D49 /* CrashReportSenderExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReportSenderExtensions.swift; sourceTree = "<group>"; };
4B0295182537BC6700E00CEF /* ConfigurationDebugViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationDebugViewController.swift; sourceTree = "<group>"; };
4B0F3F4F2B9BFF2100392892 /* NetworkProtectionFAQView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionFAQView.swift; sourceTree = "<group>"; };
4B274F5F2AFEAECC003F0745 /* NetworkProtectionWidgetRefreshModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkProtectionWidgetRefreshModel.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3888,6 +3890,7 @@
37CF915E2BB4735F00BADCAE /* Crashes */ = {
isa = PBXGroup;
children = (
46DD3D592D0A29F400F33D49 /* CrashReportSenderExtensions.swift */,
37CF915F2BB4737300BADCAE /* CrashCollectionOnboarding.swift */,
37CF91612BB474AA00BADCAE /* CrashCollectionOnboardingView.swift */,
37CF91632BB4A82A00BADCAE /* CrashCollectionOnboardingViewModel.swift */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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" */ = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
},
{
Expand Down
4 changes: 3 additions & 1 deletion DuckDuckGo/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/AppUserDefaults.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions DuckDuckGo/CrashCollectionOnboarding.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 6 additions & 0 deletions DuckDuckGo/CrashCollectionOnboardingViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import Foundation
import SwiftUI
import Crashes

final class CrashCollectionOnboardingViewModel: ObservableObject {

Expand Down Expand Up @@ -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()
}
}
}
}
40 changes: 40 additions & 0 deletions DuckDuckGo/CrashReportSenderExtensions.swift
Original file line number Diff line number Diff line change
@@ -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<CrashReportSenderError> = .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)
}
}
}
}
5 changes: 5 additions & 0 deletions DuckDuckGo/SettingsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import Common
import Combine
import SyncUI
import DuckPlayer
import Crashes

import Subscription
import NetworkProtection
Expand Down Expand Up @@ -377,6 +378,10 @@ final class SettingsViewModel: ObservableObject {
Binding<Bool>(
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
}
Expand Down
1 change: 0 additions & 1 deletion DuckDuckGoTests/AppSettingsMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ class AppSettingsMock: AppSettings {
var autoconsentEnabled = true

var crashCollectionOptInStatus: CrashCollectionOptInStatus = .undetermined

var crashCollectionShouldRevertOptedInStatusTrigger: Int = 0

var newTabPageSectionsEnabled: Bool = false
Expand Down

0 comments on commit 8ce3815

Please sign in to comment.