Address review feedback: fix runtime bugs and add tests

- messaging.Message.__call__: fix dead no-arg fast path (args.count -> len(args))
- messaging.OutgoingRequest.wait_for_response: honor raise_if_failed=False by
  returning the error body instead of asserting; return type now MessageDict|Exception
- messaging: complete AssociableMessageDict migration; associate_with is now a real
  method backed by a shared associated_dicts list instead of a setattr closure
- server/api._settrace: only latch called on success so a failed settrace no
  longer permanently blocks configure(); drop no-op except/finally
- adapter/servers.authenticate: fail closed when authorize result is an Exception
- common/util.Observable.observers: revert class default to immutable tuple to avoid
  shared-mutable footgun; typed and callers reassign instead of in-place +=
- common/json._converter: document the int/float narrowing from numbers.Number
- add unit tests for wait_for_response(raise_if_failed=False), Message.__call__()
  no-arg, and the configure() already-running guard incl. the settrace-failure case

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: rchiodo

src/debugpy/adapter/components.py

@@ -61,7 +61,7 @@ def __init__(self, session: "Session", stream: "Union[messaging.JsonIOStream, No
         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}]"

src/debugpy/adapter/servers.py

@@ -174,7 +174,12 @@ def authenticate(self):
         auth = self.channel.request(
             "pydevdAuthorize", {"debugServerAccessToken": access_token}
         )
-        if not isinstance(auth, Exception) and auth["clientAccessToken"] != adapter.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.')
 

src/debugpy/adapter/sessions.py

@@ -63,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}]"

src/debugpy/common/json.py

@@ -95,6 +95,9 @@ def __format__(self, format_spec):
 
 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, int) or issubclass(one_info, float):
             try:

src/debugpy/common/messaging.py

@@ -429,11 +429,22 @@ def wrap(self, key, *args, **kwargs):
 
 
 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):
-        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, MessageDict) and hasattr(obj, "associate_with")
+    return isinstance(obj, AssociableMessageDict)
 
 def _payload(value):
     """JSON validator for message payload.
@@ -495,7 +506,7 @@ def payload(self) -> MessageDict | Exception:
     def __call__(self, *args, **kwargs) -> MessageDict | Any | int | float:
         """Same as self.payload(...)."""
         assert not isinstance(self.payload, Exception)
-        if args.count == 0 and kwargs == {}:
+        if len(args) == 0 and not kwargs:
             return self.payload
         return self.payload(*args, **kwargs)
 
@@ -793,7 +804,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)-> MessageDict:
+    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.
 
@@ -810,8 +821,9 @@ def wait_for_response(self, raise_if_failed=True)-> MessageDict:
 
         if raise_if_failed and not self.response.success and isinstance( self.response.body, BaseException):
             raise self.response.body
-        
-        assert not isinstance(self.response.body, Exception)
+
+        # 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):
@@ -1377,7 +1389,9 @@ def object_hook(d):
             d = AssociableMessageDict(None, d)
             if "seq" in d:
                 self._prettify(d)
-            setattr(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
 
@@ -1386,17 +1400,13 @@ 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

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 = []

src/debugpy/server/api.py

@@ -44,12 +44,11 @@ def __new__(cls, *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:
-            return pydevd.settrace(*args, **kwargs)
-        except Exception:
-            raise
-        finally:
-            cls.called = True
+        result = pydevd.settrace(*args, **kwargs)
+        # Only latch `called` after settrace has actually succeeded, so that a failed
+        # attempt doesn't permanently block future configure() calls.
+        cls.called = True
+        return result
 
 
 def ensure_logging():

tests/debugpy/common/test_messaging.py

@@ -437,6 +437,59 @@ def response4_handler(resp):
         assert response4 is request4.response
         assert isinstance(response4.body, messaging.NoMoreMessages)
 
+    def test_wait_for_response_raise_if_failed(self):
+        def iter_responses():
+            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")
+
+        # 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 "pause not supported" in str(body)
+
+        # 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 = [
             {

tests/debugpy/server/test_api.py

@@ -0,0 +1,50 @@
+# 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 pytest
+
+from debugpy.server import api
+
+
+@pytest.fixture
+def reset_settrace_called():
+    """Isolate the global _settrace.called latch used by configure()."""
+    original = api._settrace.called
+    api._settrace.called = False
+    try:
+        yield
+    finally:
+        api._settrace.called = original
+
+
+def test_settrace_failure_does_not_block_configure(monkeypatch, reset_settrace_called):
+    # ensure_logging() writes log files; stub it out for this unit test.
+    monkeypatch.setattr(api, "ensure_logging", lambda: None)
+
+    def failing_settrace(*args, **kwargs):
+        raise RuntimeError("settrace failed")
+
+    monkeypatch.setattr(api.pydevd, "settrace", failing_settrace)
+
+    with pytest.raises(RuntimeError, match="settrace failed"):
+        api._settrace()
+
+    # A failed settrace must not latch `called`, ...
+    assert api._settrace.called is False
+
+    # ... so configure() must not reject the call as "already running".
+    api.configure()
+
+
+def test_settrace_success_blocks_configure(monkeypatch, reset_settrace_called):
+    monkeypatch.setattr(api, "ensure_logging", lambda: None)
+    monkeypatch.setattr(api.pydevd, "settrace", lambda *args, **kwargs: None)
+
+    api._settrace()
+    assert api._settrace.called is True
+
+    with pytest.raises(RuntimeError, match="already running"):
+        api.configure()