Skip to content

Commit d4e2016

Browse files
abrichrclaude
andauthored
fix: don't take a screenshot at import time (#23)
recorder.py computed monitor dimensions via utils.take_screenshot() at module scope, so `import openadapt_capture` crashed in any headless environment whose display reported a zero-size region. This took down `openadapt version` and `openadapt doctor` (found via the new CLI smoke tests in OpenAdaptAI/OpenAdapt). Move the computation into the video- setup function that is its only consumer. Adds tests/test_headless_import.py: a deterministic AST guard that no package module calls a display API (take_screenshot/get_monitor_dims/ grab) at import scope. A subprocess import test is unreliable (only reproduces on a genuinely headless display); this fails regardless of environment. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent cd7ed78 commit d4e2016

2 files changed

Lines changed: 60 additions & 3 deletions

File tree

openadapt_capture/recorder.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,6 @@ def __bool__(self):
142142
stop_sequence_detected = False
143143
ws_server_instance = None
144144

145-
# TODO XXX replace with utils.get_monitor_dims() once fixed
146-
monitor_width, monitor_height = utils.take_screenshot().size
147-
148145

149146
def collect_stats(performance_snapshots: list[tracemalloc.Snapshot]) -> None:
150147
"""Collects and appends performance snapshots using tracemalloc.
@@ -546,6 +543,8 @@ def video_pre_callback(
546543
dict[str, Any]: The updated state.
547544
"""
548545
video_file_path = video.get_video_file_path(recording.timestamp, video_dir)
546+
# TODO XXX replace with utils.get_monitor_dims() once fixed
547+
monitor_width, monitor_height = utils.take_screenshot().size
549548
video_container, video_stream, video_start_timestamp = (
550549
video.initialize_video_writer(video_file_path, monitor_width, monitor_height)
551550
)

tests/test_headless_import.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
"""Importing openadapt_capture must not call display APIs at module scope.
2+
3+
recorder.py used to compute monitor dimensions via a screenshot at
4+
module scope (`monitor_width, monitor_height = utils.take_screenshot()
5+
.size`), so `import openadapt_capture` crashed in any headless
6+
environment whose display reported a zero-size region, taking down
7+
`openadapt version`/`doctor` with it.
8+
9+
A subprocess import test is unreliable here (it only reproduces on a
10+
genuinely headless display), so this guards the invariant statically:
11+
no module-level call to a display/screenshot API in any package module.
12+
Importing a library should be cheap and side-effect free.
13+
"""
14+
15+
from __future__ import annotations
16+
17+
import ast
18+
from pathlib import Path
19+
20+
PACKAGE_ROOT = Path(__file__).resolve().parent.parent / "openadapt_capture"
21+
22+
# Calls that touch the screen/display and must not run at import time.
23+
FORBIDDEN_AT_MODULE_SCOPE = {"take_screenshot", "get_monitor_dims", "grab"}
24+
25+
26+
def _module_level_calls(tree: ast.Module):
27+
"""Yield Call nodes that execute at import (module body, not inside
28+
a function or class definition)."""
29+
for node in tree.body:
30+
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
31+
continue
32+
for sub in ast.walk(node):
33+
if isinstance(sub, ast.Call):
34+
yield sub
35+
36+
37+
def _called_name(call: ast.Call) -> str | None:
38+
func = call.func
39+
if isinstance(func, ast.Name):
40+
return func.id
41+
if isinstance(func, ast.Attribute):
42+
return func.attr
43+
return None
44+
45+
46+
def test_no_display_calls_at_module_scope():
47+
problems = []
48+
for path in PACKAGE_ROOT.rglob("*.py"):
49+
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
50+
for call in _module_level_calls(tree):
51+
name = _called_name(call)
52+
if name in FORBIDDEN_AT_MODULE_SCOPE:
53+
problems.append(f"{path}:{call.lineno}: module-level {name}()")
54+
assert not problems, (
55+
"Display/screenshot calls at import time break headless imports "
56+
"(CI, servers, containers). Move them inside the function that "
57+
"uses them:\n " + "\n ".join(problems)
58+
)

0 commit comments

Comments
 (0)