diff --git a/README.md b/README.md index 7e1d26e..a21535f 100644 --- a/README.md +++ b/README.md @@ -74,6 +74,11 @@ After completing the installation steps above, you can immediately test the syst python hello_world.py ``` +### Crash reporting + +If the plain2code client crashes unexpectedly, it sends an anonymous crash report to Codeplain to help improve the tool. To disable crash reporting, set the `CODEPLAIN_NO_TELEMETRY=1` flag. + + ## Releasing Releases are built and published with [uv](https://docs.astral.sh/uv/). The version is read from `_version.py`. diff --git a/plain2code.py b/plain2code.py index 2a822c9..6cfc857 100644 --- a/plain2code.py +++ b/plain2code.py @@ -49,6 +49,7 @@ get_log_file_path, ) from plain2code_state import RunState +from plain2code_telemetry import capture_crash, initialize_telemetry from system_config import system_config from tui.plain2code_tui import Plain2CodeTUI from tui.plain_module_render_choice_tui import PlainModuleRenderChoiceTUI @@ -56,6 +57,27 @@ DEFAULT_TEMPLATE_DIRS = importlib.resources.files("standard_template_library") RENDER_THREAD_SHUTDOWN_TIMEOUT = 0.7 +# Exceptions that represent expected, user-facing error conditions. They are +# reported to the user directly and must never be sent to Sentry as crashes. +EXPECTED_EXCEPTIONS = ( + InvalidFridArgument, + FileNotFoundError, + MissingResource, + TemplateNotFoundError, + PlainSyntaxError, + MissingPreviousFunctionalitiesError, + MissingAPIKey, + InvalidAPIKey, + OutdatedClientVersion, + ConflictingRequirements, + RenderingCreditBalanceTooLow, + NetworkConnectionError, + ModuleDoesNotExistError, + UnsupportedResourceType, + GitNotInstalledError, + SystemExit, +) + def setup_logging( args, @@ -324,6 +346,8 @@ def main(): # noqa: C901 args, event_bus, run_state, args.log_to_file, args.log_file_name, args.filename, args.headless ) + initialize_telemetry() + exc_info = None error_message = None @@ -340,36 +364,17 @@ def main(): # noqa: C901 else: error_message = str(e) if str(e) else repr(e) - if not isinstance( - e, - ( - InvalidFridArgument, - FileNotFoundError, - MissingResource, - TemplateNotFoundError, - PlainSyntaxError, - MissingPreviousFunctionalitiesError, - MissingAPIKey, - InvalidAPIKey, - OutdatedClientVersion, - ConflictingRequirements, - RenderingCreditBalanceTooLow, - NetworkConnectionError, - ModuleDoesNotExistError, - UnsupportedResourceType, - GitNotInstalledError, - ), - ): + if not isinstance(e, EXPECTED_EXCEPTIONS): exc_info = sys.exc_info() finally: + if exc_info: + dump_crash_logs(args, run_state) + capture_crash(exc_info, run_state, args) print_exit_summary( run_state, args.filename, error_message=error_message, ) - if exc_info: - # Log traceback - dump_crash_logs(args, run_state) if args.headless and (exc_info is not None or not run_state.render_succeeded): sys.exit(1) diff --git a/plain2code_state.py b/plain2code_state.py index d5a7a09..08268e2 100644 --- a/plain2code_state.py +++ b/plain2code_state.py @@ -23,6 +23,10 @@ def __init__(self, spec_filename: str, replay_with: Optional[str] = None): self.unittest_batch_id: int = 0 self.render_time_accumulated: int = 0 self.last_render_start_timestamp: float = time.monotonic() + # Mirrored from the render state machine for crash reporting; not authoritative. + self.current_module: Optional[str] = None + self.current_frid: Optional[str] = None + self.current_render_state: Optional[str] = None def increment_call_count(self): self.call_count += 1 diff --git a/plain2code_telemetry.py b/plain2code_telemetry.py new file mode 100644 index 0000000..cf3214d --- /dev/null +++ b/plain2code_telemetry.py @@ -0,0 +1,114 @@ +"""Crash reporting via Sentry. + +Only unexpected exceptions are reported (the caller decides which exceptions are +expected; see EXPECTED_EXCEPTIONS in plain2code.py). Reporting is on by default +and can be disabled by setting the CODEPLAIN_NO_TELEMETRY environment variable +to any non-empty value. +""" + +import os +from typing import Any, Optional + +import sentry_sdk +from sentry_sdk.integrations.atexit import AtexitIntegration +from sentry_sdk.integrations.dedupe import DedupeIntegration +from sentry_sdk.integrations.modules import ModulesIntegration +from sentry_sdk.scrubber import DEFAULT_DENYLIST, EventScrubber + +from plain2code_state import RunState +from system_config import system_config + +SENTRY_DSN = "https://64d0d86b50b34e2dede3e4eaf5142282@o4510793955934208.ingest.us.sentry.io/4511540621213696" + +NO_TELEMETRY_ENV_VAR = "CODEPLAIN_NO_TELEMETRY" +ENVIRONMENT_ENV_VAR = "CODEPLAIN_ENV" +DEFAULT_ENVIRONMENT = "production" + +FLUSH_TIMEOUT_SECONDS = 2 + +# Local variable names whose values may contain proprietary spec or generated +# code content and must be scrubbed from stack traces (extends Sentry's +# default denylist, which already covers api_key, auth, secrets etc.). +# "headers" and "x-api-key" cover the request headers local in +# codeplain_REST_api.post_request; the default denylist only has the +# underscore form "x_api_key". +SCRUB_DENYLIST = DEFAULT_DENYLIST + [ + "authorization", + "headers", + "x-api-key", + "plain_source", + "plain_source_tree", + "full_plain_source", + "existing_files_content", + "file_content", + "files_content", + "content", + "source", + "response_json", + "payload", +] + + +def telemetry_enabled() -> bool: + """Return True if crash reporting should be active.""" + if os.environ.get(NO_TELEMETRY_ENV_VAR): + return False + return True + + +def initialize_telemetry(**init_overrides: Any) -> bool: + """Initialize Sentry crash reporting. Returns True if initialized.""" + if not telemetry_enabled(): + return False + + try: + init_kwargs: dict[str, Any] = dict( + dsn=SENTRY_DSN, + release=system_config.client_version, + environment=os.environ.get(ENVIRONMENT_ENV_VAR, DEFAULT_ENVIRONMENT), + send_default_pii=False, + server_name="", # hostname is identifying; don't send it + default_integrations=False, + auto_enabling_integrations=False, + integrations=[ + AtexitIntegration(callback=lambda pending, timeout: None), + DedupeIntegration(), + ModulesIntegration(), + ], + include_local_variables=True, + event_scrubber=EventScrubber(denylist=SCRUB_DENYLIST, recursive=True), + shutdown_timeout=FLUSH_TIMEOUT_SECONDS, + ) + init_kwargs.update(init_overrides) + sentry_sdk.init(**init_kwargs) + return True + except Exception: + return False + + +def capture_crash(exc_info, run_state: Optional[RunState], args) -> bool: + """Report an unexpected crash to Sentry. Returns True if an event was sent.""" + if not telemetry_enabled(): + return False + + try: + with sentry_sdk.new_scope() as scope: + if run_state is not None: + scope.set_tag("render_id", run_state.render_id) + scope.set_tag("render_state", run_state.current_render_state) + scope.set_tag("current_module", run_state.current_module) + scope.set_tag("current_frid", run_state.current_frid) + scope.set_tag("headless", bool(getattr(args, "headless", False))) + scope.set_tag("unittests_script_provided", bool(getattr(args, "unittests_script", None))) + scope.set_tag("conformance_tests_script_provided", bool(getattr(args, "conformance_tests_script", None))) + scope.set_tag( + "prepare_environment_script_provided", bool(getattr(args, "prepare_environment_script", None)) + ) + + event_id = sentry_sdk.capture_exception(exc_info[1]) + + sentry_sdk.flush(timeout=FLUSH_TIMEOUT_SECONDS) + return event_id is not None + except Exception: + # Telemetry must never break the CLI or mask the original crash. + return False diff --git a/pyproject.toml b/pyproject.toml index 6e4dba3..22d835b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,7 @@ dependencies = [ "rich==15.0.0", "python-frontmatter==1.3.0", "networkx==3.6.1", + "sentry-sdk==2.62.0", ] [project.optional-dependencies] diff --git a/render_machine/code_renderer.py b/render_machine/code_renderer.py index 925832a..cb8de1a 100644 --- a/render_machine/code_renderer.py +++ b/render_machine/code_renderer.py @@ -42,6 +42,7 @@ def __init__(self, render_context: RenderContext): def run(self): """Execute the main rendering workflow.""" self.render_context.event_bus.publish(RenderModuleStarted(module_name=self.render_context.module_name)) + self.render_context.run_state.current_module = self.render_context.module_name previous_action_payload = None previous_state = None @@ -63,6 +64,7 @@ def run(self): ) ) previous_state = deepcopy(self.render_context.state) + self.render_context.run_state.current_render_state = self.render_context.state self.render_context.script_execution_history.should_update_script_outputs = False self.render_context.previous_action_payload = previous_action_payload diff --git a/render_machine/render_context.py b/render_machine/render_context.py index ecda27f..e7daf2e 100644 --- a/render_machine/render_context.py +++ b/render_machine/render_context.py @@ -162,6 +162,7 @@ def start_implementing_frid(self): linked_resources=linked_resources, functional_requirement_render_attempts=0, ) + self.run_state.current_frid = frid return def check_frid_iteration_limit(self): diff --git a/requirements.txt b/requirements.txt index cf65824..42efdce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ pytest==8.3.4 textual>=7.5.0 networkx==3.6.1 transitions==0.9.3 +sentry-sdk==2.62.0 # Development dependencies diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py new file mode 100644 index 0000000..3a70498 --- /dev/null +++ b/tests/test_telemetry.py @@ -0,0 +1,204 @@ +import json +import sys +from argparse import Namespace + +import pytest +import sentry_sdk +from sentry_sdk.envelope import Envelope +from sentry_sdk.transport import Transport + +import plain2code_telemetry +from plain2code_state import RunState +from plain2code_telemetry import NO_TELEMETRY_ENV_VAR, capture_crash, initialize_telemetry, telemetry_enabled + + +class CaptureTransport(Transport): + """Transport that records events instead of sending them over the network.""" + + def __init__(self, options=None): + super().__init__(options) + self.events = [] + + def capture_envelope(self, envelope: Envelope): + event = envelope.get_event() + if event is not None: + self.events.append(event) + + +def make_exc_info(exception): + try: + raise exception + except type(exception): + return sys.exc_info() + + +def make_args(**overrides): + args = Namespace( + headless=False, + unittests_script="run_unittests.sh", + conformance_tests_script=None, + prepare_environment_script=None, + ) + for key, value in overrides.items(): + setattr(args, key, value) + return args + + +@pytest.fixture(autouse=True) +def clean_telemetry_env(monkeypatch): + """Ensure tests are not affected by the developer's environment and never send real events.""" + monkeypatch.delenv(NO_TELEMETRY_ENV_VAR, raising=False) + monkeypatch.delenv(plain2code_telemetry.ENVIRONMENT_ENV_VAR, raising=False) + yield + client = sentry_sdk.get_client() + if client.is_active(): + client.close(timeout=0) + + +@pytest.fixture +def transport(): + return CaptureTransport() + + +def init_with_transport(transport): + assert initialize_telemetry(transport=transport) + + +def test_no_telemetry_env_var_disables(monkeypatch, transport): + monkeypatch.setenv(NO_TELEMETRY_ENV_VAR, "1") + + assert not telemetry_enabled() + assert not initialize_telemetry(transport=transport) + assert not capture_crash(make_exc_info(KeyError("boom")), None, make_args()) + assert transport.events == [] + + +def test_capture_crash_sends_event_with_tags(transport): + init_with_transport(transport) + + run_state = RunState(spec_filename="test.plain") + run_state.current_module = "my_module" + run_state.current_frid = "2.1" + run_state.current_render_state = "IMPLEMENTING_FRID" + + assert capture_crash(make_exc_info(KeyError("boom")), run_state, make_args(headless=True)) + sentry_sdk.flush(timeout=2) + + assert len(transport.events) == 1 + tags = transport.events[0]["tags"] + assert tags["render_id"] == run_state.render_id + assert tags["current_module"] == "my_module" + assert tags["current_frid"] == "2.1" + assert tags["render_state"] == "IMPLEMENTING_FRID" + assert tags["headless"] is True + assert tags["unittests_script_provided"] is True + assert tags["conformance_tests_script_provided"] is False + assert tags["prepare_environment_script_provided"] is False + + +def test_capture_crash_without_run_state(transport): + init_with_transport(transport) + + assert capture_crash(make_exc_info(ValueError("boom")), None, make_args()) + sentry_sdk.flush(timeout=2) + + assert len(transport.events) == 1 + assert "render_id" not in transport.events[0]["tags"] + + +def test_local_variables_are_scrubbed(transport): + init_with_transport(transport) + + def crash_with_sensitive_locals(): + api_key = "super-secret-key" # noqa: F841 + plain_source = "proprietary spec content" # noqa: F841 + raise KeyError("boom") + + try: + crash_with_sensitive_locals() + except KeyError: + exc_info = sys.exc_info() + + assert capture_crash(exc_info, None, make_args()) + sentry_sdk.flush(timeout=2) + + frames = transport.events[0]["exception"]["values"][0]["stacktrace"]["frames"] + crash_frame_vars = frames[-1]["vars"] + assert crash_frame_vars["api_key"] == "[Filtered]" + assert crash_frame_vars["plain_source"] == "[Filtered]" + + +def test_request_headers_local_is_scrubbed(transport): + """The `headers` local in codeplain_REST_api.post_request holds the API key + (X-API-Key); the whole variable must be filtered from stack traces.""" + init_with_transport(transport) + + # Built at runtime so the secret never appears in the source-context lines + # that Sentry attaches to stack frames. + secret = "".join(["super", "-secret-", "key"]) + + def crash_with_headers_local(): + headers = {"X-API-Key": secret, "Content-Type": "application/json"} # noqa: F841 + raise KeyError("boom") + + try: + crash_with_headers_local() + except KeyError: + exc_info = sys.exc_info() + + assert capture_crash(exc_info, None, make_args()) + sentry_sdk.flush(timeout=2) + + frames = transport.events[0]["exception"]["values"][0]["stacktrace"]["frames"] + crash_frame_vars = frames[-1]["vars"] + assert crash_frame_vars["headers"] == "[Filtered]" + assert secret not in json.dumps(transport.events[0], default=str) + + +def test_nested_sensitive_keys_are_scrubbed(transport): + """Scrubbing is recursive: sensitive keys nested inside dict locals are + filtered even when the variable name itself is innocuous.""" + init_with_transport(transport) + + secret = "".join(["nested", "-secret-", "key"]) + + def crash_with_nested_secret(): + request_info = {"url": "https://api.codeplain.ai", "x-api-key": secret} # noqa: F841 + raise KeyError("boom") + + try: + crash_with_nested_secret() + except KeyError: + exc_info = sys.exc_info() + + assert capture_crash(exc_info, None, make_args()) + sentry_sdk.flush(timeout=2) + + assert secret not in json.dumps(transport.events[0], default=str) + + +def test_environment_defaults_to_production(transport): + init_with_transport(transport) + assert sentry_sdk.get_client().options["environment"] == "production" + + +def test_environment_env_var_respected(monkeypatch, transport): + monkeypatch.setenv(plain2code_telemetry.ENVIRONMENT_ENV_VAR, "development") + init_with_transport(transport) + assert sentry_sdk.get_client().options["environment"] == "development" + + +def test_release_is_client_version(transport): + from system_config import system_config + + init_with_transport(transport) + assert sentry_sdk.get_client().options["release"] == system_config.client_version + + +def test_capture_crash_never_raises(monkeypatch): + monkeypatch.setattr( + sentry_sdk, "capture_exception", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("sdk broken")) + ) + init_with_transport(CaptureTransport()) + + assert capture_crash(make_exc_info(KeyError("boom")), None, make_args()) is False