Skip to content

Conversation

@aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented Sep 16, 2025

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

Description

Work around platform bug (b/229000935) where startForegroundService() for
VpnService can throw ForegroundServiceStartNotAllowedException when called
from background. We now catch this and fall back to startService(), which
is allowed for VPNs. Service still promotes itself to foreground as usual.

Steps to test this PR

Smoke test AppTP and VPN on off scenarios for Android on 31+ and below 31

Copy link
Collaborator Author

aitorvs commented Sep 16, 2025

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

@aitorvs aitorvs force-pushed the fix/aitor/vpn/crash_ForegroundServiceStartNotAllowedException branch from d955802 to c7e5b7d Compare September 16, 2025 21:18
@karlenDimla karlenDimla merged commit 907bc41 into develop Sep 17, 2025
9 checks passed
@karlenDimla karlenDimla deleted the fix/aitor/vpn/crash_ForegroundServiceStartNotAllowedException branch September 17, 2025 09:09
aitorvs added a commit that referenced this pull request Sep 23, 2025
Task/Issue URL:
https://app.asana.com/1/137249556945/project/1202552961248957/task/1211418384396819?focus=true

### Description
In #6808 we bubbled up the
`ForegroundServiceStartNotAllowedException` to notifyVpnStart function,
when calling `startForeground` method.

This didn't get rid of the crashes but was a bit of an improvement.

This PR will try-catch the startForeground call and return the
appropriate error so that the error path can be executed. The error path
sends a pixel (m_vpn_ev_notify_start_failed_c/d) are expected to
increase and will also stop the service showing the disabled
notification. Which is a better UX

### Steps to test this PR

Smoke test the VPN and AppTP
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