Skip to content

fix: recover from browser context crash without server restart #385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
whyyagswhy wants to merge 1 commit into
base: main
Choose a base branch
from
+34 −0
Open

fix: recover from browser context crash without server restart #385

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions linkedin_mcp_server/sequential_tool_middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,21 @@

import mcp.types as mt

from fastmcp.exceptions import ToolError
from fastmcp.server.middleware import CallNext, Middleware, MiddlewareContext
from fastmcp.tools import ToolResult

logger = logging.getLogger(__name__)

# Patchright/Playwright error message emitted when the browser context dies.
# Matched as a substring so it works across Playwright versions and transports.
_BROWSER_CONTEXT_CLOSED = "Target page, context or browser has been closed"


def _is_browser_context_closed(exc: Exception) -> bool:
"""Return True if *exc* indicates the Patchright browser context has died."""
return _BROWSER_CONTEXT_CLOSED in str(exc)
Comment on lines +22 to +24
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 22, 2026

Choose a reason for hiding this comment

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

P2 Substring match may miss localized or wrapped error messages

_is_browser_context_closed converts the exception to a string and checks for a substring. If the Playwright / Patchright version in use ever wraps TargetClosedError inside another exception type (e.g. a transport or serialization wrapper), str(exc) may only contain the outer exception's message and the match will silently fail, leaving the server in the broken-singleton state. Checking type(exc).__name__ == "TargetClosedError" (or a similar type-based guard) as a primary condition, with the substring as a fallback, would be more robust.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: linkedin_mcp_server/sequential_tool_middleware.py
Line: 22-24

Comment:
**Substring match may miss localized or wrapped error messages**

`_is_browser_context_closed` converts the exception to a string and checks for a substring. If the Playwright / Patchright version in use ever wraps `TargetClosedError` inside another exception type (e.g. a transport or serialization wrapper), `str(exc)` may only contain the outer exception's message and the match will silently fail, leaving the server in the broken-singleton state. Checking `type(exc).__name__ == "TargetClosedError"` (or a similar type-based guard) as a primary condition, with the substring as a fallback, would be more robust.

How can I resolve this? If you propose a fix, please make it concise.



class SequentialToolExecutionMiddleware(Middleware):
"""Ensure only one MCP tool call executes at a time per server process."""
Expand Down Expand Up @@ -63,6 +73,30 @@ async def on_call_tool(
hold_started = time.perf_counter()
try:
return await call_next(context)
except Exception as exc:
if _is_browser_context_closed(exc):
# The Patchright browser context died mid-operation.
# Reset the browser singleton so the next tool call gets a
# fresh context (cookies are safe on disk — no re-login needed).
logger.warning(
"Browser context closed during tool '%s' — resetting for next call",
tool_name,
)
try:
# Lazy import avoids a circular dependency at module load time.
from linkedin_mcp_server.drivers.browser import close_browser
await close_browser()
except Exception:
logger.debug(
"close_browser() failed during crash recovery",
exc_info=True,
)
Comment on lines +85 to +93
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 22, 2026

Choose a reason for hiding this comment

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

P2 close_browser() may itself raise on a crashed context

close_browser() resets the singleton first (safe), but then calls await browser.close() without a surrounding try/except. When the browser context is already dead, that call is likely to raise another Playwright error, which propagates out of close_browser() and is silently swallowed here at DEBUG level. Recovery still works because the singleton was already cleared, but the only observable signal is a DEBUG log rather than a WARNING. Consider logging at WARNING when the outer catch fires, or wrapping browser.close() inside close_browser() itself so the crash path is explicit.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: linkedin_mcp_server/sequential_tool_middleware.py
Line: 85-93

Comment:
**`close_browser()` may itself raise on a crashed context**

`close_browser()` resets the singleton first (safe), but then calls `await browser.close()` without a surrounding `try/except`. When the browser context is already dead, that call is likely to raise another Playwright error, which propagates out of `close_browser()` and is silently swallowed here at `DEBUG` level. Recovery still works because the singleton was already cleared, but the only observable signal is a DEBUG log rather than a WARNING. Consider logging at `WARNING` when the outer catch fires, or wrapping `browser.close()` inside `close_browser()` itself so the crash path is explicit.

How can I resolve this? If you propose a fix, please make it concise.

raise ToolError(
"The browser context crashed mid-operation. "
"The browser has been reset — please retry this tool. "
"Your LinkedIn session is still active."
) from exc
raise
finally:
hold_seconds = time.perf_counter() - hold_started
logger.debug(
Expand Down
Loading