Skip to content

fix: per-user nav health tracking, session-scoped recovery, single-flight race - #9324

Open
dyiapanis wants to merge 1 commit into
jo-inc:masterfrom
dyiapanis:d/fix/nav-health-per-user-recovery
Open

fix: per-user nav health tracking, session-scoped recovery, single-flight race#9324
dyiapanis wants to merge 1 commit into
jo-inc:masterfrom
dyiapanis:d/fix/nav-health-per-user-recovery

Conversation

@dyiapanis

Copy link
Copy Markdown

Summary

Fixes three bugs in the browser health tracking system:

Also fixes #8554restartBrowser() cleared browserLaunchPromise before calling ensureBrowser(), creating a window for a concurrent second browser launch.

Changes

Per-user health tracking (#9321)

Replaced the single healthState.consecutiveNavFailures counter with a Map<userId, UserNavHealth>. recordNavSuccess(userId) and recordNavFailure(userId) now accept a userId parameter and operate only on that user's entry. Process-global fields (activeOps, isRecovering, lastSuccessfulNav) remain on the shared healthState object for the health probe and /health endpoint.

Session-scoped recovery (#9322)

Added recoverUserSession(userId, reason) which destroys only the failing user's session via destroySession(userId) instead of closeAllSessions(). restartBrowser() is still used for browser-level failures (health probe failure, disconnection), but per-user nav failures now trigger session-scoped recovery.

Wired nav health into routes (#9323)

All five navigation entry points now call the health functions:

  • POST /tabs — after successful page.goto() and on failure
  • POST /tabs/:tabId/navigate — after successful navigation and on failure
  • POST /tabs/open — after successful navigation and on failure
  • POST /navigate (OpenClaw) — after successful navigation and on failure
  • handleRouteError() — records nav failure for navigation-related timeouts (covers /act clicks)

On successful navigation: recordNavSuccess(userId) resets the user's failure counter.
On failed navigation: recordNavFailure(userId) increments the counter. When the counter reaches FAILURE_THRESHOLD (3), recoverUserSession(userId) fires — destroying and recreating only that user's session.

Single-flight race fix (#8554)

Removed the browserLaunchPromise = null assignment from restartBrowser(). ensureBrowser() already owns the single-flight primitive — its .finally(() => { browserLaunchPromise = null }) handles cleanup. Clearing it manually opened a window where a concurrent request could start a second launch.

Cleanup

  • destroySession(userId) now calls deleteUserNavHealth(userId) to clean up per-user state.
  • /health endpoint aggregates per-user failures via Array.from(userNavHealth.values()).reduce(...).

Testing

NODE_OPTIONS='--experimental-vm-modules' npx jest tests/unit/ --runInBand --forceExit
# 56 suites passed, 788 tests passed

New test file tests/unit/navHealthRecovery.test.js (8 tests) verifies:

  • Each user has an independent failure counter
  • recordNavSuccess resets only the specified user's counter
  • recordNavFailure returns true only when the user's own threshold is exceeded
  • Interleaved failures from different users do not trigger false recovery
  • deleteUserNavHealth removes only the specified user
  • Total failures aggregate correctly across all users
  • recordNavFailure with no userId is a no-op
  • A user reaching threshold does not cause another user to recover

Fixes #9321, #9322, #9323
Relates to #8554

…ight race

- Replace process-global healthState.consecutiveNavFailures with
  Map<userId, UserNavHealth> for per-user failure tracking (jo-inc#9321)
- Wire recordNavSuccess(userId)/recordNavFailure(userId) into all
  navigation routes: POST /tabs, POST /tabs/:tabId/navigate,
  POST /tabs/open, POST /navigate, and handleRouteError for /act (jo-inc#9323)
- Add recoverUserSession(userId) for session-scoped recovery instead
  of restartBrowser() killing all sessions on nav failures (jo-inc#9322)
- Remove manual browserLaunchPromise = null in restartBrowser() —
  ensureBrowser() owns the single-flight primitive (jo-inc#8554)
- Clean up per-user nav health on destroySession()
- Aggregate per-user failures in /health endpoint
- Add tests/unit/navHealthRecovery.test.js (8 tests)

Fixes jo-inc#9321, jo-inc#9322, jo-inc#9323
Relates to jo-inc#8554
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.

bug: process-global healthState causes cross-user recovery interference

1 participant