Skip to content
This repository was archived by the owner on Feb 24, 2025. It is now read-only.

Crash report cohort ID support for iOS #3692

Merged
merged 12 commits into from
Dec 19, 2024

Conversation

jdjackson
Copy link
Contributor

@jdjackson jdjackson commented Dec 6, 2024

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). 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)
  3. 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.
  4. 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).

Definition of Done (Internal Only):

Copy Testing: N/A

  • Use of correct apostrophes in new copy, ie rather than

Orientation Testing: N/A

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPhone 15 Pro
  • iPad

OS Testing:

  • iOS 15
  • iOS 16
  • iOS 17
  • iOS 18

Theme Testing: N/A

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

Remove app-level CRCID user setting, and prepare for clearing CRCID owned by BSK (BSK changes still to come).
Oops, clearing CRCID needs to be done through CrashCollection, which now supports this properly.
Support for sending error pixels when crash report submission fails or a CRCID is not returned by the server when expected.
@jdjackson jdjackson marked this pull request as ready for review December 18, 2024 20:42
# By Michal Smaga (10) and others
# Via Michal Smaga (4) and others
* main: (66 commits)
  DuckPlayer: Don’t open new tabs or DuckPlayer at launch when in alwaysAsk mode (#3738)
  Fix BrowsingMenu layout (#3712)
  Remove ESLint config files (#3739)
  Release 7.150.0-1 (#3742)
  An additional protective in case users try to access the passwords list via the extension, before launching the app
  Ensure migration has occurred before accessing vault
  Populate credential store if user has enabled before launching app
  Update to target main app
  Ensure migration has occurred before accessing vault
  Release 7.149.1-0 (#3740)
  An additional protective in case users try to access the passwords list via the extension, before launching the app
  Ensure migration has occurred before accessing vault
  Populate credential store if user has enabled before launching app
  Update to target main app
  Ensure migration has occurred before accessing vault
  Bugfix: Fix Youtube Internal links when 'Watching on Youtube' from Duck Player (#3733)
  point to BSK branch (#3732)
  Remove duck.ai 10min timer (#3731)
  Release 7.150.0-0 (#3729)
  Update autoconsent to v12.3.0 (#3728)
  ...

# Conflicts:
#	Core/PixelEvent.swift
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@jdjackson jdjackson merged commit 8ce3815 into main Dec 19, 2024
13 checks passed
@jdjackson jdjackson deleted the jjackson/crash-report-cohort-support branch December 19, 2024 21:25
samsymons added a commit that referenced this pull request Dec 19, 2024
* main:
  Crash report cohort ID support for iOS (#3692)
  Fix sharing via the Share & Action extensions on iOS 18 (#3745)
  DuckPlayer: Don’t open new tabs or DuckPlayer at launch when in alwaysAsk mode (#3738)
  Fix BrowsingMenu layout (#3712)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants