Skip to content
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

Fix VPN toggle failures #2892

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Fix VPN toggle failures #2892

merged 2 commits into from
Jun 21, 2024

Conversation

diegoreymendez
Copy link
Contributor

@diegoreymendez diegoreymendez commented Jun 21, 2024

Task/Issue URL: https://app.asana.com/0/1207603085593419/1207629954356116/f

iOS PR: duckduckgo/iOS#2979
BSK PR: duckduckgo/BrowserServicesKit#858

Description

The VPN toggle is sometimes failing silently, apparently due to a race condition created by calls to sendProviderMessage while the VPN is starting up.

This PR changes our logic to limit when those calls are made, since none of them make sense to call when the VPN is connecting.

Testing

  • Test turning the VPN ON and OFF.
  • Go to activity monitor, kill the tunnel extension and try again.
  • Try variants, make sure the server address, location and data volume look right.

Internal references:

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

@diegoreymendez diegoreymendez self-assigned this Jun 21, 2024
@diegoreymendez diegoreymendez changed the title Fix VPN toggle failures VPN: Fix VPN toggle failures Jun 21, 2024
@diegoreymendez diegoreymendez changed the title VPN: Fix VPN toggle failures Fix VPN toggle failures Jun 21, 2024
@diegoreymendez diegoreymendez changed the base branch from main to release/1.93.0 June 21, 2024 10:09
@diegoreymendez diegoreymendez marked this pull request as ready for review June 21, 2024 10:09
Copy link
Contributor

@aataraxiaa aataraxiaa left a comment

Choose a reason for hiding this comment

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

Tested starting/stopping/force quitting via Activity Monitor. Seemed to behave as expected. Only issue I saw was a keychain issue, but I don’t think its related. However, I’ll let you make the call on that. See attached.

Screenshot 2024-06-21 at 12 26 15

diegoreymendez added a commit to duckduckgo/BrowserServicesKit that referenced this pull request Jun 21, 2024
Task/Issue URL: https://app.asana.com/0/1207603085593419/1207629954356116/f

iOS PR: duckduckgo/iOS#2979
macOS PR: duckduckgo/macos-browser#2892

What kind of version bump will this require?: Patch

## Description

The VPN toggle is sometimes failing silently, apparently due to a race condition created by calls to `sendProviderMessage` while the VPN is starting up.

This PR changes our logic to limit when those calls are made, since none of them make sense to call when the VPN is connecting.
@diegoreymendez
Copy link
Contributor Author

Thanks @aataraxiaa . That's a debug error so it won't show to users. It should be fine as it certainly wasn't introduced here, so I'll move forward.

@diegoreymendez diegoreymendez merged commit c712413 into release/1.93.0 Jun 21, 2024
20 checks passed
@diegoreymendez diegoreymendez deleted the diego/fix-vpn-toggle branch June 21, 2024 12:41
diegoreymendez added a commit to duckduckgo/iOS that referenced this pull request Jun 21, 2024
Task/Issue URL: https://app.asana.com/0/1207603085593419/1207629954356116/f

macOS PR: duckduckgo/macos-browser#2892
BSK PR: duckduckgo/BrowserServicesKit#858

## Description

The VPN toggle is sometimes failing silently, apparently due to a race condition created by calls to `sendProviderMessage` while the VPN is starting up.

This PR changes our logic to limit when those calls are made, since none of them make sense to call when the VPN is connecting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants