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(MarkdownFence): gracefully handle missing Static child in _retheme callback #5525

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gonzaloruizdevilla
Copy link

@gonzaloruizdevilla gonzaloruizdevilla commented Feb 13, 2025

This PR addresses a race condition in the MarkdownFence widget where the _retheme callback can be invoked before the widget's child (Static) is mounted. In such cases, calling self.get_child_by_type(Static).update(self._block()) would raise a NoMatches exception, potentially causing unwanted crashes.

Crash stacktrace:

╭───────────────────────────────────────────────── Traceback (most recent call last) ──────────────────────────────────────────────────╮
│ /Users/xxxx/Documents/xxxxx/xxxxxxx/xxxxxxxxxx/venv/lib/python3.13/site-packages/textual/widgets/_markdown.py:638 in _on_mount      │
│                                                                                                                                      │
│    635 │                                                                                        ╭──────── locals ────────╮           │
│    636 │   def _on_mount(self, _: Mount) -> None:                                               │    _ = Mount()         │           │
│    637 │   │   """Watch app theme switching."""                                                 │ self = MarkdownFence() │           │
│ ❱  638 │   │   self.watch(self.app, "theme", self._retheme)                                     ╰────────────────────────╯           │
│    639 │                                                                                                                             │
│    640 │   def _retheme(self) -> None:                                                                                               │
│    641 │   │   """Rerender when the theme changes."""                                                                                │
│                                                                                                                                      │
│ /Users/xxxx/Documents/xxxxx/xxxxxxx/xxxxxxxxxx/venv/lib/python3.13/site-packages/textual/dom.py:1244 in watch                       │
│                                                                                                                                      │
│   1241 │   │   │   callback: A callback to run when attribute changes.                                                               │
│   1242 │   │   │   init: Check watchers on first call.                                                                               │
│   1243 │   │   """                                                                                                                   │
│ ❱ 1244 │   │   _watch(self, obj, attribute_name, callback, init=init)                                                                │
│   1245 │                                                                                                                             │
│   1246 │   def get_pseudo_classes(self) -> set[str]:                                                                                 │
│   1247 │   │   """Pseudo classes for a widget.                                                                                       │
│                                                                                                                                      │
│ ╭──────────────────────────────────────────── locals ─────────────────────────────────────────────╮                                  │
│ │ attribute_name = 'theme'                                                                        │                                  │
│ │       callback = <bound method MarkdownFence._retheme of MarkdownFence()>                       │                                  │
│ │           init = True                                                                           │                                  │
│ │            obj = Compi(title='Compi', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'}) │                                  │
│ │           self = MarkdownFence()                                                                │                                  │
│ ╰─────────────────────────────────────────────────────────────────────────────────────────────────╯                                  │
│                                                                                                                                      │
│ /Users/xxxx/Documents/xxxxx/xxxxxxx/xxxxxxxxxx/venv/lib/python3.13/site-packages/textual/reactive.py:486 in _watch                  │
│                                                                                                                                      │
│   483 │   │   return                                                                                                                 │
│   484 │   if init:                                                                                                                   │
│   485 │   │   current_value = getattr(obj, attribute_name, None)                                                                     │
│ ❱ 486 │   │   invoke_watcher(obj, callback, current_value, current_value)                                                            │
│   487 │   watcher_list.append((node, callback))                                                                                      │
│   488                                                                                                                                │
│                                                                                                                                      │
│ ╭───────────────────────────────────────────────────── locals ──────────────────────────────────────────────────────╮                │
│ │ attribute_name = 'theme'                                                                                          │                │
│ │       callback = <bound method MarkdownFence._retheme of MarkdownFence()>                                         │                │
│ │  current_value = 'textual-dark'                                                                                   │                │
│ │           init = True                                                                                             │                │
│ │           node = MarkdownFence()                                                                                  │                │
│ │            obj = Compi(title='Compi', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'})                   │                │
│ │   watcher_list = [                                                                                                │                │
│ │                  │   (                                                                                            │                │
│ │                  │   │   ExtendedTextArea(id='chat_input'),                                                       │                │
│ │                  │   │   <bound method TextArea._app_theme_changed of ExtendedTextArea(id='chat_input')>          │                │
│ │                  │   ),                                                                                           │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),                 │                │
│ │                  │   ... +1089                                                                                    │                │
│ │                  ]                                                                                                │                │
│ │       watchers = {                                                                                                │                │
│ │                  │   'theme': [                                                                                   │                │
│ │                  │   │   (                                                                                        │                │
│ │                  │   │   │   ExtendedTextArea(id='chat_input'),                                                   │                │
│ │                  │   │   │   <bound method TextArea._app_theme_changed of ExtendedTextArea(id='chat_input')>      │                │
│ │                  │   │   ),                                                                                       │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   (MarkdownFence(), <bound method MarkdownFence._retheme of MarkdownFence()>),             │                │
│ │                  │   │   ... +1089                                                                                │                │
│ │                  │   ],                                                                                           │                │
│ │                  │   'title': [(Header(), <function Header._on_mount.<locals>.set_title at 0x16ece84a0>)],        │                │
│ │                  │   'sub_title': [                                                                               │                │
│ │                  │   │   (Header(), <function Header._on_mount.<locals>.set_sub_title at 0x16ecebd80>)            │                │
│ │                  │   ]                                                                                            │                │
│ │                  }                                                                                                │                │
│ ╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯                │
│                                                                                                                                      │
│ /Users/xxxx/Documents/xxxxx/xxxxxxx/xxxxxxxxxx/venv/lib/python3.13/site-packages/textual/widgets/_markdown.py:647 in _retheme       │
│                                                                                                                                      │
│    644 │   │   │   if self.app.current_theme.dark                                               ╭──────── locals ────────╮           │
│    645 │   │   │   else self._markdown.code_light_theme                                         │ self = MarkdownFence() │           │
│    646 │   │   )                                                                                ╰────────────────────────╯           │
│ ❱  647 │   │   self.get_child_by_type(Static).update(self._block())                                                                  │
│    648 │                                                                                                                             │
│    649 │   def compose(self) -> ComposeResult:                                                                                       │
│    650 │   │   yield Static(                                                                                                         │
│                                                                                                                                      │
│ /Users/xxxx/Documents/xxxxx/xxxxxxx/xxxxxxxxxx/venv/lib/python3.13/site-packages/textual/widget.py:1003 in get_child_by_type        │
│                                                                                                                                      │
│   1000 │   │   │   if type(child) is expect_type:                                                                                    │
│   1001 │   │   │   │   assert isinstance(child, expect_type)                                                                         │
│   1002 │   │   │   │   return child                                                                                                  │
│ ❱ 1003 │   │   raise NoMatches(f"No immediate child of type {expect_type}; {self._nodes}")                                           │
│   1004 │                                                                                                                             │
│   1005 │   def get_component_rich_style(self, *names: str, partial: bool = False) -> Style:                                          │
│   1006 │   │   """Get a *Rich* style for a component.                                                                                │
│                                                                                                                                      │
│ ╭──────────────────────── locals ────────────────────────╮                                                                           │
│ │ expect_type = <class 'textual.widgets._static.Static'> │                                                                           │
│ │        self = MarkdownFence()                          │                                                                           │
│ ╰────────────────────────────────────────────────────────╯                                                                           │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
NoMatches: No immediate child of type <class 'textual.widgets._static.Static'>; <NodeList []>

…e callback

Wrap update call in try/except to prevent exceptions when the Static child isn't yet mounted.
@willmcgugan
Copy link
Collaborator

Can you please add an MRE or explanation of how to reproduce this.

I will also need a test.

@gonzaloruizdevilla
Copy link
Author

I've rarely seen this error when streaming the output of a llm to a Markdown component. That is why I pasted the log, I'm not able to reproduce it consistently. If I find a way to reproduce it in a test I will update this PR.

I'm doing basically
self.app.call_from_thread(self.mount_response, response_widget)
and then
self.app.call_from_thread(response_widget.update, partial_text)

being response_widget a Markdown component.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants