feat: add OFFLINE_GRACE_PERIOD env to extend keepAlive timeout#33
feat: add OFFLINE_GRACE_PERIOD env to extend keepAlive timeout#33kilyabin wants to merge 1 commit intoPasarGuard:devfrom
Conversation
Adds optional OFFLINE_GRACE_PERIOD environment variable that extends
the disconnect timeout beyond the panel's keepAlive value.
Fully backward compatible — default behavior preserved when not set.
WalkthroughThis change adds optional offline grace period support configured through the OFFLINE_GRACE_PERIOD environment variable. The Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable poems in the walkthrough.Disable the |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controller/controller.go (1)
127-140: Consider centralizing OFFLINE_GRACE_PERIOD in config/config.go.The implementation is functionally correct, but it reads the environment variable directly via
os.Getenv()instead of following the established pattern inconfig/config.gowhere all environment variables are read through typed helper functions (GetEnv,GetEnvAsInt,GetEnvAsBool) during startup.Benefits of centralization:
- Consistent validation and logging at startup
- Single source of truth in the
Configstruct- Easier to test by injecting config rather than setting env vars
However, the current approach is acceptable if runtime flexibility (reading per-connection) is desired.
♻️ Optional: Add a helper to config/config.go
Add to
config/config.go:func GetEnvAsDuration(name string, defaultVal time.Duration) time.Duration { valStr := GetEnv(name, "") if val, err := time.ParseDuration(valStr); err == nil { return val } return defaultVal }Then read in
Load():cfg.OfflineGracePeriod = GetEnvAsDuration("OFFLINE_GRACE_PERIOD", 0)And pass it through to
keepAliveTracker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/controller.go` around lines 127 - 140, The code reads OFFLINE_GRACE_PERIOD directly with os.Getenv inside the controller (affecting effectiveTimeout calculation) instead of centralizing it in the Config; refactor by adding a typed helper GetEnvAsDuration in config/config.go, load the value into the Config (e.g., Config.OfflineGracePeriod) during Load(), and replace the os.Getenv/time.ParseDuration block in controller.go to use the injected cfg.OfflineGracePeriod when computing effectiveTimeout from keepAlive so env parsing/validation and logging happen at startup and the controller uses the typed config value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@controller/controller.go`:
- Around line 127-140: The code reads OFFLINE_GRACE_PERIOD directly with
os.Getenv inside the controller (affecting effectiveTimeout calculation) instead
of centralizing it in the Config; refactor by adding a typed helper
GetEnvAsDuration in config/config.go, load the value into the Config (e.g.,
Config.OfflineGracePeriod) during Load(), and replace the
os.Getenv/time.ParseDuration block in controller.go to use the injected
cfg.OfflineGracePeriod when computing effectiveTimeout from keepAlive so env
parsing/validation and logging happen at startup and the controller uses the
typed config value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58ca9081-156a-4df4-992d-8681bf63fce6
📒 Files selected for processing (2)
.env.examplecontroller/controller.go
|
However, as far as I know, the panel does not automatically reconnect to the node after a prolonged downtime. This should also be addressed — otherwise the grace period only delays the problem rather than solving it. The full solution would require auto-reconnect logic on the panel side as well. |
|
Add the new env in config package and read it from there |
|
You can just increase keep alive instead of this |
I didn’t find in the documentation how to change or somehow manipulate this setting |
|
Just modify the node |
3 participants
Adds optional
OFFLINE_GRACE_PERIODenvironment variable that extends the disconnect timeout beyond the panel's keepAlive value. When set, the node continues running Xray for the configured duration even if the panel becomes unreachable, instead of immediately shutting down user connections.This improves resilience in deployments where the panel may experience temporary downtime (server restart, network issue, maintenance, etc.).
Changes
controller/controller.go: ModifiedkeepAliveTrackerto readOFFLINE_GRACE_PERIODand calculate effective timeout.env.example: Added documentation and example configurationBehavior
OFFLINE_GRACE_PERIODnot setOFFLINE_GRACE_PERIOD=1hOFFLINE_GRACE_PERIOD=abc)Configuration
# .env or docker environment OFFLINE_GRACE_PERIOD=1hFormat: Go duration string (
1h,30m,24h, etc.)Default:
0(disabled — original behavior preserved)Logging
On startup (after panel connects), the node logs:
On invalid value:
Testing
Backward Compatibility
✅ Fully backward compatible — if
OFFLINE_GRACE_PERIODis not set, behavior is identical to the original implementation.Known Limitations
Checklist
go build ./...)go vet ./...)go test ./controller -v).env.exampleSummary by CodeRabbit
OFFLINE_GRACE_PERIODenvironment variable that extends the timeout window for disconnect detection, allowing for more graceful handling of temporary connectivity issues.