Skip to content

PIR: Add additional pixels and params for download errors#7894

Open
karlenDimla wants to merge 2 commits intodevelopfrom
fix/karl/pir/additional-pixels-download
Open

PIR: Add additional pixels and params for download errors#7894
karlenDimla wants to merge 2 commits intodevelopfrom
fix/karl/pir/additional-pixels-download

Conversation

@karlenDimla
Copy link
Contributor

@karlenDimla karlenDimla commented Mar 6, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/task/1213531646709003?focus=true

Description

Add additional params to download errors
Separate pixel for main config and json errors

Steps to test this PR

https://app.asana.com/1/137249556945/task/1213552181080627?focus=true


Note

Medium Risk
Medium risk because it changes PirPixelSender method signatures (now suspend) and wiring/DI to always derive VPN state from NetworkProtectionState, which can break call sites or affect pixel emission timing if any integration is missed.

Overview
Adds more granular PIR download error telemetry by introducing a dedicated m_dbp_download_broker-json_failure pixel and by attaching vpn_connection_state to main-config download failure pixels.

Updates the PIR pixel-sending pipeline so VPN state is computed inside RealPirPixelSender (via injected NetworkProtectionState) rather than being passed through, and adjusts download/update flows (BrokerJsonUpdater) to emit the new broker-json failure pixel when broker JSON downloads fail without advancing etags.

Tests and integration setup are updated to match the new constructor dependencies and the suspend pixel-reporting APIs.

Written by Cursor Bugbot for commit c94b9be. This will update automatically on new commits. Configure here.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Privacy Review task: https://app.asana.com/0/69071770703008/1213521097314010

@karlenDimla karlenDimla force-pushed the fix/karl/pir/additional-pixels-download branch 2 times, most recently from 2c6dead to 0964c16 Compare March 6, 2026 15:40
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Inner runCatching swallows CancellationException from suspend function
    • Added explicit check to rethrow CancellationException in the onFailure block before other error handling, ensuring proper coroutine cancellation propagation.

Create PR

Or push these changes by commenting:

@cursor push 95a8946f0d
Preview (95a8946f0d)
diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/brokers/BrokerJsonUpdater.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/brokers/BrokerJsonUpdater.kt
--- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/brokers/BrokerJsonUpdater.kt
+++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/brokers/BrokerJsonUpdater.kt
@@ -24,6 +24,7 @@
 import com.duckduckgo.pir.impl.store.PirRepository
 import com.duckduckgo.pir.impl.store.PirRepository.BrokerJson
 import com.squareup.anvil.annotations.ContributesBinding
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.withContext
 import logcat.LogPriority.ERROR
 import logcat.asLog
@@ -77,6 +78,7 @@
                                 pirRepository.updateMainEtag(config.etag)
                             }
                             .onFailure { e ->
+                                if (e is CancellationException) throw e
                                 logcat(ERROR) { "PIR-update: Failed to download broker json files: $e" }
                                 val message = e.asLog().sanitize() ?: e.message ?: "Unknown error"
                                 pixelSender.reportDownloadBrokerJsonFailure(message)

@karlenDimla karlenDimla force-pushed the fix/karl/pir/additional-pixels-download branch from 0964c16 to 9bfade9 Compare March 6, 2026 15:52
@karlenDimla karlenDimla force-pushed the fix/karl/pir/additional-pixels-download branch from 9bfade9 to c94b9be Compare March 6, 2026 16:00
Copy link
Contributor

@landomen landomen left a comment

Choose a reason for hiding this comment

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

Works as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants