fix(session): repair the expiry warning and the gaps the review found
Review of 742f376 (Fable). Three defects made the feature not work for the
case it exists for:
- armExpiryWarning() cancelled the alarm before its own early return. Dart
re-pushes the deadline every 60 s with no dedupe (rule 93's ">5 s" dedupe
does not exist), so the first sync after the warning came due deleted the
armed alarm and never re-armed it. Cancel now happens only on the branch
that re-arms.
- cancelSessionWarningAlarm() built its PendingIntent with
FLAG_UPDATE_CURRENT, rewriting the live alarm's RUNTIME_TOKEN to "" so a
delivery in flight was dropped as a stale generation. It now looks the
existing PendingIntent up with FLAG_NO_CREATE.
- SOCKS5 mode never requested POST_NOTIFICATIONS (requestPermission() is
TUN-only because it also raises the VPN consent dialog), so on Android 13+
both alerts were dropped. New plugin method requests it on its own.
Also:
- the native deadline wins the expiry race against the 1 s Dart tick, so the
commonest session end reached _onVpnConnectionChanged as a bare tunnel
drop with no message. It now reconciles and sets the expiry text, with
postAlert false since the native side already posted.
- a resume reconnect no longer resets the support-ad claim: it re-attaches
to the same server session, where the top-up has already been spent.
- RevoltAlerts sets setOnlyAlertOnce so a replacing post does not re-sound.
- AGENTS.md rule 93 described a setSessionExpiry/SESSION_EXPIRES_AT_MS path
that no longer exists, dedupe included. Corrected; that stale text is what
the warning bug was written against.
- CHANGELOG no longer claims the alert covers the tunnel stopping on its own.
flutter analyze clean; debug APK builds.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
1 parent
742f37683e
commit
dd1082c8de
7 files changed
+92
-14
No files matched your search
+6
-3
@@ -11,9 +11,12 @@ Versions follow [semver](https://semver.org/): MAJOR.MINOR.PATCH
|
|||||||
- **Five-minute warning before a session ends.** The VPN service schedules it
|
- **Five-minute warning before a session ends.** The VPN service schedules it
|
||||||
off the same deadline it already enforces, so the warning still arrives when
|
off the same deadline it already enforces, so the warning still arrives when
|
||||||
the app is closed, and a support top-up moves it with the deadline.
|
the app is closed, and a support top-up moves it with the deadline.
|
||||||
- **"Disconnected — not protected" notification** whenever a session ends for
|
- **"Disconnected — not protected" notification** when a session ends without
|
||||||
any reason other than you tapping disconnect: expiry, the data limit, the
|
you asking: the 2-hour expiry, the data limit, the server ending the session,
|
||||||
server ending the session, or the tunnel stopping on its own.
|
or the tunnel reporting an error.
|
||||||
|
- **SOCKS5 mode now asks for the notification permission.** It never did,
|
||||||
|
because it never raises the VPN consent dialog, so on Android 13+ these
|
||||||
|
alerts would have been dropped before they were shown.
|
||||||
- **The app now shows what went wrong.** Connect errors, the reason a session
|
- **The app now shows what went wrong.** Connect errors, the reason a session
|
||||||
ended, and progress while connecting appear under the status bar instead of
|
ended, and progress while connecting appear under the status bar instead of
|
||||||
the button silently going back to idle.
|
the button silently going back to idle.
|
||||||
|
|||||||
@@ -141,14 +141,28 @@ class SessionTimer extends ChangeNotifier with WidgetsBindingObserver {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (vpnConnection.status == VpnStatus.disconnected) {
|
if (vpnConnection.status == VpnStatus.disconnected) {
|
||||||
// No alert here. The runtime going away covers both the native deadline
|
// The native deadline fires on the exact second while this 1 s tick is
|
||||||
// (which posts the alert itself, from a process that outlives this one)
|
// in arbitrary phase, so expiry almost always arrives here rather than
|
||||||
// and the notification's Disconnect action, which the user tapped.
|
// at the countdown check in _tick - without this, the commonest session
|
||||||
unawaited(_doDisconnect('VPN tunnel dropped'));
|
// end left the UI with nothing to say.
|
||||||
|
_reconcileElapsedTime();
|
||||||
|
final expired = _hasSyncedOnce && _remainingSeconds <= 2;
|
||||||
|
// postAlert is false either way: the native side already posted for its
|
||||||
|
// own deadline, and the notification's Disconnect action is the user
|
||||||
|
// asking, which must never raise a "not protected" alert.
|
||||||
|
unawaited(_doDisconnect(
|
||||||
|
'VPN tunnel dropped',
|
||||||
|
endedMessage:
|
||||||
|
expired ? 'Your session ended. You are not protected.' : null,
|
||||||
|
postAlert: false,
|
||||||
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
Future<void> start() async {
|
// resetSupportClaim is false when re-attaching to a session the device
|
||||||
|
// already holds: the support top-up is once per server session, and the
|
||||||
|
// claim flag is the only thing enforcing that.
|
||||||
|
Future<void> start({bool resetSupportClaim = true}) async {
|
||||||
_sessionEpoch++;
|
_sessionEpoch++;
|
||||||
_remainingSeconds = 0;
|
_remainingSeconds = 0;
|
||||||
_remainingAtLastSync = 0;
|
_remainingAtLastSync = 0;
|
||||||
@@ -165,10 +179,12 @@ class SessionTimer extends ChangeNotifier with WidgetsBindingObserver {
|
|||||||
_lastEndMessage = null;
|
_lastEndMessage = null;
|
||||||
NotificationService.reset();
|
NotificationService.reset();
|
||||||
|
|
||||||
|
if (resetSupportClaim) {
|
||||||
_supportStateEpoch++;
|
_supportStateEpoch++;
|
||||||
_supportRewardClaimed = false;
|
_supportRewardClaimed = false;
|
||||||
_supportRewardStateLoaded = true;
|
_supportRewardStateLoaded = true;
|
||||||
unawaited(_persistSupportRewardState(false));
|
unawaited(_persistSupportRewardState(false));
|
||||||
|
}
|
||||||
|
|
||||||
_timer?.cancel();
|
_timer?.cancel();
|
||||||
_timer = Timer.periodic(const Duration(seconds: 1), _tick);
|
_timer = Timer.periodic(const Duration(seconds: 1), _tick);
|
||||||
@@ -233,7 +249,11 @@ class SessionTimer extends ChangeNotifier with WidgetsBindingObserver {
|
|||||||
// not ask for, and its presence is also what posts the "not protected"
|
// not ask for, and its presence is also what posts the "not protected"
|
||||||
// notification. Both exist so nobody is left on their real IP without being
|
// notification. Both exist so nobody is left on their real IP without being
|
||||||
// told. A user-requested disconnect passes nothing.
|
// told. A user-requested disconnect passes nothing.
|
||||||
Future<void> _doDisconnect(String reason, {String? endedMessage}) async {
|
Future<void> _doDisconnect(
|
||||||
|
String reason, {
|
||||||
|
String? endedMessage,
|
||||||
|
bool postAlert = true,
|
||||||
|
}) async {
|
||||||
if (_isDisconnecting) return;
|
if (_isDisconnecting) return;
|
||||||
_isDisconnecting = true;
|
_isDisconnecting = true;
|
||||||
_sessionEpoch++;
|
_sessionEpoch++;
|
||||||
@@ -252,7 +272,7 @@ class SessionTimer extends ChangeNotifier with WidgetsBindingObserver {
|
|||||||
notifyListeners();
|
notifyListeners();
|
||||||
|
|
||||||
await vpnConnection.disconnect();
|
await vpnConnection.disconnect();
|
||||||
if (endedMessage != null) {
|
if (endedMessage != null && postAlert) {
|
||||||
unawaited(NotificationService.showDisconnectedAlert());
|
unawaited(NotificationService.showDisconnectedAlert());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -224,6 +224,18 @@ class VpnConnection extends ChangeNotifier {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SOCKS5 mode skips requestPermission below (it would raise the VPN
|
||||||
|
// consent dialog for an interface it never creates), so ask for the
|
||||||
|
// notification permission on its own - the session alerts need it.
|
||||||
|
if (!kIsWeb && _activeMode == ConnectionMode.proxy) {
|
||||||
|
try {
|
||||||
|
await _nativeControl.invokeMethod<void>('requestNotificationPermission');
|
||||||
|
} catch (e) {
|
||||||
|
debugPrint('[VPN] Notification permission request failed: $e');
|
||||||
|
}
|
||||||
|
if (!_isCurrentConnect(connectEpoch)) return false;
|
||||||
|
}
|
||||||
|
|
||||||
if (!kIsWeb && _activeMode == ConnectionMode.tun) {
|
if (!kIsWeb && _activeMode == ConnectionMode.tun) {
|
||||||
final ok = await _vless.requestPermission();
|
final ok = await _vless.requestPermission();
|
||||||
if (!_isCurrentConnect(connectEpoch)) return false;
|
if (!_isCurrentConnect(connectEpoch)) return false;
|
||||||
|
|||||||
@@ -82,7 +82,7 @@ class _ConnectionModeTileState extends State<ConnectionModeTile> {
|
|||||||
if (!mounted) return;
|
if (!mounted) return;
|
||||||
if (await vpn.connect(resume: true)) {
|
if (await vpn.connect(resume: true)) {
|
||||||
if (!mounted) return;
|
if (!mounted) return;
|
||||||
await timer.start();
|
await timer.start(resetSupportClaim: false);
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
if (mounted) {
|
if (mounted) {
|
||||||
|
|||||||
+25
@@ -240,6 +240,31 @@ class FlutterVlessPlugin : FlutterPlugin, ActivityAware, PluginRegistry.Activity
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// POST_NOTIFICATIONS on its own, without VpnService.prepare: SOCKS5
|
||||||
|
// mode never establishes a VPN interface, so it never called
|
||||||
|
// requestPermission and both session alerts were dropped unasked
|
||||||
|
// for on Android 13+.
|
||||||
|
"requestNotificationPermission" -> {
|
||||||
|
val currentActivity = activity
|
||||||
|
if (currentActivity == null) {
|
||||||
|
result.error("NO_ACTIVITY", "Activity is not attached", null)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU &&
|
||||||
|
ActivityCompat.checkSelfPermission(
|
||||||
|
currentActivity,
|
||||||
|
Manifest.permission.POST_NOTIFICATIONS,
|
||||||
|
) != PackageManager.PERMISSION_GRANTED
|
||||||
|
) {
|
||||||
|
ActivityCompat.requestPermissions(
|
||||||
|
currentActivity,
|
||||||
|
arrayOf(Manifest.permission.POST_NOTIFICATIONS),
|
||||||
|
REQUEST_CODE_POST_NOTIFICATIONS,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
result.success(null)
|
||||||
|
}
|
||||||
|
|
||||||
"requestPermission" -> {
|
"requestPermission" -> {
|
||||||
val currentActivity = activity
|
val currentActivity = activity
|
||||||
if (currentActivity == null) {
|
if (currentActivity == null) {
|
||||||
|
|||||||
+18
-3
@@ -473,15 +473,18 @@ class XrayVPNService : VpnService() {
|
|||||||
// scheduled must never stop a working tunnel, so nothing here fails closed.
|
// scheduled must never stop a working tunnel, so nothing here fails closed.
|
||||||
// Dart re-pushes the deadline on every sync, so this runs about once a minute.
|
// Dart re-pushes the deadline on every sync, so this runs about once a minute.
|
||||||
private fun armExpiryWarning(runtimeToken: String, expiresAtEpochMs: Long) {
|
private fun armExpiryWarning(runtimeToken: String, expiresAtEpochMs: Long) {
|
||||||
cancelSessionWarningAlarm()
|
|
||||||
if (runtimeToken.isEmpty()) return
|
if (runtimeToken.isEmpty()) return
|
||||||
|
|
||||||
val warnAt = expiresAtEpochMs - RevoltAlerts.WARNING_LEAD_MS
|
val warnAt = expiresAtEpochMs - RevoltAlerts.WARNING_LEAD_MS
|
||||||
// Deadline already inside the warning window (or past): either the alarm
|
// Deadline already inside the warning window (or past): either the alarm
|
||||||
// has fired and its notification should stay up, or this is the short
|
// has fired and its notification should stay up, or this is the short
|
||||||
// bootstrap deadline at connect, which must not warn at all.
|
// bootstrap deadline at connect, which must not warn at all. Nothing is
|
||||||
|
// cancelled here - Dart re-pushes the same deadline every 60 s with no
|
||||||
|
// dedupe, so cancelling on this branch would reliably delete the armed
|
||||||
|
// warning on the first sync after it came due and never re-arm it.
|
||||||
if (warnAt <= System.currentTimeMillis()) return
|
if (warnAt <= System.currentTimeMillis()) return
|
||||||
|
|
||||||
|
cancelSessionWarningAlarm()
|
||||||
// More than the lead away, so any warning on screen belongs to an older
|
// More than the lead away, so any warning on screen belongs to an older
|
||||||
// deadline - a support top-up just pushed it out.
|
// deadline - a support top-up just pushed it out.
|
||||||
RevoltAlerts.cancelExpiryWarning(this)
|
RevoltAlerts.cancelExpiryWarning(this)
|
||||||
@@ -691,12 +694,24 @@ class XrayVPNService : VpnService() {
|
|||||||
private fun cancelSessionWarningAlarm() {
|
private fun cancelSessionWarningAlarm() {
|
||||||
try {
|
try {
|
||||||
val alarmManager = getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
val alarmManager = getSystemService(Context.ALARM_SERVICE) as AlarmManager
|
||||||
alarmManager.cancel(sessionWarningPendingIntent(""))
|
// Look the existing PendingIntent up instead of building one: with
|
||||||
|
// FLAG_UPDATE_CURRENT, constructing it to cancel rewrites the live
|
||||||
|
// alarm's RUNTIME_TOKEN to "" first, and a delivery already in
|
||||||
|
// flight would then be dropped as a stale generation.
|
||||||
|
existingSessionWarningPendingIntent()?.let { alarmManager.cancel(it) }
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.w(TAG, "Failed to cancel session-warning alarm", e)
|
Log.w(TAG, "Failed to cancel session-warning alarm", e)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun existingSessionWarningPendingIntent(): PendingIntent? {
|
||||||
|
val intent = Intent(this, XrayVPNService::class.java)
|
||||||
|
.putExtra(EXTRA_SESSION_WARNING, true)
|
||||||
|
val flags = PendingIntent.FLAG_NO_CREATE or
|
||||||
|
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) PendingIntent.FLAG_IMMUTABLE else 0
|
||||||
|
return PendingIntent.getService(this, SESSION_WARNING_REQUEST_CODE, intent, flags)
|
||||||
|
}
|
||||||
|
|
||||||
// A PendingIntent is identified by component plus request code, not extras,
|
// A PendingIntent is identified by component plus request code, not extras,
|
||||||
// so the warning needs its own code or it would overwrite - and later
|
// so the warning needs its own code or it would overwrite - and later
|
||||||
// cancel - the expiry alarm.
|
// cancel - the expiry alarm.
|
||||||
|
|||||||
+3
@@ -97,6 +97,9 @@ object RevoltAlerts {
|
|||||||
.setSmallIcon(icon)
|
.setSmallIcon(icon)
|
||||||
.setAutoCancel(true)
|
.setAutoCancel(true)
|
||||||
.setOngoing(false)
|
.setOngoing(false)
|
||||||
|
// Both processes can post the disconnected alert; the second
|
||||||
|
// post replaces the first and must not sound again.
|
||||||
|
.setOnlyAlertOnce(true)
|
||||||
|
|
||||||
launchIntent(context)?.let { builder.setContentIntent(it) }
|
launchIntent(context)?.let { builder.setContentIntent(it) }
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user