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

Conversation

@diegoreymendez
Copy link
Contributor

Task/Issue URL: https://app.asana.com/0/1199230911884351/1205324932335850/f

Description

Standardizes the TDS loading error handling to avoid uninformative pixel spikes.

Testing

There's not a lot to test here. Ensure the code matches our iOS implementation: https://github.com/duckduckgo/iOS/blob/aecf989b650b751a409abdf92c66fcb0b5828c06/Core/FileStore.swift#L57-L67


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Comment on lines +99 to +104
let nserror = error as NSError

if nserror.domain != NSCocoaErrorDomain || nserror.code != NSFileReadNoSuchFileError {
Pixel.fire(.debug(event: .trackerDataCouldNotBeLoaded, error: error))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've considered to add tests to validate that this code is doing what it should, but the problem is Pixel isn't easy to inject for implementing tests, and I don't want to derail the scope of these changes or introduce complexity to this PR.

@diegoreymendez diegoreymendez marked this pull request as ready for review August 22, 2023 13:26
Copy link
Contributor

@afterxleep afterxleep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@diegoreymendez diegoreymendez merged commit 1e12a38 into develop Aug 22, 2023
@diegoreymendez diegoreymendez deleted the diego/remove-tds-loading-error-noise branch August 22, 2023 14:14
diegoreymendez added a commit that referenced this pull request Aug 23, 2023
Task/Issue URL:
https://app.asana.com/0/1199230911884351/1205324932335850/f

## Description

Standardizes the TDS loading error handling to avoid uninformative pixel
spikes.
samsymons added a commit that referenced this pull request Aug 23, 2023
# By Diego Rey Mendez (7) and others
# Via Sam Symons (2) and others
* develop: (26 commits)
  Improve Sync-related database cleaning logic (#1529)
  Update onboarding-related error states (#1504)
  Prevents launching our menu agent without an auth code. (#1516)
  Autofill UI letter icons (#1535)
  Cleans up some code (#1517)
  Revert "Autofill Letter Icons" (#1534)
  Adds remote pre-commit installer, which includes automatic fix for linter (#1369)
  Autofill Letter Icons (#1475)
  change context menu for mailto links (#1513)
  Updates the version to 1.53.1
  Updated the embedded files for 1.53.1
  Update the phased rollout tester to avoid caching the config (#1520)
  Require Duck Player scheme URL to be passed from YouTube Overlay User Script (#1519)
  Add pixels related to Duck Player usage (#1515)
  only allow error reloads on http(s) urls (#1523)
  Standardize TDS Loading Error handling (#1524)
  Move pixel sender logic into the main view controller (#1528)
  Update the phased rollout tester to avoid caching the config (#1520)
  Set version to 1.52.3.
  Move pixel sender logic into the main view controller (#1528)
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/AppDelegate/AppDelegate.swift
#	DuckDuckGo/Common/Localizables/UserText.swift
#	DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift
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