Fix --no-print-stacktrace. (pantsbuild#13539)

Unfortunately, it looks like `--no-print-stacktrace` broke in a critical location a while back, resulting in unnecessary noise. See for example: pantsbuild#13491 (comment)

Fix, and add an integration test. Also fixes the location of the `exceptions.log`, and removes `ExceptionSink` `Ctrl+C` tests, since `Ctrl+C` is now directly handled by the engine (fixes pantsbuild#12108).

[ci skip-rust]
[ci skip-build-wheels]

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):

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):

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

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)

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

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

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

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!")