-
Notifications
You must be signed in to change notification settings - Fork 432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Release PR] Update VPN metadata reporter #2808
Conversation
Also, add underlying errors.
case .overslept: return "overslept" | ||
case .noNetworkAvailable: return "noNetworkAvailable" | ||
case .unrecoverableNetworkChange: return "unrecoverableNetworkChange" | ||
case .configurationFailed: return "configurationFailed" | ||
case .serverAddressResolutionFailed: return "serverAddressResolutionFailed" | ||
case .serverNotResponding: return "serverNotResponding" | ||
case .serverDead: return "serverDead" | ||
case .authenticationFailed: return "authenticationFailed" | ||
case .clientCertificateInvalid: return "clientCertificateInvalid" | ||
case .clientCertificateNotYetValid: return "clientCertificateNotYetValid" | ||
case .clientCertificateExpired: return "clientCertificateExpired" | ||
case .pluginFailed: return "pluginFailed" | ||
case .configurationNotFound: return "configurationNotFound" | ||
case .pluginDisabled: return "pluginDisabled" | ||
case .negotiationFailed: return "negotiationFailed" | ||
case .serverDisconnected: return "serverDisconnected" | ||
case .serverCertificateInvalid: return "serverCertificateInvalid" | ||
case .serverCertificateNotYetValid: return "serverCertificateNotYetValid" | ||
case .serverCertificateExpired: return "serverCertificateExpired" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very rarely see these appear in the metadata, so I've simplified it a bit. The only consequence of this is that we'll have to look up the reason by code each time it happens, but I personally think that's fine. But, let me know if you'd rather I add some field like codeDescription
which maps a code to its actual meaning where possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and the metadata reporter still seems to work, but I’m not sure how to create a specific error. Will approve anyway but lemme know if you have some test steps soon.
Since we're looking at a new internal release soon, I'll merge this. |
* main: Release 7.118.0-1 (#2812) [Release PR] Update VPN metadata reporter (#2808) fix address bar weirdness (#2810) Fix RMF button styling for "big_two_action" format (#2811) Existing experiment disabled, the new Settings experiment activated (#2801) Create Asana Subtask on PR requested (#2803) Remove ATB from default params (#2430) Fix Kingfisher deprecation warnings (#2799) VPN server failure detection recovery (#2779) Disable the feedback send button when there’s no text (#2800)
# By Sam Symons (3) and others # Via Chris Brind (1) and GitHub (1) * main: Release 7.118.0-1 (#2812) [Release PR] Update VPN metadata reporter (#2808) fix address bar weirdness (#2810) Fix RMF button styling for "big_two_action" format (#2811) Existing experiment disabled, the new Settings experiment activated (#2801) Create Asana Subtask on PR requested (#2803) Remove ATB from default params (#2430) Fix Kingfisher deprecation warnings (#2799) VPN server failure detection recovery (#2779) # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Task/Issue URL: https://app.asana.com/0/414235014887631/1207207179448754/f
Tech Design URL:
CC:
Description:
This PR updates the VPN metadata reporter to include the domain, code, and information about any underlying errors. It also includes the localized description.
Steps to test this PR:
startTunnel
function, add the following code:Now just try to toggle the VPN, and you'll see it shut off immediately. After that, go to the VPN debug menu and check the metadata - you should see error info, and an underlying error.
Device Testing:
OS Testing:
Theme Testing:
Internal references:
Software Engineering Expectations
Technical Design Template