Skip to content

Commit f4a98c4

Browse files
pdepetrometa-codesync[bot]
authored andcommitted
Use static "Snapshot" label in DAP stopped event description
Summary: The dynamic `Snapshot {N}/{M} — {HH:MM:SS.fff} ({ExceptionType})` description that `_snapshot_display_name` builds for the `stopped` event looks correct on the initial launch but stays stale on subsequent `tintypeJumpToSnapshot` requests. VS Code does not consistently re-render the CALL STACK description on every `stopped` event after the first one, so the label drifts out of sync with the actual cursor — and a stale `1/3` label next to snapshot 2 is more misleading than no detail at all. The bootstrap-then-real pair in `handle_launch` is also affected: in practice the bootstrap event's plain `"Snapshot"` is what users see on launch, hiding the dynamic format entirely. Collapse `_snapshot_display_name` to always return `"Snapshot"`. The snapshot index and timestamp the user actually needs are already visible in the Tintype Snapshots panel — the CALL STACK description doesn't need to duplicate them. The bootstrap-then-real `_send_stopped_event` pair in `handle_launch` is kept intact so any future re-introduction of a dynamic label (e.g. once we have a reliable way to push description updates to VS Code) can use it without code changes. Drops the now-unused `import datetime`. Reviewed By: bvu405 Differential Revision: D106092777 fbshipit-source-id: d0e3cb034883423dde01b50a3f1d001f730ea7d9
1 parent d2838e9 commit f4a98c4

2 files changed

Lines changed: 42 additions & 104 deletions

File tree

dap/session.py

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
from __future__ import annotations
1616

17-
import datetime
1817
import logging
1918
import os
2019
import re
@@ -1223,58 +1222,23 @@ def _require_snapshot(self) -> Snapshot:
12231222
# Display-name helpers
12241223
# ---------------------------------------------------------------
12251224

1226-
def _snapshot_display_name(self, snapshot: Snapshot) -> str | None:
1225+
def _snapshot_display_name(self, _snapshot: Snapshot) -> str | None:
12271226
"""One-line description of the current snapshot.
12281227
12291228
Surfaced via the ``stopped`` event's ``description`` field, which
1230-
VS Code renders next to the session row in CALL STACK. Keep it
1231-
short — the field is a single-line label and VS Code truncates.
1232-
1233-
Default format::
1234-
1235-
Snapshot {N}/{M} — {HH:MM:SS.fff} ({ExceptionType})
1236-
1237-
Returns ``None`` to suppress the description entirely.
1229+
VS Code renders next to the session row in CALL STACK.
1230+
1231+
Currently always returns the static label ``"Snapshot"`` rather
1232+
than a dynamic ``N/M — timestamp`` format. The dynamic format
1233+
looks correct on the initial launch but stays stale on
1234+
subsequent ``tintypeJumpToSnapshot`` requests because VS Code
1235+
does not consistently re-render the CALL STACK description on
1236+
every ``stopped`` event after the first one. A stale dynamic
1237+
label is more misleading than a static one — the snapshot
1238+
index / timestamp the user actually needs is already visible
1239+
in the snapshots panel.
12381240
"""
1239-
parts: list[str] = []
1240-
try:
1241-
total = self._require_reader().snapshot_count()
1242-
except DispatchError:
1243-
total = 1
1244-
if total > 1:
1245-
parts.append(f"Snapshot {self._snapshot_index + 1}/{total}")
1246-
else:
1247-
parts.append("Snapshot")
1248-
1249-
try:
1250-
# ``snapshot.timestamp`` is microseconds since the Unix epoch.
1251-
# Format using UTC so cross-timezone teams opening the same
1252-
# ``.pytb`` see identical labels in CALL STACK (the label
1253-
# otherwise depends on the viewer's local wall clock, which
1254-
# is confusing when correlating snapshots with server-side
1255-
# timestamps).
1256-
ts_seconds = int(snapshot.timestamp) / 1_000_000.0
1257-
dt = datetime.datetime.fromtimestamp(ts_seconds, tz=datetime.timezone.utc)
1258-
parts.append("— " + dt.strftime("%H:%M:%S.%f")[:-3] + " UTC")
1259-
except (ValueError, OSError, OverflowError, TypeError) as e:
1260-
# Concrete failure modes of ``int()`` (TypeError / ValueError),
1261-
# ``fromtimestamp()`` (OverflowError / OSError on Windows for
1262-
# out-of-range timestamps), and attribute access on a
1263-
# malformed Snapshot (AttributeError is intentionally NOT
1264-
# caught — it signals a real bug, not a bad timestamp). Log
1265-
# at warning so bug reports can surface the real cause; the
1266-
# description field just loses the timestamp decoration.
1267-
logger.warning(
1268-
"failed to format timestamp for snapshot display name: %s", e
1269-
)
1270-
1271-
for st in snapshot.stacktraces.values():
1272-
exc = st.exception_object
1273-
if exc is not None:
1274-
parts.append(f"({type(exc).__name__})")
1275-
break
1276-
1277-
return " ".join(parts) if parts else None
1241+
return "Snapshot"
12781242

12791243

12801244
# ---------------------------------------------------------------

dap/tests/test_session.py

Lines changed: 29 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -167,28 +167,23 @@ def test_launch_emits_initial_events(self) -> None:
167167

168168
messages = _drain_messages(self.stream)
169169
# We expect: launch response, initialized, thread(s), two
170-
# stopped events (bootstrap + real). The bootstrap carries a
171-
# generic ``"Snapshot"`` description so VS Code has something to
172-
# overwrite on its initial-launch race; the real event that
173-
# follows sets the actual display name the user sees in the
174-
# CALL STACK panel. We still emit NO ``process`` event — that
175-
# surface is owned by the Tintype Snapshots sidebar.
170+
# stopped events (bootstrap + real). Both currently carry the
171+
# static ``"Snapshot"`` description; the bootstrap-then-real
172+
# pattern is retained because VS Code overwrites the
173+
# description on the first event of a launch, and any future
174+
# reintroduction of a dynamic display name needs the second
175+
# event to settle the real value. We still emit NO ``process``
176+
# event — that surface is owned by the Tintype Snapshots
177+
# sidebar.
176178
event_names = [m.get("event") for m in messages if m["type"] == "event"]
177179
self.assertIn("initialized", event_names)
178180
self.assertNotIn("process", event_names)
179181
self.assertIn("thread", event_names)
180182
self.assertEqual(event_names.count("stopped"), 2)
181183

182184
stopped_events = [m for m in messages if m.get("event") == "stopped"]
183-
# First is bootstrap — generic "Snapshot" label.
184185
self.assertEqual(stopped_events[0]["body"]["description"], "Snapshot")
185-
# Second is the real one — must include a richer description.
186-
self.assertIn("description", stopped_events[1]["body"])
187-
self.assertIn("Snapshot", stopped_events[1]["body"]["description"])
188-
self.assertNotEqual(
189-
stopped_events[0]["body"]["description"],
190-
stopped_events[1]["body"]["description"],
191-
)
186+
self.assertEqual(stopped_events[1]["body"]["description"], "Snapshot")
192187

193188
def test_launch_succeeds_on_empty_snapshot_file(self) -> None:
194189
"""An empty ``.pytb`` is a valid launch state.
@@ -1774,9 +1769,13 @@ def test_exception_info_on_virtual_chain_thread_returns_innermost_cause(
17741769

17751770
class StoppedEventDescriptionTest(unittest.TestCase):
17761771
"""``stopped`` events carry a ``description`` field with the snapshot's
1777-
display name. On launch we send a bootstrap ``"Snapshot"`` event first so
1778-
VS Code has something to overwrite on its initial-launch race, followed
1779-
by the real description event the user actually sees."""
1772+
display name. The label is currently the static string ``"Snapshot"`` —
1773+
a dynamic ``N/M — timestamp`` form looked correct on launch but went
1774+
stale on subsequent ``tintypeJumpToSnapshot`` calls because VS Code
1775+
does not consistently re-render CALL STACK descriptions on every
1776+
``stopped`` event after the first. A stale dynamic label is more
1777+
misleading than a static one — the snapshot index / timestamp the
1778+
user needs is already visible in the snapshots panel."""
17801779

17811780
def setUp(self) -> None:
17821781
self.stream = RecordingStream()
@@ -1805,38 +1804,9 @@ def _launch_with_snapshots(self, snapshots: list[Any]) -> MagicMock:
18051804
)
18061805
return reader
18071806

1808-
def test_launch_description_includes_timestamp_and_index(self) -> None:
1809-
st = _make_stacktrace(
1810-
100, [_make_frame("/a/b.py", "foo", 1, {})], thread_name="MainThread"
1811-
)
1812-
snap = _make_snapshot([st], ts=1_700_000_000_000_000)
1813-
self._launch_with_snapshots([snap])
1814-
1815-
stopped_events = [
1816-
m for m in _drain_messages(self.stream) if m.get("event") == "stopped"
1817-
]
1818-
real = stopped_events[-1]["body"]
1819-
self.assertIn("description", real)
1820-
self.assertIn("Snapshot", real["description"])
1821-
# HH:MM:SS.fff pattern must show up in the real-stop description.
1822-
self.assertRegex(real["description"], r"\d{2}:\d{2}:\d{2}\.\d{3}")
1823-
1824-
def test_description_mentions_exception_type(self) -> None:
1825-
st = _make_stacktrace(
1826-
100,
1827-
[_make_frame("/a/b.py", "foo", 1, {})],
1828-
thread_name="MainThread",
1829-
exception=KeyError("missing"),
1830-
)
1831-
snap = _make_snapshot([st])
1832-
self._launch_with_snapshots([snap])
1833-
1834-
real = [m for m in _drain_messages(self.stream) if m.get("event") == "stopped"][
1835-
-1
1836-
]["body"]
1837-
self.assertIn("KeyError", real["description"])
1838-
1839-
def test_description_updates_across_snapshot_jumps(self) -> None:
1807+
def test_description_is_static_snapshot_label(self) -> None:
1808+
"""Every ``stopped`` event — launch, jump, and exception — uses
1809+
the static ``"Snapshot"`` description."""
18401810
snap1 = _make_snapshot(
18411811
[
18421812
_make_stacktrace(
@@ -1853,16 +1823,18 @@ def test_description_updates_across_snapshot_jumps(self) -> None:
18531823
100,
18541824
[_make_frame("/a/b.py", "foo", 2, {})],
18551825
thread_name="MainThread",
1826+
exception=KeyError("missing"),
18561827
)
18571828
],
18581829
ts=1_700_000_005_000_000,
18591830
)
18601831
self._launch_with_snapshots([snap1, snap2])
18611832

1862-
launch_desc = [
1833+
launch_stops = [
18631834
m for m in _drain_messages(self.stream) if m.get("event") == "stopped"
1864-
][-1]["body"]["description"]
1865-
self.assertIn("1/2", launch_desc)
1835+
]
1836+
for evt in launch_stops:
1837+
self.assertEqual(evt["body"]["description"], "Snapshot")
18661838

18671839
self.stream.seek(0)
18681840
self.stream.truncate()
@@ -1878,9 +1850,11 @@ def test_description_updates_across_snapshot_jumps(self) -> None:
18781850
]
18791851
# Jump emits exactly one stopped event (no bootstrap).
18801852
self.assertEqual(len(jump_events), 1)
1881-
jump_desc = jump_events[0]["body"]["description"]
1882-
self.assertIn("2/2", jump_desc)
1883-
self.assertNotEqual(launch_desc, jump_desc)
1853+
# Even though snap2 carries an exception, the static label
1854+
# does not mention the exception type. CALL STACK shows the
1855+
# exception via the thread name + stop reason; the description
1856+
# is intentionally minimal.
1857+
self.assertEqual(jump_events[0]["body"]["description"], "Snapshot")
18841858

18851859

18861860
if __name__ == "__main__":

0 commit comments

Comments
 (0)