-
Notifications
You must be signed in to change notification settings - Fork 46
Description
Bug Description
ModelClient in Python SDK has two major issues:
-
Missing error handling for
json.loads(): Theparse_request_line()andparse_response_line()methods calljson.loads(meta_json)without try/catch. If the log file is partially written or corrupted, this will throw an unhandledJSONDecodeErrorexception. -
Synchronous file I/O in async methods: All file operations (
read_last_request_line(),read_last_response_line(),wait_for_first_request(),_append_response()) use synchronousopen()andreadlines()insideasyncmethods. This blocks the asyncio event loop and defeats the purpose of using async/await.
Steps to Reproduce
- Create a
ModelClientinstance - Simulate a corrupted log file with malformed JSON in the meta section:
# Write a corrupted line to the log file with open(log_file, 'w') as f: f.write("__REQUEST_START__some_request__REQUEST_END__{invalid json}\n")
- Call
await client.pop_request(1) - Observe the unhandled
JSONDecodeError
Expected Behavior
json.loads()should be wrapped in try/catch, and a meaningful error should be raised or logged when JSON parsing fails.- File I/O operations should use async libraries like
aiofilesto avoid blocking the event loop.
Actual Behavior
JSONDecodeErroris thrown without context, making it difficult to debug log file issues.- Async methods perform blocking I/O, which can cause performance issues in high-concurrency scenarios.
Error Logs
Traceback (most recent call last):
File "...", line X, in parse_request_line
meta = json.loads(meta_json)
^^^^^^^^^^^^^^^^^^^^^
json.decoder.JSONDecodeError: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)
Environment Information
- OS: [e.g. Ubuntu 22.04, macOS 13.0, Windows 11]
- Python Version: [e.g. 3.11.5]
- ROCK Version: [e.g. 0.2.0]
- Installation Method: [e.g. pip install rl-rock, source installation with uv]
- Docker Version: [e.g. 24.0.6] (if using sandbox features)
- Deployment Type: [e.g. local, distributed, ray]
ROCK Configuration
- Runtime Environment Type: [e.g. uv, pip, conda]
- Sandbox Image: [e.g. python:3.11, custom image]
- Resource Allocation: [e.g. memory=8g, cpus=2.0]
Component Affected
- Sandbox
- Actions
- Deployments
- SDK & API
- Envhub
- CLI
- Performance & Optimization
- Documentation & Examples
Suggested Fix
- Add try/catch for JSON parsing:
async def parse_request_line(self, line_content: str) -> tuple[str, dict]:
if SESSION_END_MARKER in line_content:
return SESSION_END_MARKER, {}
try:
meta_json = line_content.split(REQUEST_END_MARKER)[1]
request_json = line_content.split(REQUEST_END_MARKER)[0].split(REQUEST_START_MARKER)[1]
meta = json.loads(meta_json)
return request_json, meta
except (IndexError, json.JSONDecodeError) as e:
logger.error(f"Failed to parse request line: {line_content!r}, error: {e}")
raise ValueError(f"Invalid request line format: {e}") from e- Use
aiofilesfor async file I/O:
import aiofiles
async def read_last_request_line(self) -> str:
async with aiofiles.open(self.log_file) as f:
lines = await f.readlines()
# ... rest of the logicOr alternatively, use asyncio.to_thread() to wrap synchronous I/O:
async def read_last_request_line(self) -> str:
def _sync_read():
with open(self.log_file) as f:
return f.readlines()
lines = await asyncio.to_thread(_sync_read)
# ... rest of the logic