Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix --no-print-stacktrace. (cherrypick of #13539) #13541

Merged
merged 1 commit into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/python/pants/base/exception_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
67 changes: 7 additions & 60 deletions src/python/pants/base/exception_sink_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import signal
import time
from pathlib import Path
from textwrap import dedent
from typing import List, Tuple

import pytest
Expand Down Expand Up @@ -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]+)
Expand All @@ -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)

Expand All @@ -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):
Expand Down
8 changes: 5 additions & 3 deletions src/python/pants/base/exception_sink_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions src/python/pants/bin/local_pants_runner_integration_test.py
Original file line number Diff line number Diff line change
@@ -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
5 changes: 0 additions & 5 deletions src/python/pants/bin/pants_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import importlib
import locale
import logging
import os
import sys
import time
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
6 changes: 0 additions & 6 deletions src/python/pants/engine/internals/selectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import ast
import itertools
import os
from abc import ABCMeta
from dataclasses import dataclass
from functools import partial
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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!")