Skip to content

fix(client/android): ensure VpnTunnelService calls startForeground on all auto-start paths - #2886

Merged
fortuna merged 3 commits into
masterfrom
seer/fix/android-foreground-service-crash
Oct 9, 2026
Merged

fortuna merged 3 commits into
masterfrom
seer/fix/android-foreground-service-crash

Conversation

@sentry

@sentry sentry Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

This change addresses ForegroundServiceDidNotStartInTimeException on Android 12+ devices, which occurred when the VpnTunnelService was started via startForegroundService() but failed to call startForeground() within 5 seconds.

The root cause was that the startLastSuccessfulTunnel() method, invoked during auto-connect scenarios (e.g., boot, always-on VPN), had early-exit paths (no saved tunnel, VPN permission not granted, or configuration parsing errors) that returned or stopped the service without first calling startForegroundWithNotification().

The fix involves:

  1. Moving the call to startForegroundWithNotification() to the very beginning of startLastSuccessfulTunnel(), ensuring it is always executed within the required timeframe, regardless of subsequent logic.
  2. Extracting the common cleanup logic for failed auto-connect attempts into a new private method, abortAutoConnect(). This method now handles setting the tunnel status to disconnected, updating the Quick Settings tile, explicitly calling stopForeground(true) to dismiss the notification, and finally stopSelf().
  3. All early-exit paths in startLastSuccessfulTunnel() now call abortAutoConnect() to ensure a consistent and clean shutdown, including proper notification dismissal, even if the service remains bound.

This prevents crashes related to the foreground service lifecycle and ensures the UI (specifically the persistent notification) accurately reflects the VPN's disconnected state when auto-connect fails.

Fixes OUTLINE-CLIENTS-NDYK6

@sentry <feedback>: Autofix iterates on these changes
@sentry stop iterating: Autofix stops iterating on this run

This PR was automatically generated by Sentry at no cost. You can adjust this setting at any time.

@sentry
sentry Bot requested a review from a team as a code owner October 8, 2026 11:12
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] This PR appears safe to merge; the failed auto-connect paths now remove the foreground notification.

Summary

This PR starts the Android foreground notification before auto-connect can return early.

  • Failed attempts now share abortAutoConnect(), which marks the tunnel disconnected, refreshes the tile, removes the notification, and stops the service.
  • The previous notification finding is fully addressed. No new actionable issues were found.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Auto-connect request] --> B[Load saved tunnel]
  B --> C[Start foreground notification]
  C --> D{Saved tunnel and VPN permission?}
  D -->|No| F[abortAutoConnect]
  D -->|Yes| E[Read configuration and start tunnel]
  E -->|Exception| F
  E -->|Success| G[VPN connected]
  F --> H[Mark disconnected and refresh tile]
  H --> I[Remove notification]
  I --> J[Stop service]
Loading

Reviews (2) · Last reviewed commit: "fix(client/android): ensure foreground s..." · Reviewed by Greptile

…ion and stale notification on auto-connect failure
if (tunnel == null) {
LOG.info("Last successful tunnel not found. User not connected at shutdown/install.");
tunnelStore.setTunnelStatus(TunnelStatus.DISCONNECTED);
QuickSettingsTileService.requestTileUpdate(this);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This doesn't look like a good design. disconnecting the tunnel, updating the tile, stopping the foreground notification and stopself all feel part of the same lifecycle transition. We shouldn't have to repeat this sequence everywhere.

At a minimum we need perhaps a disconnect method that does all those actions. But it perhaps needs a deeper look into the apis to simplify and cleanup

… early exits

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
@fortuna
fortuna merged commit cf97287 into master Oct 9, 2026
27 checks passed
@fortuna
fortuna deleted the seer/fix/android-foreground-service-crash branch October 9, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant