Skip to content

Conversation

@sergiopaniego
Copy link
Contributor

Summary

This PR fixes BrowserGym compatibility issues that caused exceptions when using Playwright, and increases the WebSocket message size limit to support large observations.
It's related to #343

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Test Plan

The updated env is deployed here

Claude Code Review

"N/A"

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 3, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 3, 2026

Greptile Overview

Greptile Summary

This PR fixes critical threading issues with BrowserGym's Playwright integration and increases WebSocket message limits to handle large observations.

Key Changes:

  • Threading Fix: Modified HTTPEnvServer to create environments within their dedicated executor threads. This ensures Playwright/greenlet objects are created and destroyed in the same thread, preventing "greenlet.error: cannot switch to a different thread" exceptions.
  • WebSocket Message Limit: Increased default max_message_size to 100MB in EnvClient to accommodate large observations (screenshots, DOM data) from browser environments.
  • Documentation Updates: Fixed Docker build paths across multiple READMEs and Dockerfiles after repository reorganization from src/core/ to src/openenv/core/.
  • Dependency Update: Changed browsergym_env to use git dependency for openenv-core to pick up async API changes from PR [1/n] Make EnvClient async by default with SyncEnvClient wrapper #343.

The threading fix is particularly important - it moves environment instantiation and cleanup into the executor thread, which is required for libraries like Playwright that use greenlets and thread-local storage.

Confidence Score: 4/5

  • This PR is safe to merge with low risk - fixes critical bug and adds sensible defaults
  • High confidence due to: (1) clear bug fix addressing documented Playwright threading issues, (2) sensible 100MB message size default for browser observations, (3) mostly documentation updates. Slight reduction for lacking tests covering the threading fix edge cases.
  • Pay close attention to src/openenv/core/env_server/http_server.py to verify the threading fix handles all error cases correctly

Important Files Changed

Filename Overview
src/openenv/core/env_server/http_server.py Fixed Playwright/greenlet threading issues by creating environment in dedicated executor thread and ensuring cleanup happens in same thread
src/openenv/core/env_client.py Added configurable WebSocket message size limit (default 100MB) to support large observations like screenshots and DOM data
envs/browsergym_env/pyproject.toml Updated to use git dependency to get latest openenv-core with async API changes from PR #343
envs/browsergym_env/README.md Fixed Docker build instructions to use correct paths and build context

Sequence Diagram

sequenceDiagram
    participant Client as EnvClient
    participant HTTPServer as HTTPEnvServer
    participant Executor as ThreadPoolExecutor
    participant Env as BrowserGym Environment
    participant Playwright as Playwright/Greenlet

    Note over Client,Playwright: WebSocket Session Creation

    Client->>HTTPServer: WebSocket connect /ws
    HTTPServer->>Executor: Create ThreadPoolExecutor(max_workers=1)
    Note over HTTPServer,Executor: Store executor before env creation
    HTTPServer->>Executor: run_in_executor(env_factory)
    Executor->>Env: Create environment instance
    Env->>Playwright: Initialize Playwright browser
    Note over Playwright: All Playwright objects created in executor thread
    Playwright-->>Env: Browser ready
    Env-->>Executor: Environment ready
    Executor-->>HTTPServer: Environment instance
    HTTPServer->>HTTPServer: Store session (env + executor)
    HTTPServer-->>Client: WebSocket connected

    Note over Client,Playwright: Step Operations

    Client->>HTTPServer: step(action) via WebSocket
    HTTPServer->>Executor: run_in_executor(env.step)
    Note over Executor: Same thread as env creation
    Executor->>Env: step(action)
    Env->>Playwright: Browser interactions
    Playwright-->>Env: Observation (screenshot, DOM)
    Note over Env: Large message (up to 100MB)
    Env-->>Executor: Observation
    Executor-->>HTTPServer: Result
    HTTPServer-->>Client: Observation via WebSocket
    Note over Client: max_message_size=100MB handles large obs

    Note over Client,Playwright: Session Cleanup

    Client->>HTTPServer: WebSocket disconnect
    HTTPServer->>Executor: run_in_executor(env.close)
    Note over Executor: Close in same thread as creation
    Executor->>Env: close()
    Env->>Playwright: Cleanup browser resources
    Playwright-->>Env: Closed
    Env-->>Executor: Done
    Executor-->>HTTPServer: Cleanup complete
    HTTPServer->>Executor: shutdown(wait=False)
    HTTPServer-->>Client: Session destroyed
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 297 to 300
try:
env = self._env_factory()
# Create environment in the executor thread
loop = asyncio.get_event_loop()
env = await loop.run_in_executor(executor, self._env_factory)
Copy link
Contributor

Choose a reason for hiding this comment

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

environment creation inside _session_lock blocks other concurrent session operations

If _env_factory() takes significant time (e.g., launching Playwright browser), this will block other WebSocket connections from being created/destroyed. Consider releasing the lock before awaiting the executor, then re-acquiring it to update _sessions.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/openenv/core/env_server/http_server.py
Line: 297:300

Comment:
environment creation inside `_session_lock` blocks other concurrent session operations

If `_env_factory()` takes significant time (e.g., launching Playwright browser), this will block other WebSocket connections from being created/destroyed. Consider releasing the lock before awaiting the executor, then re-acquiring it to update `_sessions`.

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@sergiopaniego do we need to fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

@burtenshaw burtenshaw added the bug Something isn't working label Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants