diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b95e2f4b..fa83e705 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,6 +55,15 @@ On Linux or macOS: .../debugpy$ python3 -m black ``` +## Type checking +We use [Pyright](https://github.com/microsoft/pyright) for static type checking, configured under `[tool.pyright]` in [pyproject.toml](pyproject.toml). It runs in `standard` type-checking mode, and `# type: ignore` comments are disabled repository-wide (`enableTypeIgnoreComments = false`). + +Because `# type: ignore` is disabled, it has no effect and must not be used to suppress diagnostics. When a specific diagnostic genuinely needs to be suppressed, use a targeted Pyright rule suppression instead, and keep it as narrow as possible: +```python +some_expression # pyright: ignore[reportSomeSpecificRule] +``` +Prefer fixing the underlying typing issue (e.g. adding an annotation or an explicit assertion that narrows the type) over suppressing it. Reserve `# pyright: ignore[...]` for cases where the code is correct but the type checker cannot verify it (such as access to private/stable runtime APIs). + ## Running tests We use tox to run tests in an isolated environment. This ensures that debugpy is first built as a package, and tox also takes care of installing all the test prerequisites into the environment. On Windows: @@ -91,7 +100,7 @@ The tests are run concurrently, and the default number of workers is 8. You can ### Running tests without tox -While tox is the recommended way to run the test suite, pytest can also be invoked directly from the root of the repository. This requires packages in tests/requirements.txt to be installed first. +While tox is the recommended way to run the test suite, pytest can also be invoked directly from the root (src/debugpy) of the repository. This requires packages in tests/requirements.txt to be installed first. Using a venv created by tox in the '.tox' folder can make it easier to get the pytest configuration correct. Debugpy needs to be installed into the venv for the tests to run, so using the tox generated .venv makes that easier. diff --git a/pyproject.toml b/pyproject.toml index bbba5ea3..80bbe2b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,6 +21,8 @@ ignore = ["src/debugpy/_vendored/pydevd", "src/debugpy/_version.py"] executionEnvironments = [ { root = "src" }, { root = "." } ] +typeCheckingMode = "standard" +enableTypeIgnoreComments = false [tool.ruff] # Enable the pycodestyle (`E`) and Pyflakes (`F`) rules by default. diff --git a/src/debugpy/adapter/__main__.py b/src/debugpy/adapter/__main__.py index a6705c06..916a42a1 100644 --- a/src/debugpy/adapter/__main__.py +++ b/src/debugpy/adapter/__main__.py @@ -8,6 +8,7 @@ import locale import os import sys +from typing import Any # WARNING: debugpy and submodules must not be imported on top level in this module, # and should be imported locally inside main() instead. @@ -55,7 +56,7 @@ def main(): if args.for_server is None: adapter.access_token = codecs.encode(os.urandom(32), "hex").decode("ascii") - endpoints = {} + endpoints: dict[str, Any] = {} try: client_host, client_port = clients.serve(args.host, args.port) except Exception as exc: diff --git a/src/debugpy/adapter/clients.py b/src/debugpy/adapter/clients.py index d3314216..78a1327e 100644 --- a/src/debugpy/adapter/clients.py +++ b/src/debugpy/adapter/clients.py @@ -7,13 +7,13 @@ import atexit import os import sys +from typing import Any, Callable, Union, cast import debugpy from debugpy import adapter, common, launcher from debugpy.common import json, log, messaging, sockets from debugpy.adapter import clients, components, launchers, servers, sessions - class Client(components.Component): """Handles the client side of a debug session.""" @@ -67,7 +67,7 @@ def __init__(self, sock): fully handled. """ - self.start_request = None + self.start_request: Union[messaging.Request, None] = None """The "launch" or "attach" request as received from the client. """ @@ -124,11 +124,12 @@ def propagate_after_start(self, event): self.client.channel.propagate(event) def _propagate_deferred_events(self): - log.debug("Propagating deferred events to {0}...", self.client) - for event in self._deferred_events: - log.debug("Propagating deferred {0}", event.describe()) - self.client.channel.propagate(event) - log.info("All deferred events propagated to {0}.", self.client) + if self._deferred_events is not None: + log.debug("Propagating deferred events to {0}...", self.client) + for event in self._deferred_events: + log.debug("Propagating deferred {0}", event.describe()) + self.client.channel.propagate(event) + log.info("All deferred events propagated to {0}.", self.client) self._deferred_events = None # Generic event handler. There are no specific handlers for client events, because @@ -203,9 +204,12 @@ def initialize_request(self, request): # # See https://github.com/microsoft/vscode/issues/4902#issuecomment-368583522 # for the sequence of request and events necessary to orchestrate the start. - def _start_message_handler(f): + @staticmethod + def _start_message_handler( + f: Callable[..., Any], + ) -> Callable[..., object | None]: @components.Component.message_handler - def handle(self, request): + def handle(self, request: messaging.Request): assert request.is_request("launch", "attach") if self._initialize_request is None: raise request.isnt_valid("Session is not initialized yet") @@ -216,8 +220,9 @@ def handle(self, request): if self.session.no_debug: servers.dont_wait_for_first_connection() + request_options: list[Any] = cast("list[Any]", request("debugOptions", json.array(str))) self.session.debug_options = debug_options = set( - request("debugOptions", json.array(str)) + request_options ) f(self, request) @@ -336,6 +341,7 @@ def property_or_debug_option(prop_name, flag_name): launcher_python = python[0] program = module = code = () + args = [] if "program" in request: program = request("program", str) args = [program] @@ -392,7 +398,7 @@ def property_or_debug_option(prop_name, flag_name): if cwd == (): # If it's not specified, but we're launching a file rather than a module, # and the specified path has a directory in it, use that. - cwd = None if program == () else (os.path.dirname(program) or None) + cwd = None if program == () else (os.path.dirname(str(program)) or None) sudo = bool(property_or_debug_option("sudo", "Sudo")) if sudo and sys.platform == "win32": @@ -487,6 +493,9 @@ def attach_request(self, request): else: if not servers.is_serving(): servers.serve(localhost) + # servers.serve() above guarantees a listener; fail fast if it's missing + # rather than handing the debuggee an empty ("", 0) address it can't use. + assert servers.listener is not None host, port = sockets.get_address(servers.listener) # There are four distinct possibilities here. @@ -584,9 +593,9 @@ def on_output(category, output): request.cant_handle("{0} is already being debugged.", conn) @message_handler - def configurationDone_request(self, request): + def configurationDone_request(self, request: messaging.Request): if self.start_request is None or self.has_started: - request.cant_handle( + raise request.cant_handle( '"configurationDone" is only allowed during handling of a "launch" ' 'or an "attach" request' ) @@ -627,6 +636,10 @@ def configurationDone_request(self, request): @message_handler def evaluate_request(self, request): propagated_request = self.server.channel.propagate(request) + if propagated_request is None: + raise request.cant_handle( + '"{0}" could not be propagated to the debug server', request.command + ) def handle_response(response): request.respond(response.body) @@ -657,7 +670,7 @@ def debugpySystemInfo_request(self, request): result = {"debugpy": {"version": debugpy.__version__}} if self.server: try: - pydevd_info = self.server.channel.request("pydevdSystemInfo") + pydevd_info: messaging.MessageDict = self.server.channel.request("pydevdSystemInfo") except Exception: # If the server has already disconnected, or couldn't handle it, # report what we've got. @@ -770,6 +783,10 @@ def notify_of_subprocess(self, conn): body["connect"]["host"] = host or localhost if "port" not in body["connect"]: if port is None: + # A subprocess is only reported once the client is connected, so the + # client listener must be serving; fail fast rather than sending the + # child a null port it can't connect to. + assert listener is not None _, port = sockets.get_address(listener) body["connect"]["port"] = port diff --git a/src/debugpy/adapter/components.py b/src/debugpy/adapter/components.py index 1a653407..3f35ab5a 100644 --- a/src/debugpy/adapter/components.py +++ b/src/debugpy/adapter/components.py @@ -3,7 +3,12 @@ # for license information. import functools +from typing import TYPE_CHECKING, Type, TypeVar, Union, cast +if TYPE_CHECKING: + # Dont import this during runtime. There's an order + # of imports issue that causes the debugger to hang. + from debugpy.adapter.sessions import Session from debugpy.common import json, log, messaging, util @@ -31,7 +36,7 @@ class Component(util.Observable): to wait_for() a change caused by another component. """ - def __init__(self, session, stream=None, channel=None): + def __init__(self, session: "Session", stream: "Union[messaging.JsonIOStream, None]"=None, channel: "Union[messaging.JsonMessageChannel, None]"=None): assert (stream is None) ^ (channel is None) try: @@ -44,18 +49,19 @@ def __init__(self, session, stream=None, channel=None): self.session = session - if channel is None: + if channel is None and stream is not None: stream.name = str(self) channel = messaging.JsonMessageChannel(stream, self) channel.start() - else: + elif channel is not None: channel.name = channel.stream.name = str(self) channel.handlers = self + assert channel is not None self.channel = channel self.is_connected = True # Do this last to avoid triggering useless notifications for assignments above. - self.observers += [lambda *_: self.session.notify_changed()] + self.observers = [*self.observers, lambda *_: self.session.notify_changed()] def __str__(self): return f"{type(self).__name__}[{self.session.id}]" @@ -108,8 +114,9 @@ def disconnect(self): self.is_connected = False self.session.finalize("{0} has disconnected".format(self)) +T = TypeVar('T') -def missing(session, type): +def missing(session, type: Type[T]) -> T: class Missing(object): """A dummy component that raises ComponentNotAvailable whenever some attribute is accessed on it. @@ -124,7 +131,7 @@ def report(): except Exception as exc: log.reraise_exception("{0} in {1}", exc, session) - return Missing() + return cast(T, Missing()) class Capabilities(dict): diff --git a/src/debugpy/adapter/launchers.py b/src/debugpy/adapter/launchers.py index 454d9e25..b855fe83 100644 --- a/src/debugpy/adapter/launchers.py +++ b/src/debugpy/adapter/launchers.py @@ -3,6 +3,7 @@ # for license information. import os +import socket import subprocess import sys @@ -18,7 +19,7 @@ class Launcher(components.Component): message_handler = components.Component.message_handler - def __init__(self, session, stream): + def __init__(self, session: sessions.Session, stream): with session: assert not session.launcher super().__init__(session, stream) @@ -89,11 +90,15 @@ def spawn_debuggee( arguments = dict(start_request.arguments) if not session.no_debug: + # For a debug launch the server listener must already be up; fail fast + # rather than silently spawning a debuggee that can't connect back. + assert servers.listener is not None _, arguments["port"] = sockets.get_address(servers.listener) arguments["adapterAccessToken"] = adapter.access_token - def on_launcher_connected(sock): - listener.close() + def on_launcher_connected(sock: socket.socket): + if listener is not None: + listener.close() stream = messaging.JsonIOStream.from_socket(sock) Launcher(session, stream) diff --git a/src/debugpy/adapter/servers.py b/src/debugpy/adapter/servers.py index a4eb1f5c..147f0f8f 100644 --- a/src/debugpy/adapter/servers.py +++ b/src/debugpy/adapter/servers.py @@ -5,10 +5,12 @@ from __future__ import annotations import os +import socket import subprocess import sys import threading import time +from typing import Callable, Union, cast import debugpy from debugpy import adapter @@ -20,7 +22,7 @@ access_token = None """Access token used to authenticate with the servers.""" -listener = None +listener: Union[socket.socket, None] = None """Listener socket that accepts server connections.""" _lock = threading.RLock() @@ -60,7 +62,7 @@ class Connection(object): channel: messaging.JsonMessageChannel - def __init__(self, sock): + def __init__(self, sock: socket.socket): from debugpy.adapter import sessions self.disconnected = False @@ -78,7 +80,11 @@ def __init__(self, sock): try: self.authenticate() info = self.channel.request("pydevdSystemInfo") - process_info = info("process", json.object()) + # channel.request() either returns a non-exception result or raises, so + # assert (rather than silently skipping) to fail fast and to narrow the + # type, keeping pid/ppid unconditionally assigned before they're read. + assert not isinstance(info, Exception) + process_info: Callable[..., int] = cast(Callable[..., int], info("process", json.object())) self.pid = process_info("pid", int) self.ppid = process_info("ppid", int, optional=True) if self.ppid == (): @@ -181,6 +187,11 @@ def authenticate(self): auth = self.channel.request( "pydevdAuthorize", {"debugServerAccessToken": access_token} ) + # Fail closed: if the authorization request didn't yield a normal result, + # treat the server as unauthorized rather than skipping the token check. + if isinstance(auth, Exception): + self.channel.close() + raise RuntimeError("Failed to authorize with debug server.") from auth if auth["clientAccessToken"] != adapter.access_token: self.channel.close() raise RuntimeError('Mismatched "clientAccessToken"; server not authorized.') @@ -260,7 +271,7 @@ class Capabilities(components.Capabilities): "supportedChecksumAlgorithms": [], } - def __init__(self, session, connection): + def __init__(self, session: sessions.Session, connection): assert connection.server is None with session: assert not session.server @@ -293,12 +304,13 @@ def initialize(self, request): assert request.is_request("initialize") self.connection.authenticate() request = self.channel.propagate(request) - request.wait_for_response() - self.capabilities = self.Capabilities(self, request.response) + if request is not None: + request.wait_for_response() + self.capabilities = self.Capabilities(self, request.response) # Generic request handler, used if there's no specific handler below. @message_handler - def request(self, request): + def request(self, request: messaging.Message): # Do not delegate requests from the server by default. There is a security # boundary between the server and the adapter, and we cannot trust arbitrary # requests sent over that boundary, since they may contain arbitrary code @@ -428,21 +440,21 @@ def connections(): return list(_connections) -def wait_for_connection(session, predicate, timeout=None): +def wait_for_connection(session, predicate, timeout: Union[float, None]=None): """Waits until there is a server matching the specified predicate connected to this adapter, and returns the corresponding Connection. If there is more than one server connection already available, returns the oldest one. """ - def wait_for_timeout(): - time.sleep(timeout) - wait_for_timeout.timed_out = True + if timeout is not None: + time.sleep(timeout) + wait_for_timeout.timed_out = True # pyright: ignore[reportFunctionMemberAccess] with _lock: _connections_changed.set() - wait_for_timeout.timed_out = timeout == 0 + wait_for_timeout.timed_out = timeout == 0 # pyright: ignore[reportFunctionMemberAccess] if timeout: thread = threading.Thread( target=wait_for_timeout, name="servers.wait_for_connection() timeout" @@ -457,7 +469,7 @@ def wait_for_timeout(): _connections_changed.clear() conns = (conn for conn in _connections if predicate(conn)) conn = next(conns, None) - if conn is not None or wait_for_timeout.timed_out: + if conn is not None or wait_for_timeout.timed_out: # pyright: ignore[reportFunctionMemberAccess] return conn _connections_changed.wait() @@ -485,6 +497,10 @@ def dont_wait_for_first_connection(): def inject(pid, debugpy_args, on_output): + # inject() is only reached for attach-by-PID, where the server listener must be + # serving so the injected debug server can connect back. Fail fast if it isn't, + # rather than spawning an injector that connects to ":0" and never attaches. + assert listener is not None host, port = sockets.get_address(listener) cmdline = [ diff --git a/src/debugpy/adapter/sessions.py b/src/debugpy/adapter/sessions.py index ca87483f..7fd99b92 100644 --- a/src/debugpy/adapter/sessions.py +++ b/src/debugpy/adapter/sessions.py @@ -7,6 +7,7 @@ import signal import threading import time +from typing import Union from debugpy import common from debugpy.common import log, util @@ -26,6 +27,7 @@ class Session(util.Observable): """ _counter = itertools.count(1) + pid: Union[int, None] = None def __init__(self): from debugpy.adapter import clients @@ -61,7 +63,7 @@ def __init__(self): self.is_finalizing = False """Whether finalize() has been invoked.""" - self.observers += [lambda *_: self.notify_changed()] + self.observers = [*self.observers, lambda *_: self.notify_changed()] def __str__(self): return f"Session[{self.id}]" @@ -94,7 +96,7 @@ def notify_changed(self): _sessions.remove(self) _sessions_changed.set() - def wait_for(self, predicate, timeout=None): + def wait_for(self, predicate, timeout: Union[float, None]=None): """Waits until predicate() becomes true. The predicate is invoked with the session locked. If satisfied, the method @@ -111,13 +113,14 @@ def wait_for(self, predicate, timeout=None): seconds regardless of whether the predicate was satisfied. The method returns False if it timed out, and True otherwise. """ - def wait_for_timeout(): - time.sleep(timeout) - wait_for_timeout.timed_out = True + if timeout is not None: + time.sleep(timeout) + wait_for_timeout.timed_out = True # pyright: ignore[reportFunctionMemberAccess] self.notify_changed() - wait_for_timeout.timed_out = False + wait_for_timeout.timed_out = False # pyright: ignore[reportFunctionMemberAccess] + if timeout is not None: thread = threading.Thread( target=wait_for_timeout, name="Session.wait_for() timeout" @@ -127,7 +130,7 @@ def wait_for_timeout(): with self: while not predicate(): - if wait_for_timeout.timed_out: + if wait_for_timeout.timed_out: # pyright: ignore[reportFunctionMemberAccess] return False self._changed_condition.wait() return True @@ -180,7 +183,7 @@ def _finalize(self, why, terminate_debuggee): # can ask the launcher to kill it, do so instead of disconnecting # from the server to prevent debuggee from running any more code. self.launcher.terminate_debuggee() - else: + elif self.server.channel is not None: # Otherwise, let the server handle it the best it can. try: self.server.channel.request( @@ -218,7 +221,8 @@ def _finalize(self, why, terminate_debuggee): self.wait_for(lambda: not self.launcher.is_connected) try: - self.launcher.channel.close() + if self.launcher.channel is not None: + self.launcher.channel.close() except Exception: log.swallow_exception() @@ -230,7 +234,8 @@ def _finalize(self, why, terminate_debuggee): if self.client.restart_requested: body["restart"] = True try: - self.client.channel.send_event("terminated", body) + if self.client.channel is not None: + self.client.channel.send_event("terminated", body) except Exception: pass diff --git a/src/debugpy/common/json.py b/src/debugpy/common/json.py index 6f3e2b21..c86c49b3 100644 --- a/src/debugpy/common/json.py +++ b/src/debugpy/common/json.py @@ -7,8 +7,8 @@ import builtins import json -import numbers import operator +from typing import Any, Callable, Literal, Tuple, Union JsonDecoder = json.JSONDecoder @@ -21,14 +21,14 @@ class JsonEncoder(json.JSONEncoder): result is serialized instead of the object itself. """ - def default(self, value): + def default(self, o): try: - get_state = value.__getstate__ + get_state = o.__getstate__ except AttributeError: pass else: return get_state() - return super().default(value) + return super().default(o) class JsonObject(object): @@ -93,10 +93,13 @@ def __format__(self, format_spec): # some substitutions - e.g. replacing () with some default value. -def _converter(value, classinfo): +def _converter(value: str, classinfo) -> Union[int, float, None]: """Convert value (str) to number, otherwise return None if is not possible""" + # Only int/float are accepted here (DAP number payloads are int or float); + # this deliberately narrows from numbers.Number and does not handle + # Decimal/complex/Fraction, which never appear in DAP messages. for one_info in classinfo: - if issubclass(one_info, numbers.Number): + if issubclass(one_info, int) or issubclass(one_info, float): try: return one_info(value) except ValueError: @@ -171,7 +174,7 @@ def validate(value): return validate -def array(validate_item=False, vectorize=False, size=None): +def array(validate_item: Union[Callable[..., Any], Literal[False]]=False, vectorize=False, size=None): """Returns a validator for a JSON array. If the property is missing, it is treated as if it were []. Otherwise, it must @@ -213,11 +216,11 @@ def array(validate_item=False, vectorize=False, size=None): ) elif isinstance(size, tuple): assert 1 <= len(size) <= 2 - size = tuple(operator.index(n) for n in size) - min_len, max_len = (size + (None,))[0:2] + sizes = tuple(operator.index(n) for n in size) + min_len, max_len = (sizes + (None,))[0:2] validate_size = lambda value: ( "must have at least {0} elements".format(min_len) - if len(value) < min_len + if min_len is None or len(value) < min_len else "must have at most {0} elements".format(max_len) if max_len is not None and len(value) < max_len else True @@ -250,7 +253,7 @@ def validate(value): return validate -def object(validate_value=False): +def object(validate_value: Union[Callable[..., Any], Tuple, Literal[False]]=False): """Returns a validator for a JSON object. If the property is missing, it is treated as if it were {}. Otherwise, it must diff --git a/src/debugpy/common/log.py b/src/debugpy/common/log.py index fda91e1c..1dd827ab 100644 --- a/src/debugpy/common/log.py +++ b/src/debugpy/common/log.py @@ -13,6 +13,12 @@ import sys import threading import traceback +from typing import TYPE_CHECKING, Any, NoReturn, Protocol, Union + +if TYPE_CHECKING: + # Careful not force this import in production code, as it's not available in all + # code that we run. + from typing_extensions import TypeIs import debugpy from debugpy.common import json, timestamp, util @@ -122,7 +128,7 @@ def newline(level="info"): stderr.write(level, "\n") -def write(level, text, _to_files=all): +def write(level, text: str, _to_files=all): assert level in LEVELS t = timestamp.current() @@ -143,7 +149,7 @@ def write(level, text, _to_files=all): return text -def write_format(level, format_string, *args, **kwargs): +def write_format(level, format_string: str, *args, **kwargs) -> Union[str, None]: # Don't spend cycles doing expensive formatting if we don't have to. Errors are # always formatted, so that error() can return the text even if it's not logged. if level != "error" and level not in _levels: @@ -215,7 +221,7 @@ def swallow_exception(format_string="", *args, **kwargs): _exception(format_string, *args, **kwargs) -def reraise_exception(format_string="", *args, **kwargs): +def reraise_exception(format_string="", *args, **kwargs) -> NoReturn: """Like swallow_exception(), but re-raises the current exception after logging it.""" assert "exc_info" not in kwargs @@ -278,6 +284,15 @@ def prefixed(format_string, *args, **kwargs): finally: _tls.prefix = old_prefix +class HasNameAndVersion(Protocol): + name: str + version: str + +def has_name_and_version(obj: Any) -> "TypeIs[HasNameAndVersion]": + try: + return hasattr(obj, "name") and hasattr(obj, "version") + except NameError: + return False def get_environment_description(header): import sysconfig @@ -353,7 +368,10 @@ def report_paths(get_paths, label=None): report("Installed packages:\n") try: for pkg in importlib_metadata.distributions(): - report(" {0}=={1}\n", pkg.name, pkg.version) + if has_name_and_version(pkg): + report(" {0}=={1}\n", pkg.name, pkg.version) + else: + report(" {0}\n", pkg) except Exception: # pragma: no cover swallow_exception("Error while enumerating installed packages.", level="info") @@ -387,7 +405,8 @@ def _repr(value): # pragma: no cover def _vars(*names): # pragma: no cover - locals = inspect.currentframe().f_back.f_locals + frame = inspect.currentframe() + locals = frame.f_back.f_locals if frame is not None and frame.f_back is not None else {} if names: locals = {name: locals[name] for name in names if name in locals} warning("$VARS {0!r}", locals) diff --git a/src/debugpy/common/messaging.py b/src/debugpy/common/messaging.py index eb29c189..8ac0f8d9 100644 --- a/src/debugpy/common/messaging.py +++ b/src/debugpy/common/messaging.py @@ -14,11 +14,17 @@ import collections import contextlib import functools +import io import itertools import os import socket import sys import threading +from typing import TYPE_CHECKING, BinaryIO, Callable, ClassVar, Union, cast, Any +if TYPE_CHECKING: + # Careful not force this import in production code, as it's not available in all + # code that we run. + from typing_extensions import TypeIs from debugpy.common import json, log, util from debugpy.common.util import hide_thread_from_debugger @@ -86,7 +92,7 @@ def from_process(cls, process, name="stdio"): return cls(process.stdout, process.stdin, name) @classmethod - def from_socket(cls, sock, name=None): + def from_socket(cls: type[JsonIOStream], sock: socket.socket, name: Union[str, None]=None): """Creates a new instance that sends and receives messages over a socket.""" sock.settimeout(None) # make socket blocking if name is None: @@ -96,7 +102,7 @@ def from_socket(cls, sock, name=None): # sockets is very slow! Although the implementation of readline() itself is # native code, it calls read(1) in a loop - and that then ultimately calls # SocketIO.readinto(), which is implemented in Python. - socket_io = sock.makefile("rwb", 0) + socket_io: socket.SocketIO = sock.makefile("rwb", 0) # SocketIO.close() doesn't close the underlying socket. def cleanup(): @@ -108,7 +114,13 @@ def cleanup(): return cls(socket_io, socket_io, name, cleanup) - def __init__(self, reader, writer, name=None, cleanup=lambda: None): + def __init__( + self, + reader: Union[io.RawIOBase, BinaryIO], + writer: Union[io.RawIOBase, BinaryIO], + name: Union[str, None] = None, + cleanup=lambda: None, + ): """Creates a new JsonIOStream. reader must be a BytesIO-like object, from which incoming messages will be @@ -158,11 +170,13 @@ def close(self): except Exception: # pragma: no cover log.reraise_exception("Error while closing {0} message stream", self.name) - def _log_message(self, dir, data, logger=log.debug): + def _log_message( + self, dir, data, logger: Callable[..., Union[str, None]] = log.debug + ): return logger("{0} {1} {2}", self.name, dir, data) - def _read_line(self, reader): - line = b"" + def _read_line(self, reader: Union[io.RawIOBase, BinaryIO]) -> bytes: + line: bytes = b"" while True: try: line += reader.readline() @@ -202,6 +216,7 @@ def log_message_and_reraise_exception(format_string="", *args, **kwargs): raw_chunks = [] headers = {} + line: Union[bytes, None] = None while True: try: @@ -222,9 +237,12 @@ def log_message_and_reraise_exception(format_string="", *args, **kwargs): if line == b"": break - key, _, value = line.partition(b":") + key, _, value = ( + line.partition(b":") if line is not None else (b"", b"", b"") + ) headers[key] = value + length = 0 try: length = int(headers[b"Content-Length"]) if not (0 <= length <= self.MAX_BODY_SIZE): @@ -256,10 +274,11 @@ def log_message_and_reraise_exception(format_string="", *args, **kwargs): except Exception: # pragma: no cover log_message_and_reraise_exception() - try: - body = decoder.decode(body) - except Exception: # pragma: no cover - log_message_and_reraise_exception() + if isinstance(body, str): + try: + body = decoder.decode(body) + except Exception: # pragma: no cover + log_message_and_reraise_exception() # If parsed successfully, log as JSON for readability. self._log_message("-->", body) @@ -283,6 +302,7 @@ def write_json(self, value, encoder=None): # information as we already have at the point of the failure. For example, # if it fails after it is serialized to JSON, log that JSON. + body: Union[str, bytes] = "" try: body = encoder.encode(value) except Exception: # pragma: no cover @@ -326,7 +346,7 @@ class MessageDict(collections.OrderedDict): such guarantee for outgoing messages. """ - def __init__(self, message, items=None): + def __init__(self, message: Union[Message, None], items: Union[dict, None]=None): assert message is None or isinstance(message, Message) if items is None: @@ -384,19 +404,19 @@ def __call__(self, key, validate, optional=False): try: value = validate(value) except (TypeError, ValueError) as exc: - message = Message if self.message is None else self.message + message = Message.empty() if self.message is None else self.message err = str(exc) if not err.startswith("["): err = " " + err raise message.isnt_valid("{0}{1}", json.repr(key), err) return value - def _invalid_if_no_key(func): + def _invalid_if_no_key(func: Callable[..., Any]): # pyright: ignore[reportSelfClsParameterName] def wrap(self, key, *args, **kwargs): try: return func(self, key, *args, **kwargs) except KeyError: - message = Message if self.message is None else self.message + message = Message.empty() if self.message is None else self.message raise message.isnt_valid("missing property {0!r}", key) return wrap @@ -408,6 +428,24 @@ def wrap(self, key, *args, **kwargs): del _invalid_if_no_key +class AssociableMessageDict(MessageDict): + # When this dict is parsed as part of an incoming message, all dicts that + # belong to that message share this list, so associating the top-level dict + # associates every nested dict with the same Message. It stays None for + # synthesized payloads, which are associated individually. + associated_dicts: "list[AssociableMessageDict] | None" = None + + def associate_with(self, message: Message): + if self.associated_dicts is None: + self.message = message + return + for d in self.associated_dicts: + d.message = message + + +def is_associable(obj) -> "TypeIs[AssociableMessageDict]": + return isinstance(obj, AssociableMessageDict) + def _payload(value): """JSON validator for message payload. @@ -422,12 +460,7 @@ def _payload(value): # Missing payload. Construct a dummy MessageDict, and make it look like it was # deserialized. See JsonMessageChannel._parse_incoming_message for why it needs # to have associate_with(). - - def associate_with(message): - value.message = message - - value = MessageDict(None) - value.associate_with = associate_with + value = AssociableMessageDict(None) return value @@ -452,7 +485,7 @@ def __init__(self, channel, seq, json=None): """ def __str__(self): - return json.repr(self.json) if self.json is not None else repr(self) + return str(json.repr(self.json)) if self.json is not None else repr(self) def describe(self): """A brief description of the message that is enough to identify it. @@ -464,15 +497,22 @@ def describe(self): raise NotImplementedError @property - def payload(self) -> MessageDict: + def payload(self) -> MessageDict | Exception: """Payload of the message - self.body or self.arguments, depending on the message type. """ raise NotImplementedError - def __call__(self, *args, **kwargs): + def __call__(self, *args, **kwargs) -> MessageDict | Any | int | float: """Same as self.payload(...).""" - return self.payload(*args, **kwargs) + payload = self.payload + # Handle exception payloads explicitly rather than via assert, so behavior is + # consistent regardless of whether assertions are stripped under `python -O`. + if isinstance(payload, Exception): + raise payload + if len(args) == 0 and not kwargs: + return payload + return payload(*args, **kwargs) def __contains__(self, key): """Same as (key in self.payload).""" @@ -524,7 +564,10 @@ def isnt_valid(self, *args, **kwargs): def cant_handle(self, *args, **kwargs): """Same as self.error(MessageHandlingError, ...).""" return self.error(MessageHandlingError, *args, **kwargs) - + + @classmethod + def empty(cls) -> Message: + return Message(None, None) class Event(Message): """Represents an incoming event. @@ -551,12 +594,12 @@ class Event(Message): the appropriate exception type that applies_to() the Event object. """ - def __init__(self, channel, seq, event, body, json=None): + def __init__(self, channel, seq, event, body: MessageDict, json=None): super().__init__(channel, seq, json) self.event = event - if isinstance(body, MessageDict) and hasattr(body, "associate_with"): + if is_associable(body): body.associate_with(self) self.body = body @@ -645,16 +688,16 @@ class Request(Message): the appropriate exception type that applies_to() the Request object. """ - def __init__(self, channel, seq, command, arguments, json=None): + def __init__(self, channel, seq, command, arguments: MessageDict, json=None): super().__init__(channel, seq, json) self.command = command - if isinstance(arguments, MessageDict) and hasattr(arguments, "associate_with"): + if is_associable(arguments): arguments.associate_with(self) self.arguments = arguments - self.response = None + self.response: Union[Response, None] = None """Response to this request. For incoming requests, it is set as soon as the request handler returns. @@ -684,6 +727,11 @@ def respond(self, body): with self.channel._send_message(d) as seq: pass + if body is None: + # A successful response with no body is modeled as an empty payload so + # that Response.body is always a MessageDict or an Exception, matching + # how incoming empty responses are parsed. + body = _payload(None) self.response = Response(self.channel, seq, self, body) @staticmethod @@ -754,7 +802,12 @@ class OutgoingRequest(Request): response to be received, and register a response handler. """ - _parse = _handle = None + # Outgoing requests are never parsed or handled as incoming messages, so the + # inherited _parse/_handle are explicitly disabled. Declared as class-level + # optional attributes (rather than set via setattr) so the override stays + # statically visible to the type checker. + _parse: ClassVar[None] = None # pyright: ignore[reportIncompatibleMethodOverride] + _handle: ClassVar[None] = None # pyright: ignore[reportIncompatibleMethodOverride] def __init__(self, channel, seq, command, arguments): super().__init__(channel, seq, command, arguments) @@ -763,7 +816,7 @@ def __init__(self, channel, seq, command, arguments): def describe(self): return f"{self.seq} request {json.repr(self.command)} to {self.channel}" - def wait_for_response(self, raise_if_failed=True): + def wait_for_response(self, raise_if_failed=True) -> MessageDict | Exception: """Waits until a response is received for this request, records the Response object for it in self.response, and returns response.body. @@ -778,8 +831,11 @@ def wait_for_response(self, raise_if_failed=True): while self.response is None: self.channel._handlers_enqueued.wait() - if raise_if_failed and not self.response.success: + if raise_if_failed and not self.response.success and isinstance( self.response.body, BaseException): raise self.response.body + + # When raise_if_failed is False, a failed response intentionally returns its + # error body (an Exception such as NoMoreMessages), so this must not assert. return self.response.body def on_response(self, response_handler): @@ -865,13 +921,13 @@ class Response(Message): the appropriate exception type that applies_to() the Response object. """ - def __init__(self, channel, seq, request, body, json=None): + def __init__(self, channel, seq, request, body: MessageDict | Exception, json=None): super().__init__(channel, seq, json) self.request = request """The request to which this is the response.""" - if isinstance(body, MessageDict) and hasattr(body, "associate_with"): + if is_associable(body): body.associate_with(self) self.body = body """Body of the response if the request was successful, or an instance @@ -905,8 +961,10 @@ def result(self): """ if self.success: return self.body - else: + elif isinstance(self.body, Exception): raise self.body + else: + raise Exception(self.body) @staticmethod def _parse(channel, message_dict, body=None): @@ -1264,7 +1322,10 @@ def send_event(self, event, body=None): def request(self, *args, **kwargs): """Same as send_request(...).wait_for_response()""" - return self.send_request(*args, **kwargs).wait_for_response() + # This should always raise an exception on failure + result = self.send_request(*args, **kwargs).wait_for_response() + assert not isinstance(result, BaseException) + return result def propagate(self, message): """Sends a new message with the same type and payload. @@ -1283,7 +1344,7 @@ def delegate(self, message): """ try: result = self.propagate(message) - if result.is_request(): + if result is not None and result.is_request(): result = result.wait_for_response() return result except MessageHandlingError as exc: @@ -1337,10 +1398,12 @@ def _parse_incoming_message(self): # for all JSON objects, and track them so that they can be later wired up to # the Message they belong to, once it is instantiated. def object_hook(d): - d = MessageDict(None, d) + d = AssociableMessageDict(None, d) if "seq" in d: self._prettify(d) - d.associate_with = associate_with + # Share the list of all dicts parsed for this message so that + # associate_with() on the top-level dict wires up every nested dict. + d.associated_dicts = message_dicts message_dicts.append(d) return d @@ -1349,22 +1412,18 @@ def object_hook(d): # cannot be done until the actual Message is created - which happens after the # dicts are created during deserialization. # - # So, upon deserialization, every dict in the message payload gets a method - # that can be called to set MessageDict.message for *all* dicts belonging to - # that message. This method can then be invoked on the top-level dict by the - # parser, after it has parsed enough of the dict to create the appropriate - # instance of Event, Request, or Response for this message. - def associate_with(message): - for d in message_dicts: - d.message = message - del d.associate_with - - message_dicts = [] + # So, upon deserialization, every dict in the message payload shares the + # message_dicts list below. AssociableMessageDict.associate_with() can then be + # invoked on the top-level dict by the parser, after it has parsed enough of the + # dict to create the appropriate instance of Event, Request, or Response for this + # message, and it will set MessageDict.message for *all* dicts belonging to that + # message. + message_dicts: "list[AssociableMessageDict]" = [] decoder = self.stream.json_decoder_factory(object_hook=object_hook) message_dict = self.stream.read_json(decoder) assert isinstance(message_dict, MessageDict) # make sure stream used decoder - msg_type = message_dict("type", json.enum("event", "request", "response")) + msg_type: str = cast(str, message_dict("type", json.enum("event", "request", "response"))) parser = self._message_parsers[msg_type] try: parser(self, message_dict) @@ -1422,7 +1481,7 @@ def _run_handlers(self): while True: with self: closed = self._closed - if closed: + if closed and self._parser_thread is not None: # Wait for the parser thread to wrap up and enqueue any remaining # handlers, if it is still running. self._parser_thread.join() diff --git a/src/debugpy/common/singleton.py b/src/debugpy/common/singleton.py index d515a4ab..89aa8fc6 100644 --- a/src/debugpy/common/singleton.py +++ b/src/debugpy/common/singleton.py @@ -86,12 +86,16 @@ def __init__(self, *args, **kwargs): def __enter__(self): """Lock this singleton to prevent concurrent access.""" - type(self)._lock.acquire() + lock = type(self)._lock + assert lock is not None + lock.acquire() return self def __exit__(self, exc_type, exc_value, exc_tb): """Unlock this singleton to allow concurrent access.""" - type(self)._lock.release() + lock = type(self)._lock + assert lock is not None + lock.release() def share(self): """Share this singleton, if it was originally created with shared=False.""" @@ -137,15 +141,18 @@ def __init__(self, *args, **kwargs): # with @threadsafe_method. Such methods should perform the necessary locking to # ensure thread safety for the callers. - @staticmethod def assert_locked(self): lock = type(self)._lock - assert lock.acquire(blocking=False), ( + assert lock is not None + # Side-effect-free check: verify the current thread already owns the lock, + # rather than acquire()/release() (which mutates the RLock recursion count + # and, under `python -O` with the assert stripped, would leak a release()). + # _is_owned() is a private but stable RLock helper (used by threading itself). + assert lock._is_owned(), ( # pyright: ignore[reportAttributeAccessIssue] "ThreadSafeSingleton accessed without locking. Either use with-statement, " "or if it is a method or property, mark it as @threadsafe_method or with " "@autolocked_method, as appropriate." ) - lock.release() def __getattribute__(self, name): value = object.__getattribute__(self, name) diff --git a/src/debugpy/common/sockets.py b/src/debugpy/common/sockets.py index 47db4d89..aecb6d83 100644 --- a/src/debugpy/common/sockets.py +++ b/src/debugpy/common/sockets.py @@ -5,6 +5,7 @@ import socket import sys import threading +from typing import Any, Callable, Union from debugpy.common import log from debugpy.common.util import hide_thread_from_debugger @@ -68,7 +69,7 @@ def create_server(host, port=0, backlog=socket.SOMAXCONN, timeout=None): if port is None: port = 0 ipv6 = host.count(":") > 1 - + server: Union[socket.socket, None] = None try: server = _new_sock(ipv6) if port != 0: @@ -87,7 +88,8 @@ def create_server(host, port=0, backlog=socket.SOMAXCONN, timeout=None): server.settimeout(timeout) server.listen(backlog) except Exception: # pragma: no cover - server.close() + if server is not None: + server.close() raise return server @@ -138,7 +140,7 @@ def close_socket(sock): sock.close() -def serve(name, handler, host, port=0, backlog=socket.SOMAXCONN, timeout=None): +def serve(name: str, handler: Callable[[socket.socket], Any], host: str, port: int=0, backlog=socket.SOMAXCONN, timeout: Union[int, None]=None): """Accepts TCP connections on the specified host and port, and invokes the provided handler function for every new connection. @@ -148,7 +150,7 @@ def serve(name, handler, host, port=0, backlog=socket.SOMAXCONN, timeout=None): assert backlog > 0 try: - listener = create_server(host, port, backlog, timeout) + listener: socket.socket = create_server(host, port, backlog, timeout) except Exception: # pragma: no cover log.reraise_exception( "Error listening for incoming {0} connections on {1}:{2}:", name, host, port diff --git a/src/debugpy/common/util.py b/src/debugpy/common/util.py index 54850a07..efc6b663 100644 --- a/src/debugpy/common/util.py +++ b/src/debugpy/common/util.py @@ -5,6 +5,7 @@ import inspect import os import sys +from typing import Any, Callable, Sequence def evaluate(code, path=__file__, mode="eval"): @@ -19,7 +20,10 @@ def evaluate(code, path=__file__, mode="eval"): class Observable(object): """An object with change notifications.""" - observers = () # used when attributes are set before __init__ is invoked + # Immutable default, used when attributes are set before __init__ is invoked. + # Kept as a tuple (rather than a list) so it can't be mutated and accidentally + # shared across instances. + observers: Sequence[Callable[..., Any]] = () def __init__(self): self.observers = [] @@ -162,3 +166,4 @@ def hide_thread_from_debugger(thread): if hide_debugpy_internals(): thread.pydev_do_not_trace = True thread.is_pydev_daemon_thread = True + diff --git a/src/debugpy/launcher/debuggee.py b/src/debugpy/launcher/debuggee.py index ec0c9eff..0b70e1ea 100644 --- a/src/debugpy/launcher/debuggee.py +++ b/src/debugpy/launcher/debuggee.py @@ -10,6 +10,7 @@ import subprocess import sys import threading +from typing import Any from debugpy import launcher from debugpy.common import log, messaging @@ -34,7 +35,7 @@ def describe(): - return f"Debuggee[PID={process.pid}]" + return f"Debuggee[PID={process.pid if process is not None else 0}]" def spawn(process_name, cmdline, env, redirect_output): @@ -47,6 +48,8 @@ def spawn(process_name, cmdline, env, redirect_output): ) close_fds = set() + stdout_r = 0 + stderr_r = 0 try: if redirect_output: # subprocess.PIPE behavior can vary substantially depending on Python version @@ -54,7 +57,7 @@ def spawn(process_name, cmdline, env, redirect_output): stdout_r, stdout_w = os.pipe() stderr_r, stderr_w = os.pipe() close_fds |= {stdout_r, stdout_w, stderr_r, stderr_w} - kwargs = dict(stdout=stdout_w, stderr=stderr_w) + kwargs: dict[str, Any] = dict(stdout=stdout_w, stderr=stderr_w) else: kwargs = {} @@ -195,6 +198,10 @@ def kill(): def wait_for_exit(): try: + # wait_for_exit() only runs after the debuggee was spawned successfully, so + # process is always set here; assert to fail fast rather than reporting a + # bogus clean exit (code 0) if that invariant is ever broken. + assert process is not None code = process.wait() if sys.platform != "win32" and code < 0: # On POSIX, if the process was terminated by a signal, Popen will use @@ -242,7 +249,7 @@ def _wait_for_user_input(): log.debug("msvcrt available - waiting for user input via getch()") sys.stdout.write("Press any key to continue . . . ") sys.stdout.flush() - msvcrt.getch() + msvcrt.getch() # pyright: ignore[reportPossiblyUnboundVariable, reportAttributeAccessIssue] else: log.debug("msvcrt not available - waiting for user input via read()") sys.stdout.write("Press Enter to continue . . . ") diff --git a/src/debugpy/launcher/output.py b/src/debugpy/launcher/output.py index 70cd5218..3f26d211 100644 --- a/src/debugpy/launcher/output.py +++ b/src/debugpy/launcher/output.py @@ -18,7 +18,7 @@ class CaptureOutput(object): instances = {} """Keys are output categories, values are CaptureOutput instances.""" - def __init__(self, whose, category, fd, stream): + def __init__(self, whose, category, fd: int, stream): assert category not in self.instances self.instances[category] = self log.info("Capturing {0} of {1}.", category, whose) @@ -98,8 +98,9 @@ def _process_chunk(self, s, final=False): if written == 0: # This means that the output stream was closed from the other end. # Do the same to the debuggee, so that it knows as well. - os.close(self._fd) - self._fd = None + if self._fd is not None: + os.close(self._fd) + self._fd = None break i += written except Exception: diff --git a/src/debugpy/launcher/winapi.py b/src/debugpy/launcher/winapi.py index a93dbc70..4b470f5f 100644 --- a/src/debugpy/launcher/winapi.py +++ b/src/debugpy/launcher/winapi.py @@ -64,14 +64,14 @@ def _errcheck(is_error_result=(lambda result: not result)): def impl(result, func, args): if is_error_result(result): log.debug("{0} returned {1}", func.__name__, result) - raise ctypes.WinError() + raise ctypes.WinError() # pyright: ignore[reportAttributeAccessIssue] else: return result return impl -kernel32 = ctypes.windll.kernel32 +kernel32 = ctypes.windll.kernel32 # pyright: ignore[reportAttributeAccessIssue] kernel32.AssignProcessToJobObject.errcheck = _errcheck() kernel32.AssignProcessToJobObject.restype = BOOL diff --git a/src/debugpy/server/api.py b/src/debugpy/server/api.py index a1de5874..eb50a4a2 100644 --- a/src/debugpy/server/api.py +++ b/src/debugpy/server/api.py @@ -4,6 +4,7 @@ import codecs import os +from typing import Any import pydevd import socket import sys @@ -42,28 +43,25 @@ def _settrace(*args, **kwargs): log.debug("pydevd.settrace(*{0!r}, **{1!r})", args, kwargs) # The stdin in notification is not acted upon in debugpy, so, disable it. kwargs.setdefault("notify_stdin", False) - try: - pydevd.settrace(*args, **kwargs) - except Exception: - raise + return pydevd.settrace(*args, **kwargs) def ensure_logging(): """Starts logging to log.log_dir, if it hasn't already been done.""" - if ensure_logging.ensured: + if ensure_logging.ensured: # pyright: ignore[reportFunctionMemberAccess] return - ensure_logging.ensured = True + ensure_logging.ensured = True # pyright: ignore[reportFunctionMemberAccess] log.to_file(prefix="debugpy.server") log.describe_environment("Initial environment:") if log.log_dir is not None: pydevd.log_to(log.log_dir + "/debugpy.pydevd.log") -ensure_logging.ensured = False +ensure_logging.ensured = False # pyright: ignore[reportFunctionMemberAccess] def log_to(path): - if ensure_logging.ensured: + if getattr(ensure_logging, "ensured"): raise RuntimeError("logging has already begun") log.debug("log_to{0!r}", (path,)) @@ -238,7 +236,10 @@ def listen(address, settrace_kwargs, in_process_debug_adapter=False): sock.settimeout(None) sock_io = sock.makefile("rb", 0) try: - endpoints = json.loads(sock_io.read().decode("utf-8")) + data = sock_io.read() + if not data: + raise EOFError("EOF while reading adapter endpoints") + endpoints = json.loads(data.decode("utf-8")) finally: sock_io.close() finally: @@ -299,7 +300,7 @@ def connect(address, settrace_kwargs, access_token=None, parent_session_pid=None _settrace(host=host, port=port, client_access_token=access_token, ppid=parent_session_pid or 0, **settrace_kwargs) -class wait_for_client: +class wait_for_client_cls: def __call__(self): ensure_logging() log.debug("wait_for_client()") @@ -313,12 +314,10 @@ def __call__(self): pydevd._wait_for_attach(cancel=cancel_event) @staticmethod - def cancel(): + def cancel() -> None: raise RuntimeError("wait_for_client() must be called first") - -wait_for_client = wait_for_client() - +wait_for_client = wait_for_client_cls() def is_client_connected(): return pydevd._is_attached() @@ -336,6 +335,7 @@ def breakpoint(): stop_at_frame = sys._getframe().f_back while ( stop_at_frame is not None + and pydb is not None and pydb.get_file_type(stop_at_frame) == pydb.PYDEV_FILE ): stop_at_frame = stop_at_frame.f_back @@ -360,7 +360,7 @@ def trace_this_thread(should_trace): ensure_logging() log.debug("trace_this_thread({0!r})", should_trace) - pydb = get_global_debugger() + pydb: Any = get_global_debugger() if should_trace: pydb.enable_tracing() else: diff --git a/src/debugpy/server/attach_pid_injected.py b/src/debugpy/server/attach_pid_injected.py index a8df6e1e..866d9916 100644 --- a/src/debugpy/server/attach_pid_injected.py +++ b/src/debugpy/server/attach_pid_injected.py @@ -11,7 +11,7 @@ _debugpy_dir = os.path.dirname(os.path.dirname(os.path.dirname(__file__))) -def attach(setup): +def attach(setup) -> None: log = None try: import sys diff --git a/tests/debugpy/adapter/test_clients.py b/tests/debugpy/adapter/test_clients.py new file mode 100644 index 00000000..df1d6e13 --- /dev/null +++ b/tests/debugpy/adapter/test_clients.py @@ -0,0 +1,114 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See LICENSE in the project root +# for license information. + +"""Unit tests for debugpy.adapter.clients message handlers.""" + +import pytest + +from debugpy.common import json, messaging +from debugpy.adapter import clients + + +class _MemoryStream(object): + """Minimal in-memory JSON stream that records everything written to it.""" + + json_encoder_factory = messaging.JsonIOStream.json_encoder_factory + + def __init__(self): + self.name = "memory" + self.output = [] + + def close(self): + pass + + def write_json(self, value, encoder=None): + encoder = encoder if encoder is not None else self.json_encoder_factory() + self.output.append(json.loads(encoder.encode(value))) + + +class _FakeSession(object): + """Stands in for the reentrant session lock used by the message_handler wrapper.""" + + def __init__(self, server=None): + self.server = server + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return False + + +class _UnpropagatingChannel(object): + def propagate(self, request): + return None + + +class _FakeServer(object): + channel = _UnpropagatingChannel() + + +def _make_client(start_request, has_started): + stream = _MemoryStream() + channel = messaging.JsonMessageChannel(stream, None) + + client = clients.Client.__new__(clients.Client) + client.session = _FakeSession() + client.start_request = start_request + client.has_started = has_started + + request = messaging.Request(channel, 1, "configurationDone", {}) + return client, request, stream + + +@pytest.mark.parametrize( + "start_request, has_started, scenario", + [ + (None, False, "before a start request"), + (object(), True, "after startup has already begun"), + ], +) +def test_configuration_done_out_of_order_is_rejected(start_request, has_started, scenario): + client, request, stream = _make_client(start_request, has_started) + + # The guard must fail the request loudly rather than silently falling through + # and delegating to the server (the previously ineffective guard did the latter). + with pytest.raises(messaging.MessageHandlingError): + clients.Client.configurationDone_request(client, request) + + (response,) = stream.output + assert response["type"] == "response" + assert response["command"] == "configurationDone" + assert response["success"] is False + assert response["message"] == ( + '"configurationDone" is only allowed during handling of a "launch" ' + 'or an "attach" request' + ) + # The guard must run before any startup side effects. + assert client.has_started is has_started + + +def test_evaluate_request_that_cannot_be_propagated_is_rejected(): + stream = _MemoryStream() + channel = messaging.JsonMessageChannel(stream, None) + + client = clients.Client.__new__(clients.Client) + client.session = _FakeSession(_FakeServer()) + request = messaging.Request(channel, 1, "evaluate", {}) + + with pytest.raises( + messaging.MessageHandlingError, + match='"evaluate" could not be propagated to the debug server', + ): + clients.Client.evaluate_request(client, request) + + (response,) = stream.output + assert response == { + "seq": 1, + "type": "response", + "request_seq": 1, + "success": False, + "command": "evaluate", + "message": '"evaluate" could not be propagated to the debug server', + } diff --git a/tests/debugpy/common/test_json.py b/tests/debugpy/common/test_json.py new file mode 100644 index 00000000..7993fbc0 --- /dev/null +++ b/tests/debugpy/common/test_json.py @@ -0,0 +1,47 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See LICENSE in the project root +# for license information. + +"""Unit tests for debugpy.common.json numeric conversion.""" + +from decimal import Decimal +from fractions import Fraction + +import pytest + +from debugpy.common import json + + +class TestConverter: + """Pins the numeric types that json._converter accepts. + + _converter deliberately narrows from numbers.Number to int and float, because + DAP number payloads are only ever int or float. These tests guard that contract + against accidental future widening (e.g. re-adding Decimal/complex/Fraction) or + narrowing (e.g. dropping float). + """ + + def test_converts_int(self): + assert json._converter("42", (int,)) == 42 + + def test_converts_float(self): + assert json._converter("3.5", (float,)) == 3.5 + + def test_first_matching_type_wins(self): + # int is listed first, so "10" is converted with int, not float. + result = json._converter("10", (int, float)) + assert result == 10 + assert type(result) is int + + def test_returns_none_for_invalid_value(self): + assert json._converter("not-a-number", (int,)) is None + + @pytest.mark.parametrize("classinfo_type", [Decimal, complex, Fraction]) + def test_unsupported_numeric_types_not_converted(self, classinfo_type): + # Decimal, complex, and Fraction are numbers.Number subclasses that are + # intentionally NOT accepted; _converter returns None instead of converting. + assert json._converter("1", (classinfo_type,)) is None + + def test_unsupported_type_ignored_when_mixed_with_supported(self): + # Only the supported int entry drives conversion; the Decimal entry is skipped. + assert json._converter("7", (Decimal, int)) == 7 diff --git a/tests/debugpy/common/test_messaging.py b/tests/debugpy/common/test_messaging.py index 335a5ead..c6054c47 100644 --- a/tests/debugpy/common/test_messaging.py +++ b/tests/debugpy/common/test_messaging.py @@ -312,6 +312,48 @@ def pause_request(self, request): }, ] + def test_respond_none_produces_empty_body(self): + # A successful response with no body must expose Response.body as an empty + # MessageDict (never None), matching how incoming empty responses are parsed. + # This pins the contract so callers can rely on body always being a + # MessageDict-or-Exception and never need a `body is None` special case. + REQUESTS = [ + { + "seq": 1, + "type": "request", + "command": "configurationDone", + "arguments": {}, + }, + ] + + captured = [] + + class Handlers(object): + def configurationDone_request(self, request): + request.respond(None) + captured.append(request.response) + + stream = JsonMemoryStream(REQUESTS, []) + channel = messaging.JsonMessageChannel(stream, Handlers()) + channel.start() + channel.wait() + + (response,) = captured + assert response.body is not None + assert response.body == {} + assert response.success + + # "body" is omitted from the serialized JSON for an empty response. + assert stream.output == [ + { + "seq": 1, + "type": "response", + "request_seq": 1, + "command": "configurationDone", + "success": True, + }, + ] + def test_responses(self): request1_sent = threading.Event() request2_sent = threading.Event() @@ -437,6 +479,63 @@ def response4_handler(resp): assert response4 is request4.response assert isinstance(response4.body, messaging.NoMoreMessages) + def test_wait_for_response_raise_if_failed(self): + request_sent = threading.Event() + + def iter_responses(): + request_sent.wait() + yield { + "seq": 1, + "type": "response", + "request_seq": 1, + "command": "pause", + "success": False, + "message": "pause not supported", + } + + stream = JsonMemoryStream(iter_responses(), []) + channel = messaging.JsonMessageChannel(stream, None) + channel.start() + + request = channel.send_request("pause") + request_sent.set() + + # raise_if_failed=False must return the error body instead of raising, even + # though a failed response carries an Exception as its body. + body = request.wait_for_response(raise_if_failed=False) + assert isinstance(body, messaging.MessageHandlingError) + assert body is request.response.body + assert str(body) == "pause not supported" + + # raise_if_failed=True (the default) must raise that same error body. + with pytest.raises(messaging.MessageHandlingError): + request.wait_for_response() + + def test_message_call_no_args_returns_payload(self): + EVENTS = [ + { + "seq": 1, + "type": "event", + "event": "stopped", + "body": {"reason": "pause", "threadId": 3}, + }, + ] + + captured = [] + + class Handlers(object): + def stopped_event(self, event): + # Calling the message with no arguments returns the whole payload. + captured.append(event()) + + stream = JsonMemoryStream(EVENTS, []) + channel = messaging.JsonMessageChannel(stream, Handlers()) + channel.start() + channel.wait() + + (payload,) = captured + assert payload == {"reason": "pause", "threadId": 3} + def test_invalid_request_handling(self): REQUESTS = [ { diff --git a/tests/debugpy/common/test_singleton.py b/tests/debugpy/common/test_singleton.py new file mode 100644 index 00000000..791272fb --- /dev/null +++ b/tests/debugpy/common/test_singleton.py @@ -0,0 +1,42 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See LICENSE in the project root +# for license information. + +"""Unit tests for debugpy.common.singleton.ThreadSafeSingleton locking.""" + +import pytest + +from debugpy.common import singleton + + +class _Widget(singleton.ThreadSafeSingleton): + """A trivial ThreadSafeSingleton subclass used to exercise assert_locked.""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.value = 0 + + +@pytest.fixture +def widget(): + # Use shared=False so each test gets a fresh, isolated instance without + # colliding with any process-wide shared singleton of the same type. + return _Widget(shared=False) + + +def test_assert_locked_passes_when_owned(widget): + # Inside the with-statement, the current thread owns the lock, so + # attribute access (which routes through assert_locked) must succeed. + with widget: + widget.value = 42 + assert widget.value == 42 + + +def test_assert_locked_fails_when_unlocked(widget): + # Outside any with-statement no thread owns the lock, so accessing a + # non-threadsafe attribute must fail fast rather than silently proceed. + with pytest.raises(AssertionError): + widget.value + + with pytest.raises(AssertionError): + widget.value = 1 diff --git a/tests/debugpy/server/test_api.py b/tests/debugpy/server/test_api.py new file mode 100644 index 00000000..f8bd7b58 --- /dev/null +++ b/tests/debugpy/server/test_api.py @@ -0,0 +1,84 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See LICENSE in the project root +# for license information. + +"""Unit tests for debugpy.server.api behaviors that don't require a live session.""" + +import subprocess + +import pytest + +from debugpy.server import api + + +class _FakeSockIO: + def __init__(self, data): + self._data = data + + def read(self): + return self._data + + def close(self): + pass + + +class _FakeSocket: + def __init__(self, data): + self._data = data + + def settimeout(self, timeout): + pass + + def makefile(self, *args, **kwargs): + return _FakeSockIO(self._data) + + +class _FakeEndpointsListener: + def __init__(self, data): + self._data = data + + def accept(self): + return _FakeSocket(self._data), None + + def close(self): + pass + + +class _FakeAdapterProcess: + pid = 4321 + returncode = None + + def wait(self): + pass + + +def _stub_listen_environment(monkeypatch, adapter_data): + """Drive `listen()` up to reading the adapter endpoints without real I/O.""" + + # `listen()` is single-shot; reset the latch so the test can invoke it. + monkeypatch.setattr(api.listen, "called", False, raising=False) + monkeypatch.setattr(api, "ensure_logging", lambda: None) + + listener = _FakeEndpointsListener(adapter_data) + monkeypatch.setattr(api.sockets, "create_server", lambda *a, **k: listener) + monkeypatch.setattr(api.sockets, "get_address", lambda _l: ("127.0.0.1", 12345)) + monkeypatch.setattr(api.sockets, "close_socket", lambda _s: None) + + monkeypatch.setattr(subprocess, "Popen", lambda *a, **k: _FakeAdapterProcess()) + monkeypatch.setattr(api.pydevd, "add_dont_terminate_child_pid", lambda _pid: None) + + +def test_listen_empty_adapter_read_raises(monkeypatch): + # An empty read from the adapter endpoints socket is EOF: `sock_io.read()` + # returns b"" (not None) at EOF, so the `if not data:` guard must fire and + # surface an EOFError rather than falling through to json.loads (which would + # raise a confusing JSONDecodeError on empty input). + _stub_listen_environment(monkeypatch, b"") + + with pytest.raises(RuntimeError) as exc_info: + api.listen(("127.0.0.1", 0)) + + assert ( + str(exc_info.value) + == "error retrieving adapter endpoints: EOF while reading adapter endpoints" + ) diff --git a/tests/debugpy/test_output.py b/tests/debugpy/test_output.py index ad7d9644..8c22efb3 100644 --- a/tests/debugpy/test_output.py +++ b/tests/debugpy/test_output.py @@ -2,10 +2,12 @@ # Licensed under the MIT License. See LICENSE in the project root # for license information. +import codecs import pytest import sys from _pydevd_bundle.pydevd_constants import IS_PY312_OR_GREATER +from debugpy.launcher import output from tests import debug from tests.debug import runners @@ -15,6 +17,36 @@ # sequentially, by the time we get to "stopped", we also have all the output events. +def test_zero_byte_write_stops_after_fd_closed(): + class ZeroThenFailStream: + def __init__(self): + self.write_count = 0 + + def write(self, _data): + self.write_count += 1 + if self.write_count == 1: + return 0 + raise AssertionError("write retried after a zero-byte write") + + def flush(self): + pass + + stream = ZeroThenFailStream() + capture = output.CaptureOutput.__new__(output.CaptureOutput) + capture.category = "stdout" + capture._fd = None + capture._decoder = codecs.getincrementaldecoder("utf-8")(errors="surrogateescape") + capture._stream = stream + capture._encode = codecs.getencoder("utf-8") + + # Leave an incomplete character buffered, then finalize after the descriptor + # has already been cleared. + capture._process_chunk(b"\xc3") + capture._process_chunk(b"", final=True) + + assert stream.write_count == 1 + + @pytest.mark.parametrize("run", runners.all) def test_with_no_output(pyfile, target, run): @pyfile diff --git a/tests/pytest_hooks.py b/tests/pytest_hooks.py index 7ef3851f..dba34995 100644 --- a/tests/pytest_hooks.py +++ b/tests/pytest_hooks.py @@ -11,6 +11,13 @@ import tests from tests import logs +try: + # Private pytest API: stash key used by the built-in tmp_path fixture to record + # per-phase outcomes and decide retention during its finalizer. + from _pytest.tmpdir import tmppath_result_key +except ImportError: + tmppath_result_key = None + def pytest_addoption(parser): parser.addoption( @@ -39,6 +46,19 @@ def pytest_report_header(config): return log.get_environment_description(f"Test environment for tests-{os.getpid()}") +@pytest.hookimpl(tryfirst=True) +def pytest_runtest_setup(item): + # Workaround for pytest-retry's incompatibility with pytest's tmp_path fixture + # (pytest >= 8.4 / 9.x). When pytest-retry re-runs a flaky test, it invokes the + # setup/call phases directly without firing pytest_runtest_makereport, so the + # tmppath_result_key stash entry - deleted by the previous attempt's tmp_path + # finalizer - is never repopulated, and the next teardown raises KeyError while + # reading it. Re-seed the key here (which pytest-retry *does* run on each retry) + # so the finalizer always finds it. setdefault avoids clobbering a live entry. + if tmppath_result_key is not None: + item.stash.setdefault(tmppath_result_key, {}) + + @pytest.hookimpl(hookwrapper=True, tryfirst=True) def pytest_runtest_makereport(item, call): # Adds attributes setup_report, call_report, and teardown_report to the item, diff --git a/tests/requirements.txt b/tests/requirements.txt index 195e94fb..a681579e 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -26,4 +26,4 @@ typing_extensions # Used to build pydevd attach to process binaries: vswhere -Cython \ No newline at end of file +Cython