diff --git a/src/python/pants/base/exception_sink.py b/src/python/pants/base/exception_sink.py index 840c7b90029..847437f7360 100644 --- a/src/python/pants/base/exception_sink.py +++ b/src/python/pants/base/exception_sink.py @@ -221,7 +221,7 @@ def exceptions_log_path(cls, for_pid=None, in_dir=None): assert isinstance(for_pid, Pid) intermediate_filename_component = f".{for_pid}" in_dir = in_dir or cls._log_dir - return os.path.join(in_dir, ".pids", f"exceptions{intermediate_filename_component}.log") + return os.path.join(in_dir, f"exceptions{intermediate_filename_component}.log") @classmethod def _log_exception(cls, msg): diff --git a/src/python/pants/base/exception_sink_integration_test.py b/src/python/pants/base/exception_sink_integration_test.py index fa212b97695..0e3d09e4cc8 100644 --- a/src/python/pants/base/exception_sink_integration_test.py +++ b/src/python/pants/base/exception_sink_integration_test.py @@ -6,7 +6,6 @@ import signal import time from pathlib import Path -from textwrap import dedent from typing import List, Tuple import pytest @@ -49,7 +48,7 @@ def get_log_file_paths(workdir: str, pid: int) -> Tuple[str, str]: return (pid_specific_log_file, shared_log_file) -def assert_unhandled_exception_log_matches(pid: int, file_contents: str, namespace: str) -> None: +def assert_unhandled_exception_log_matches(pid: int, file_contents: str) -> None: regex_str = f"""\ timestamp: ([^\n]+) process title: ([^\n]+) @@ -58,9 +57,7 @@ def assert_unhandled_exception_log_matches(pid: int, file_contents: str, namespa Exception caught: \\([^)]*\\) (.|\n)* -Exception message:.* 1 Exception encountered: - - ResolveError: 'this-target-does-not-exist' was not found in namespace '{namespace}'\\. Did you mean one of: +Exception message:.* """ assert re.match(regex_str, file_contents) @@ -79,72 +76,22 @@ def assert_graceful_signal_log_matches(pid: int, signum, signame, contents: str) def test_logs_unhandled_exception(tmp_path: Path) -> None: - directory = "testprojects/src/python/hello/main" - pants_run = run_pants_with_workdir( - [ - "--no-pantsd", - "list", - f"{directory}:this-target-does-not-exist", - "--backend-packages=['pants.backend.python']", - ], + lifecycle_stub_cmdline(), workdir=tmp_path.as_posix(), # The backtrace should be omitted when --print-stacktrace=False. print_stacktrace=False, - hermetic=False, + extra_env={"_RAISE_EXCEPTION_ON_IMPORT": "True"}, ) pants_run.assert_failure() - regex = f"'this-target-does-not-exist' was not found in namespace '{directory}'\\. Did you mean one of:" + regex = "exception during import!" assert re.search(regex, pants_run.stderr) pid_specific_log_file, shared_log_file = get_log_file_paths(tmp_path.as_posix(), pants_run.pid) - assert_unhandled_exception_log_matches( - pants_run.pid, read_file(pid_specific_log_file), namespace=directory - ) - assert_unhandled_exception_log_matches( - pants_run.pid, read_file(shared_log_file), namespace=directory - ) - - -@pytest.mark.skip(reason="Flaky: https://github.com/pantsbuild/pants/issues/12108") -def test_fails_ctrl_c_on_import(tmp_path: Path) -> None: - # TODO: figure out the cwd of the pants subprocess, not just the "workdir"! - pants_run = run_pants_with_workdir( - lifecycle_stub_cmdline(), - workdir=tmp_path.as_posix(), - extra_env={"_RAISE_KEYBOARDINTERRUPT_ON_IMPORT": "True"}, - ) - pants_run.assert_failure() - - assert ( - dedent( - """\ - Interrupted by user: - ctrl-c during import! - """ - ) - in pants_run.stderr - ) - - pid_specific_log_file, shared_log_file = get_log_file_paths(tmp_path.as_posix(), pants_run.pid) - assert "" == read_file(pid_specific_log_file) - assert "" == read_file(shared_log_file) - - -def test_fails_ctrl_c_ffi(tmp_path: Path) -> None: - pants_run = run_pants_with_workdir( - command=lifecycle_stub_cmdline(), - workdir=tmp_path.as_posix(), - extra_env={"_RAISE_KEYBOARD_INTERRUPT_FFI": "1"}, - ) - pants_run.assert_failure() - assert "KeyboardInterrupt: ctrl-c interrupted execution during FFI" in pants_run.stderr - - pid_specific_log_file, shared_log_file = get_log_file_paths(tmp_path.as_posix(), pants_run.pid) - assert "" == read_file(pid_specific_log_file) - assert "" == read_file(shared_log_file) + assert_unhandled_exception_log_matches(pants_run.pid, read_file(pid_specific_log_file)) + assert_unhandled_exception_log_matches(pants_run.pid, read_file(shared_log_file)) class ExceptionSinkIntegrationTest(PantsDaemonIntegrationTestBase): diff --git a/src/python/pants/base/exception_sink_test.py b/src/python/pants/base/exception_sink_test.py index 0e389a09ed2..53730ee7e26 100644 --- a/src/python/pants/base/exception_sink_test.py +++ b/src/python/pants/base/exception_sink_test.py @@ -57,11 +57,11 @@ def test_set_invalid_log_location(): ), Platform.linux_arm64: ( "Error opening fatal error log streams for log location '/': [Errno 13] Permission " - "denied: '/.pids'" + "denied:" ), Platform.linux_x86_64: ( "Error opening fatal error log streams for log location '/': [Errno 13] Permission " - "denied: '/.pids'" + "denied:" ), } assert match(Platform.current, err_str) in str(exc.value) @@ -83,7 +83,9 @@ def test_log_exception(): getproctitle_mock.assert_called_once() # This should have created two log files, one specific to the current pid. - assert os.listdir(tmpdir) == [".pids"] + logfiles = os.listdir(tmpdir) + assert len(logfiles) == 2 + assert "exceptions.log" in logfiles cur_process_error_log_path = ExceptionSink.exceptions_log_path(for_pid=pid, in_dir=tmpdir) assert os.path.isfile(cur_process_error_log_path) is True diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index f806e65774b..70f99c66171 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -8,7 +8,6 @@ from dataclasses import dataclass from pants.base.build_environment import get_buildroot -from pants.base.exception_sink import ExceptionSink from pants.base.exiter import PANTS_FAILED_EXIT_CODE, PANTS_SUCCEEDED_EXIT_CODE, ExitCode from pants.base.specs import Specs from pants.base.specs_parser import SpecsParser @@ -182,7 +181,7 @@ def _perform_run(self, goals: tuple[str, ...]) -> ExitCode: try: exit_code = self._perform_run_body(goals, poll=True) except ExecutionError as e: - logger.warning(e) + logger.error(e) iterations -= 1 return exit_code @@ -234,7 +233,7 @@ def _run_inner(self) -> ExitCode: try: return self._perform_run(goals) except Exception as e: - ExceptionSink.log_exception(e) + logger.error(e) return PANTS_FAILED_EXIT_CODE except KeyboardInterrupt: print("Interrupted by user.\n", file=sys.stderr) diff --git a/src/python/pants/bin/local_pants_runner_integration_test.py b/src/python/pants/bin/local_pants_runner_integration_test.py new file mode 100644 index 00000000000..3ad937fc455 --- /dev/null +++ b/src/python/pants/bin/local_pants_runner_integration_test.py @@ -0,0 +1,18 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +from typing import Sequence + +from pants.testutil.pants_integration_test import PantsResult, run_pants + + +def test_print_stacktrace() -> None: + def run(args: Sequence[str]) -> PantsResult: + return run_pants(command=[*args, "list", "definitely-does-not-exist::"]) + + no_print_stacktrace = run(["--no-print-stacktrace"]) + assert "Traceback" not in no_print_stacktrace.stderr + assert "traceback" not in no_print_stacktrace.stderr + + print_stacktrace = run(["--print-stacktrace"]) + assert "Traceback" in print_stacktrace.stderr diff --git a/src/python/pants/bin/pants_loader.py b/src/python/pants/bin/pants_loader.py index 174add12eda..9aeec941b74 100644 --- a/src/python/pants/bin/pants_loader.py +++ b/src/python/pants/bin/pants_loader.py @@ -3,7 +3,6 @@ import importlib import locale -import logging import os import sys import time @@ -87,7 +86,6 @@ def run_alternate_entrypoint(entrypoint: str) -> None: @staticmethod def run_default_entrypoint() -> None: - logger = logging.getLogger(__name__) with maybe_profiled(os.environ.get(PANTSC_PROFILE)): start_time = time.time() try: @@ -96,9 +94,6 @@ def run_default_entrypoint() -> None: except KeyboardInterrupt as e: print(f"Interrupted by user:\n{e}", file=sys.stderr) exit_code = PANTS_FAILED_EXIT_CODE - except Exception as e: - logger.exception(e) - exit_code = PANTS_FAILED_EXIT_CODE sys.exit(exit_code) @classmethod diff --git a/src/python/pants/engine/internals/selectors.py b/src/python/pants/engine/internals/selectors.py index 8427654c452..b56fa7386f8 100644 --- a/src/python/pants/engine/internals/selectors.py +++ b/src/python/pants/engine/internals/selectors.py @@ -5,7 +5,6 @@ import ast import itertools -import os from abc import ABCMeta from dataclasses import dataclass from functools import partial @@ -699,14 +698,9 @@ def __init__(self, *args: Any) -> None: self.params = tuple(args) -_RAISE_KEYBOARD_INTERRUPT = os.environ.get("_RAISE_KEYBOARD_INTERRUPT_FFI", None) - - def native_engine_generator_send( func, arg ) -> PyGeneratorResponseGet | PyGeneratorResponseGetMulti | PyGeneratorResponseBreak: - if _RAISE_KEYBOARD_INTERRUPT: - raise KeyboardInterrupt("ctrl-c interrupted execution during FFI (for testing purposes).") try: res = func.send(arg) # TODO: It isn't currently necessary to differentiate between `Get` and `Effect` here, as diff --git a/testprojects/pants-plugins/src/python/test_pants_plugin/register.py b/testprojects/pants-plugins/src/python/test_pants_plugin/register.py index c5e551bf443..dd6fc068a17 100644 --- a/testprojects/pants-plugins/src/python/test_pants_plugin/register.py +++ b/testprojects/pants-plugins/src/python/test_pants_plugin/register.py @@ -9,9 +9,9 @@ class LifecycleStubsSubsystem(GoalSubsystem): - """Configure workflows for lifecycle tests (Pants stopping and starting).""" name = "lifecycle-stub-goal" + help = """Configure workflows for lifecycle tests (Pants stopping and starting).""" @classmethod def register_options(cls, register): @@ -39,6 +39,5 @@ async def run_lifecycle_stubs(opts: LifecycleStubsSubsystem) -> LifecycleStubsGo def rules(): return collect_rules() - -if os.environ.get("_RAISE_KEYBOARDINTERRUPT_ON_IMPORT", False): - raise KeyboardInterrupt("ctrl-c during import!") +if os.environ.get("_RAISE_EXCEPTION_ON_IMPORT", False): + raise Exception("exception during import!")