x11: Improve error handling (#32913)

Michael Sloan created

Continuing this work from a while back in #21079, now greatly aided by
agent + sonnet 4. With this change, there are now only a few spots that
explicitly panic, though errors during initialization will panic.

Motivation was this recent user panic in `handle_event`, figured fixing
all this use of unwrap was a great use of the agent.

> called `Result::unwrap()` on an `Err` value: X11 GetProperty for
_NET_WM_STATE failed.

Release Notes:

- N/A

Change summary

crates/gpui/src/platform.rs                  |   9 
crates/gpui/src/platform/linux/x11/client.rs | 570 ++++++++++++---------
crates/gpui/src/platform/linux/x11/window.rs | 130 +++--
3 files changed, 406 insertions(+), 303 deletions(-)

Detailed changes

crates/gpui/src/platform.rs 🔗

@@ -93,6 +93,9 @@ pub(crate) fn current_platform(headless: bool) -> Rc<dyn Platform> {
 
 #[cfg(any(target_os = "linux", target_os = "freebsd"))]
 pub(crate) fn current_platform(headless: bool) -> Rc<dyn Platform> {
+    #[cfg(feature = "x11")]
+    use anyhow::Context as _;
+
     if headless {
         return Rc::new(HeadlessClient::new());
     }
@@ -102,7 +105,11 @@ pub(crate) fn current_platform(headless: bool) -> Rc<dyn Platform> {
         "Wayland" => Rc::new(WaylandClient::new()),
 
         #[cfg(feature = "x11")]
-        "X11" => Rc::new(X11Client::new()),
+        "X11" => Rc::new(
+            X11Client::new()
+                .context("Failed to initialize X11 client.")
+                .unwrap(),
+        ),
 
         "Headless" => Rc::new(HeadlessClient::new()),
         _ => unreachable!(),

crates/gpui/src/platform/linux/x11/client.rs 🔗

@@ -17,6 +17,7 @@ use calloop::{
 use collections::HashMap;
 use futures::channel::oneshot;
 use http_client::Url;
+use log::Level;
 use smallvec::SmallVec;
 use util::ResultExt;
 
@@ -41,12 +42,12 @@ use xkbc::x11::ffi::{XKB_X11_MIN_MAJOR_XKB_VERSION, XKB_X11_MIN_MINOR_XKB_VERSIO
 use xkbcommon::xkb::{self as xkbc, LayoutIndex, ModMask, STATE_LAYOUT_EFFECTIVE};
 
 use super::{
-    ButtonOrScroll, ScrollDirection, button_or_scroll_from_event_detail,
+    ButtonOrScroll, ScrollDirection, X11Display, X11WindowStatePtr, XcbAtoms, XimCallbackEvent,
+    XimHandler, button_or_scroll_from_event_detail, check_reply,
     clipboard::{self, Clipboard},
-    get_valuator_axis_index, modifiers_from_state, pressed_button_from_mask,
+    get_reply, get_valuator_axis_index, handle_connection_error, modifiers_from_state,
+    pressed_button_from_mask,
 };
-use super::{X11Display, X11WindowStatePtr, XcbAtoms};
-use super::{XimCallbackEvent, XimHandler};
 
 use crate::platform::{
     LinuxCommon, PlatformWindow,
@@ -230,12 +231,14 @@ pub struct X11ClientState {
 pub struct X11ClientStatePtr(pub Weak<RefCell<X11ClientState>>);
 
 impl X11ClientStatePtr {
-    fn get_client(&self) -> X11Client {
-        X11Client(self.0.upgrade().expect("client already dropped"))
+    fn get_client(&self) -> Option<X11Client> {
+        self.0.upgrade().map(X11Client)
     }
 
     pub fn drop_window(&self, x_window: u32) {
-        let client = self.get_client();
+        let Some(client) = self.get_client() else {
+            return;
+        };
         let mut state = client.0.borrow_mut();
 
         if let Some(window_ref) = state.windows.remove(&x_window) {
@@ -262,14 +265,23 @@ impl X11ClientStatePtr {
     }
 
     pub fn update_ime_position(&self, bounds: Bounds<ScaledPixels>) {
-        let client = self.get_client();
+        let Some(client) = self.get_client() else {
+            return;
+        };
         let mut state = client.0.borrow_mut();
         if state.composing || state.ximc.is_none() {
             return;
         }
 
-        let mut ximc = state.ximc.take().unwrap();
-        let xim_handler = state.xim_handler.take().unwrap();
+        let Some(mut ximc) = state.ximc.take() else {
+            log::error!("bug: xim connection not set");
+            return;
+        };
+        let Some(xim_handler) = state.xim_handler.take() else {
+            log::error!("bug: xim handler not set");
+            state.ximc = Some(ximc);
+            return;
+        };
         let ic_attributes = ximc
             .build_ic_attributes()
             .push(
@@ -300,8 +312,8 @@ impl X11ClientStatePtr {
 pub(crate) struct X11Client(Rc<RefCell<X11ClientState>>);
 
 impl X11Client {
-    pub(crate) fn new() -> Self {
-        let event_loop = EventLoop::try_new().unwrap();
+    pub(crate) fn new() -> anyhow::Result<Self> {
+        let event_loop = EventLoop::try_new()?;
 
         let (common, main_receiver) = LinuxCommon::new(event_loop.get_signal());
 
@@ -321,39 +333,34 @@ impl X11Client {
                     }
                 }
             })
-            .unwrap();
-
-        let (xcb_connection, x_root_index) = XCBConnection::connect(None).unwrap();
-        xcb_connection
-            .prefetch_extension_information(xkb::X11_EXTENSION_NAME)
-            .unwrap();
-        xcb_connection
-            .prefetch_extension_information(randr::X11_EXTENSION_NAME)
-            .unwrap();
-        xcb_connection
-            .prefetch_extension_information(render::X11_EXTENSION_NAME)
-            .unwrap();
-        xcb_connection
-            .prefetch_extension_information(xinput::X11_EXTENSION_NAME)
-            .unwrap();
+            .map_err(|err| {
+                anyhow!("Failed to initialize event loop handling of foreground tasks: {err:?}")
+            })?;
+
+        let (xcb_connection, x_root_index) = XCBConnection::connect(None)?;
+        xcb_connection.prefetch_extension_information(xkb::X11_EXTENSION_NAME)?;
+        xcb_connection.prefetch_extension_information(randr::X11_EXTENSION_NAME)?;
+        xcb_connection.prefetch_extension_information(render::X11_EXTENSION_NAME)?;
+        xcb_connection.prefetch_extension_information(xinput::X11_EXTENSION_NAME)?;
 
         // Announce to X server that XInput up to 2.1 is supported. To increase this to 2.2 and
         // beyond, support for touch events would need to be added.
-        let xinput_version = xcb_connection
-            .xinput_xi_query_version(2, 1)
-            .unwrap()
-            .reply()
-            .unwrap();
-        // XInput 1.x is not supported.
+        let xinput_version = get_reply(
+            || "XInput XiQueryVersion failed",
+            xcb_connection.xinput_xi_query_version(2, 1),
+        )?;
         assert!(
             xinput_version.major_version >= 2,
             "XInput version >= 2 required."
         );
 
         let pointer_device_states =
-            get_new_pointer_device_states(&xcb_connection, &BTreeMap::new());
+            current_pointer_device_states(&xcb_connection, &BTreeMap::new()).unwrap_or_default();
 
-        let atoms = XcbAtoms::new(&xcb_connection).unwrap().reply().unwrap();
+        let atoms = XcbAtoms::new(&xcb_connection)
+            .context("Failed to get XCB atoms")?
+            .reply()
+            .context("Failed to get XCB atoms")?;
 
         let root = xcb_connection.setup().roots[0].root;
         let compositor_present = check_compositor_present(&xcb_connection, root);
@@ -366,11 +373,11 @@ impl X11Client {
             gtk_frame_extents_supported
         );
 
-        let xkb = xcb_connection
-            .xkb_use_extension(XKB_X11_MIN_MAJOR_XKB_VERSION, XKB_X11_MIN_MINOR_XKB_VERSION)
-            .unwrap()
-            .reply()
-            .unwrap();
+        let xkb = get_reply(
+            || "Failed to initialize XKB extension",
+            xcb_connection
+                .xkb_use_extension(XKB_X11_MIN_MAJOR_XKB_VERSION, XKB_X11_MIN_MINOR_XKB_VERSION),
+        )?;
 
         let events = xkb::EventType::STATE_NOTIFY
             | xkb::EventType::MAP_NOTIFY
@@ -383,16 +390,17 @@ impl X11Client {
             | xkb::MapPart::KEY_BEHAVIORS
             | xkb::MapPart::VIRTUAL_MODS
             | xkb::MapPart::VIRTUAL_MOD_MAP;
-        xcb_connection
-            .xkb_select_events(
+        check_reply(
+            || "Failed to select XKB events",
+            xcb_connection.xkb_select_events(
                 xkb::ID::USE_CORE_KBD.into(),
                 0u8.into(),
                 events,
                 map_notify_parts,
                 map_notify_parts,
                 &xkb::SelectEventsAux::new(),
-            )
-            .unwrap();
+            ),
+        )?;
         assert!(xkb.supported);
 
         let xkb_context = xkbc::Context::new(xkbc::CONTEXT_NO_FLAGS);
@@ -414,9 +422,10 @@ impl X11Client {
             .to_string();
         let keyboard_layout = LinuxKeyboardLayout::new(layout_name.into());
 
-        let gpu_context = BladeContext::new().expect("Unable to init GPU context");
+        let gpu_context = BladeContext::new().context("Unable to init GPU context")?;
 
-        let resource_database = x11rb::resource_manager::new_from_default(&xcb_connection).unwrap();
+        let resource_database = x11rb::resource_manager::new_from_default(&xcb_connection)
+            .context("Failed to create resource database")?;
         let scale_factor = resource_database
             .get_value("Xft.dpi", "Xft.dpi")
             .ok()
@@ -424,11 +433,11 @@ impl X11Client {
             .map(|dpi: f32| dpi / 96.0)
             .unwrap_or(1.0);
         let cursor_handle = cursor::Handle::new(&xcb_connection, x_root_index, &resource_database)
-            .unwrap()
+            .context("Failed to initialize cursor theme handler")?
             .reply()
-            .unwrap();
+            .context("Failed to initialize cursor theme handler")?;
 
-        let clipboard = Clipboard::new().unwrap();
+        let clipboard = Clipboard::new().context("Failed to initialize clipboard")?;
 
         let xcb_connection = Rc::new(xcb_connection);
 
@@ -457,7 +466,7 @@ impl X11Client {
                     }
                 },
             )
-            .expect("Failed to initialize x11 event source");
+            .map_err(|err| anyhow!("Failed to initialize X11 event source: {err:?}"))?;
 
         handle
             .insert_source(XDPEventSource::new(&common.background_executor), {
@@ -473,9 +482,9 @@ impl X11Client {
                     }
                 }
             })
-            .unwrap();
+            .map_err(|err| anyhow!("Failed to initialize XDP event source: {err:?}"))?;
 
-        X11Client(Rc::new(RefCell::new(X11ClientState {
+        Ok(X11Client(Rc::new(RefCell::new(X11ClientState {
             modifiers: Modifiers::default(),
             capslock: Capslock::default(),
             last_modifiers_changed_event: Modifiers::default(),
@@ -520,7 +529,7 @@ impl X11Client {
             clipboard,
             clipboard_item: None,
             xdnd_state: Xdnd::default(),
-        })))
+        }))))
     }
 
     pub fn process_x11_events(
@@ -606,8 +615,9 @@ impl X11Client {
                     Ok(None) => {
                         break;
                     }
-                    Err(e) => {
-                        log::warn!("error polling for X11 events: {e:?}");
+                    Err(err) => {
+                        let err = handle_connection_error(err);
+                        log::warn!("error while polling for X11 events: {err:?}");
                         break;
                     }
                 }
@@ -633,14 +643,15 @@ impl X11Client {
 
             for event in events.into_iter() {
                 let mut state = self.0.borrow_mut();
-                if state.ximc.is_none() || state.xim_handler.is_none() {
+                if !state.has_xim() {
                     drop(state);
                     self.handle_event(event);
                     continue;
                 }
 
-                let mut ximc = state.ximc.take().unwrap();
-                let mut xim_handler = state.xim_handler.take().unwrap();
+                let Some((mut ximc, mut xim_handler)) = state.take_xim() else {
+                    continue;
+                };
                 let xim_connected = xim_handler.connected;
                 drop(state);
 
@@ -654,8 +665,7 @@ impl X11Client {
                 let xim_callback_event = xim_handler.last_callback_event.take();
 
                 let mut state = self.0.borrow_mut();
-                state.ximc = Some(ximc);
-                state.xim_handler = Some(xim_handler);
+                state.restore_xim(ximc, xim_handler);
                 drop(state);
 
                 if let Some(event) = xim_callback_event {
@@ -678,12 +688,13 @@ impl X11Client {
 
     pub fn enable_ime(&self) {
         let mut state = self.0.borrow_mut();
-        if state.ximc.is_none() {
+        if !state.has_xim() {
             return;
         }
 
-        let mut ximc = state.ximc.take().unwrap();
-        let mut xim_handler = state.xim_handler.take().unwrap();
+        let Some((mut ximc, mut xim_handler)) = state.take_xim() else {
+            return;
+        };
         let mut ic_attributes = ximc
             .build_ic_attributes()
             .push(AttributeName::InputStyle, InputStyle::PREEDIT_CALLBACKS)
@@ -693,7 +704,13 @@ impl X11Client {
         let window_id = state.keyboard_focused_window;
         drop(state);
         if let Some(window_id) = window_id {
-            let window = self.get_window(window_id).unwrap();
+            let Some(window) = self.get_window(window_id) else {
+                log::error!("Failed to get window for IME positioning");
+                let mut state = self.0.borrow_mut();
+                state.ximc = Some(ximc);
+                state.xim_handler = Some(xim_handler);
+                return;
+            };
             if let Some(area) = window.get_ime_area() {
                 ic_attributes =
                     ic_attributes.nested_list(xim::AttributeName::PreeditAttributes, |b| {
@@ -709,17 +726,19 @@ impl X11Client {
         }
         ximc.create_ic(xim_handler.im_id, ic_attributes.build())
             .ok();
-        state = self.0.borrow_mut();
-        state.xim_handler = Some(xim_handler);
-        state.ximc = Some(ximc);
+        let mut state = self.0.borrow_mut();
+        state.restore_xim(ximc, xim_handler);
     }
 
     pub fn reset_ime(&self) {
         let mut state = self.0.borrow_mut();
         state.composing = false;
         if let Some(mut ximc) = state.ximc.take() {
-            let xim_handler = state.xim_handler.as_ref().unwrap();
-            ximc.reset_ic(xim_handler.im_id, xim_handler.ic_id).ok();
+            if let Some(xim_handler) = state.xim_handler.as_ref() {
+                ximc.reset_ic(xim_handler.im_id, xim_handler.ic_id).ok();
+            } else {
+                log::error!("bug: xim handler not set in reset_ime");
+            }
             state.ximc = Some(ximc);
         }
     }
@@ -799,26 +818,25 @@ impl X11Client {
                     window.handle_input(PlatformInput::FileDrop(FileDropEvent::Exited {}));
                     self.0.borrow_mut().xdnd_state = Xdnd::default();
                 } else if event.type_ == state.atoms.XdndPosition {
-                    if let Ok(pos) = state
-                        .xcb_connection
-                        .query_pointer(event.window)
-                        .unwrap()
-                        .reply()
-                    {
+                    if let Ok(pos) = get_reply(
+                        || "Failed to query pointer position",
+                        state.xcb_connection.query_pointer(event.window),
+                    ) {
                         state.xdnd_state.position =
                             Point::new(Pixels(pos.win_x as f32), Pixels(pos.win_y as f32));
                     }
                     if !state.xdnd_state.retrieved {
-                        state
-                            .xcb_connection
-                            .convert_selection(
+                        check_reply(
+                            || "Failed to convert selection for drag and drop",
+                            state.xcb_connection.convert_selection(
                                 event.window,
                                 state.atoms.XdndSelection,
                                 state.xdnd_state.drag_type,
                                 state.atoms.XDND_DATA,
                                 arg3,
-                            )
-                            .unwrap();
+                            ),
+                        )
+                        .log_err();
                     }
                     xdnd_send_status(
                         &state.xcb_connection,
@@ -848,35 +866,37 @@ impl X11Client {
             Event::SelectionNotify(event) => {
                 let window = self.get_window(event.requestor)?;
                 let mut state = self.0.borrow_mut();
-                let property = state.xcb_connection.get_property(
-                    false,
-                    event.requestor,
-                    state.atoms.XDND_DATA,
-                    AtomEnum::ANY,
-                    0,
-                    1024,
-                );
-                if property.as_ref().log_err().is_none() {
+                let reply = get_reply(
+                    || "Failed to get XDND_DATA",
+                    state.xcb_connection.get_property(
+                        false,
+                        event.requestor,
+                        state.atoms.XDND_DATA,
+                        AtomEnum::ANY,
+                        0,
+                        1024,
+                    ),
+                )
+                .log_err();
+                let Some(reply) = reply else {
                     return Some(());
-                }
-                if let Ok(reply) = property.unwrap().reply() {
-                    match str::from_utf8(&reply.value) {
-                        Ok(file_list) => {
-                            let paths: SmallVec<[_; 2]> = file_list
-                                .lines()
-                                .filter_map(|path| Url::parse(path).log_err())
-                                .filter_map(|url| url.to_file_path().log_err())
-                                .collect();
-                            let input = PlatformInput::FileDrop(FileDropEvent::Entered {
-                                position: state.xdnd_state.position,
-                                paths: crate::ExternalPaths(paths),
-                            });
-                            drop(state);
-                            window.handle_input(input);
-                            self.0.borrow_mut().xdnd_state.retrieved = true;
-                        }
-                        Err(_) => {}
+                };
+                match str::from_utf8(&reply.value) {
+                    Ok(file_list) => {
+                        let paths: SmallVec<[_; 2]> = file_list
+                            .lines()
+                            .filter_map(|path| Url::parse(path).log_err())
+                            .filter_map(|url| url.to_file_path().log_err())
+                            .collect();
+                        let input = PlatformInput::FileDrop(FileDropEvent::Entered {
+                            position: state.xdnd_state.position,
+                            paths: crate::ExternalPaths(paths),
+                        });
+                        drop(state);
+                        window.handle_input(input);
+                        self.0.borrow_mut().xdnd_state.retrieved = true;
                     }
+                    Err(_) => {}
                 }
             }
             Event::ConfigureNotify(event) => {
@@ -891,11 +911,17 @@ impl X11Client {
                     },
                 };
                 let window = self.get_window(event.window)?;
-                window.configure(bounds).unwrap();
+                window
+                    .set_bounds(bounds)
+                    .context("X11: Failed to set window bounds")
+                    .log_err();
             }
             Event::PropertyNotify(event) => {
                 let window = self.get_window(event.window)?;
-                window.property_notify(event).unwrap();
+                window
+                    .property_notify(event)
+                    .context("X11: Failed to handle property notify")
+                    .log_err();
             }
             Event::FocusIn(event) => {
                 let window = self.get_window(event.event)?;
@@ -1262,10 +1288,12 @@ impl X11Client {
                         state.pointer_device_states.remove(&info.deviceid);
                     }
                 }
-                state.pointer_device_states = get_new_pointer_device_states(
+                if let Some(pointer_device_states) = current_pointer_device_states(
                     &state.xcb_connection,
                     &state.pointer_device_states,
-                );
+                ) {
+                    state.pointer_device_states = pointer_device_states;
+                }
             }
             Event::XinputDeviceChanged(event) => {
                 let mut state = self.0.borrow_mut();
@@ -1302,8 +1330,7 @@ impl X11Client {
                     state.modifiers,
                     event.detail.into(),
                 ));
-                let mut ximc = state.ximc.take().unwrap();
-                let mut xim_handler = state.xim_handler.take().unwrap();
+                let (mut ximc, mut xim_handler) = state.take_xim()?;
                 drop(state);
                 xim_handler.window = event.event;
                 ximc.forward_event(
@@ -1312,10 +1339,10 @@ impl X11Client {
                     xim::ForwardEventFlag::empty(),
                     &event,
                 )
-                .unwrap();
+                .context("X11: Failed to forward XIM event")
+                .log_err();
                 let mut state = self.0.borrow_mut();
-                state.ximc = Some(ximc);
-                state.xim_handler = Some(xim_handler);
+                state.restore_xim(ximc, xim_handler);
                 drop(state);
             }
             event => {
@@ -1326,7 +1353,10 @@ impl X11Client {
     }
 
     fn xim_handle_commit(&self, window: xproto::Window, text: String) -> Option<()> {
-        let window = self.get_window(window).unwrap();
+        let Some(window) = self.get_window(window) else {
+            log::error!("bug: Failed to get window for XIM commit");
+            return None;
+        };
         let mut state = self.0.borrow_mut();
         let keystroke = state.pre_key_char_down.take();
         state.composing = false;
@@ -1343,11 +1373,13 @@ impl X11Client {
     }
 
     fn xim_handle_preedit(&self, window: xproto::Window, text: String) -> Option<()> {
-        let window = self.get_window(window).unwrap();
+        let Some(window) = self.get_window(window) else {
+            log::error!("bug: Failed to get window for XIM preedit");
+            return None;
+        };
 
         let mut state = self.0.borrow_mut();
-        let mut ximc = state.ximc.take().unwrap();
-        let mut xim_handler = state.xim_handler.take().unwrap();
+        let (mut ximc, mut xim_handler) = state.take_xim()?;
         state.composing = !text.is_empty();
         drop(state);
         window.handle_ime_preedit(text);
@@ -1375,8 +1407,7 @@ impl X11Client {
                 .ok();
         }
         let mut state = self.0.borrow_mut();
-        state.ximc = Some(ximc);
-        state.xim_handler = Some(xim_handler);
+        state.restore_xim(ximc, xim_handler);
         drop(state);
         Some(())
     }
@@ -1429,15 +1460,13 @@ impl LinuxClient for X11Client {
 
     fn primary_display(&self) -> Option<Rc<dyn PlatformDisplay>> {
         let state = self.0.borrow();
-
-        Some(Rc::new(
-            X11Display::new(
-                &state.xcb_connection,
-                state.scale_factor,
-                state.x_root_index,
-            )
-            .expect("There should always be a root index"),
-        ))
+        X11Display::new(
+            &state.xcb_connection,
+            state.scale_factor,
+            state.x_root_index,
+        )
+        .log_err()
+        .map(|display| Rc::new(display) as Rc<dyn PlatformDisplay>)
     }
 
     fn display(&self, id: DisplayId) -> Option<Rc<dyn PlatformDisplay>> {
@@ -1464,7 +1493,10 @@ impl LinuxClient for X11Client {
         params: WindowParams,
     ) -> anyhow::Result<Box<dyn PlatformWindow>> {
         let mut state = self.0.borrow_mut();
-        let x_window = state.xcb_connection.generate_id().unwrap();
+        let x_window = state
+            .xcb_connection
+            .generate_id()
+            .context("X11: Failed to generate window ID")?;
 
         let window = X11Window::new(
             handle,
@@ -1480,16 +1512,17 @@ impl LinuxClient for X11Client {
             state.scale_factor,
             state.common.appearance,
         )?;
-        state
-            .xcb_connection
-            .change_property32(
+        check_reply(
+            || "Failed to set XdndAware property",
+            state.xcb_connection.change_property32(
                 xproto::PropMode::REPLACE,
                 x_window,
                 state.atoms.XdndAware,
                 state.atoms.XA_ATOM,
                 &[5],
-            )
-            .unwrap();
+            ),
+        )
+        .log_err();
 
         let window_ref = WindowRef {
             window: window.0.clone(),
@@ -1532,7 +1565,7 @@ impl LinuxClient for X11Client {
             )
             .anyhow()
             .and_then(|cookie| cookie.check().anyhow())
-            .context("setting cursor style")
+            .context("X11: Failed to set cursor style")
             .log_err();
     }
 
@@ -1555,7 +1588,7 @@ impl LinuxClient for X11Client {
                 clipboard::ClipboardKind::Primary,
                 clipboard::WaitConfig::None,
             )
-            .context("Failed to write to clipboard (primary)")
+            .context("X11 Failed to write to clipboard (primary)")
             .log_with_level(log::Level::Debug);
     }
 
@@ -1568,7 +1601,7 @@ impl LinuxClient for X11Client {
                 clipboard::ClipboardKind::Clipboard,
                 clipboard::WaitConfig::None,
             )
-            .context("Failed to write to clipboard (clipboard)")
+            .context("X11: Failed to write to clipboard (clipboard)")
             .log_with_level(log::Level::Debug);
         state.clipboard_item.replace(item);
     }
@@ -1578,7 +1611,7 @@ impl LinuxClient for X11Client {
         return state
             .clipboard
             .get_any(clipboard::ClipboardKind::Primary)
-            .context("Failed to read from clipboard (primary)")
+            .context("X11: Failed to read from clipboard (primary)")
             .log_with_level(log::Level::Debug);
     }
 
@@ -1595,17 +1628,21 @@ impl LinuxClient for X11Client {
         return state
             .clipboard
             .get_any(clipboard::ClipboardKind::Clipboard)
-            .context("Failed to read from clipboard (clipboard)")
+            .context("X11: Failed to read from clipboard (clipboard)")
             .log_with_level(log::Level::Debug);
     }
 
     fn run(&self) {
-        let mut event_loop = self
+        let Some(mut event_loop) = self
             .0
             .borrow_mut()
             .event_loop
             .take()
-            .expect("App is already running");
+            .context("X11Client::run called but it's already running")
+            .log_err()
+        else {
+            return;
+        };
 
         event_loop.run(None, &mut self.clone(), |_| {}).log_err();
     }
@@ -1641,7 +1678,7 @@ impl LinuxClient for X11Client {
         let window_ids = reply
             .value
             .chunks_exact(4)
-            .map(|chunk| u32::from_ne_bytes(chunk.try_into().unwrap()))
+            .filter_map(|chunk| chunk.try_into().ok().map(u32::from_ne_bytes))
             .collect::<Vec<xproto::Window>>();
 
         let mut handles = Vec::new();
@@ -1664,6 +1701,30 @@ impl LinuxClient for X11Client {
 }
 
 impl X11ClientState {
+    fn has_xim(&self) -> bool {
+        self.ximc.is_some() && self.xim_handler.is_some()
+    }
+
+    fn take_xim(&mut self) -> Option<(X11rbClient<Rc<XCBConnection>>, XimHandler)> {
+        let ximc = self
+            .ximc
+            .take()
+            .ok_or(anyhow!("bug: XIM connection not set"))
+            .log_err()?;
+        if let Some(xim_handler) = self.xim_handler.take() {
+            Some((ximc, xim_handler))
+        } else {
+            self.ximc = Some(ximc);
+            log::error!("bug: XIM handler not set");
+            None
+        }
+    }
+
+    fn restore_xim(&mut self, ximc: X11rbClient<Rc<XCBConnection>>, xim_handler: XimHandler) {
+        self.ximc = Some(ximc);
+        self.xim_handler = Some(xim_handler);
+    }
+
     fn update_refresh_loop(&mut self, x_window: xproto::Window) {
         let Some(window_ref) = self.windows.get_mut(&x_window) else {
             return;
@@ -1697,35 +1758,41 @@ impl X11ClientState {
                 });
             }
             (true, None) => {
-                let screen_resources = self
-                    .xcb_connection
-                    .randr_get_screen_resources_current(x_window)
-                    .unwrap()
-                    .reply()
-                    .expect("Could not find available screens");
+                let Some(screen_resources) = get_reply(
+                    || "Failed to get screen resources",
+                    self.xcb_connection
+                        .randr_get_screen_resources_current(x_window),
+                )
+                .log_err() else {
+                    return;
+                };
 
                 // Ideally this would be re-queried when the window changes screens, but there
                 // doesn't seem to be an efficient / straightforward way to do this. Should also be
                 // updated when screen configurations change.
-                let refresh_rate = mode_refresh_rate(
+                let mode_info = screen_resources.crtcs.iter().find_map(|crtc| {
+                    let crtc_info = self
+                        .xcb_connection
+                        .randr_get_crtc_info(*crtc, x11rb::CURRENT_TIME)
+                        .ok()?
+                        .reply()
+                        .ok()?;
+
                     screen_resources
-                        .crtcs
+                        .modes
                         .iter()
-                        .find_map(|crtc| {
-                            let crtc_info = self
-                                .xcb_connection
-                                .randr_get_crtc_info(*crtc, x11rb::CURRENT_TIME)
-                                .ok()?
-                                .reply()
-                                .ok()?;
-
-                            screen_resources
-                                .modes
-                                .iter()
-                                .find(|m| m.id == crtc_info.mode)
-                        })
-                        .expect("Unable to find screen refresh rate"),
-                );
+                        .find(|m| m.id == crtc_info.mode)
+                });
+                let refresh_rate = match mode_info {
+                    Some(mode_info) => mode_refresh_rate(mode_info),
+                    None => {
+                        log::error!(
+                            "Failed to get screen mode info from xrandr, \
+                            defaulting to 60hz refresh rate."
+                        );
+                        Duration::from_micros(1_000_000 / 60)
+                    }
+                };
 
                 let event_loop_token = self.start_refresh_loop(x_window, refresh_rate);
                 let Some(window_ref) = self.windows.get_mut(&x_window) else {
@@ -1772,7 +1839,7 @@ impl X11ClientState {
                     calloop::timer::TimeoutAction::ToInstant(instant)
                 }
             })
-            .expect("Failed to initialize refresh timer")
+            .expect("Failed to initialize window refresh timer")
     }
 
     fn get_cursor_icon(&mut self, style: CursorStyle) -> Option<xproto::Cursor> {
@@ -1784,7 +1851,7 @@ impl X11ClientState {
         match style {
             CursorStyle::None => match create_invisible_cursor(&self.xcb_connection) {
                 Ok(loaded_cursor) => result = Ok(loaded_cursor),
-                Err(err) => result = Err(err.context("error while creating invisible cursor")),
+                Err(err) => result = Err(err.context("X11: error while creating invisible cursor")),
             },
             _ => 'outer: {
                 let mut errors = String::new();
@@ -1827,14 +1894,14 @@ impl X11ClientState {
                 {
                     Ok(default) => {
                         log_cursor_icon_warning(err.context(format!(
-                            "x11: error loading cursor icon, falling back on default icon '{}'",
+                            "X11: error loading cursor icon, falling back on default icon '{}'",
                             DEFAULT_CURSOR_ICON_NAME
                         )));
                         Some(default)
                     }
                     Err(default_err) => {
                         log_cursor_icon_warning(err.context(default_err).context(format!(
-                            "x11: error loading default cursor fallback '{}'",
+                            "X11: error loading default cursor fallback '{}'",
                             DEFAULT_CURSOR_ICON_NAME
                         )));
                         None
@@ -1857,7 +1924,7 @@ pub fn mode_refresh_rate(mode: &randr::ModeInfo) -> Duration {
 
     let millihertz = mode.dot_clock as u64 * 1_000 / (mode.htotal as u64 * mode.vtotal as u64);
     let micros = 1_000_000_000 / millihertz;
-    log::info!("Refreshing at {} micros", micros);
+    log::info!("Refreshing every {}ms", micros / 1_000);
     Duration::from_micros(micros)
 }
 
@@ -1868,66 +1935,63 @@ fn fp3232_to_f32(value: xinput::Fp3232) -> f32 {
 fn check_compositor_present(xcb_connection: &XCBConnection, root: u32) -> bool {
     // Method 1: Check for _NET_WM_CM_S{root}
     let atom_name = format!("_NET_WM_CM_S{}", root);
-    let atom = xcb_connection
-        .intern_atom(false, atom_name.as_bytes())
-        .unwrap()
-        .reply()
-        .map(|reply| reply.atom)
-        .unwrap_or(0);
-
-    let method1 = if atom != 0 {
-        xcb_connection
-            .get_selection_owner(atom)
-            .unwrap()
-            .reply()
+    let atom1 = get_reply(
+        || format!("Failed to intern {atom_name}"),
+        xcb_connection.intern_atom(false, atom_name.as_bytes()),
+    );
+    let method1 = match atom1.log_with_level(Level::Debug) {
+        Some(reply) if reply.atom != x11rb::NONE => {
+            let atom = reply.atom;
+            get_reply(
+                || format!("Failed to get {atom_name} owner"),
+                xcb_connection.get_selection_owner(atom),
+            )
             .map(|reply| reply.owner != 0)
+            .log_with_level(Level::Debug)
             .unwrap_or(false)
-    } else {
-        false
+        }
+        _ => false,
     };
 
     // Method 2: Check for _NET_WM_CM_OWNER
     let atom_name = "_NET_WM_CM_OWNER";
-    let atom = xcb_connection
-        .intern_atom(false, atom_name.as_bytes())
-        .unwrap()
-        .reply()
-        .map(|reply| reply.atom)
-        .unwrap_or(0);
-
-    let method2 = if atom != 0 {
-        xcb_connection
-            .get_property(false, root, atom, xproto::AtomEnum::WINDOW, 0, 1)
-            .unwrap()
-            .reply()
+    let atom2 = get_reply(
+        || format!("Failed to intern {atom_name}"),
+        xcb_connection.intern_atom(false, atom_name.as_bytes()),
+    );
+    let method2 = match atom2.log_with_level(Level::Debug) {
+        Some(reply) if reply.atom != x11rb::NONE => {
+            let atom = reply.atom;
+            get_reply(
+                || format!("Failed to get {atom_name}"),
+                xcb_connection.get_property(false, root, atom, xproto::AtomEnum::WINDOW, 0, 1),
+            )
             .map(|reply| reply.value_len > 0)
             .unwrap_or(false)
-    } else {
-        false
+        }
+        _ => return false,
     };
 
     // Method 3: Check for _NET_SUPPORTING_WM_CHECK
     let atom_name = "_NET_SUPPORTING_WM_CHECK";
-    let atom = xcb_connection
-        .intern_atom(false, atom_name.as_bytes())
-        .unwrap()
-        .reply()
-        .map(|reply| reply.atom)
-        .unwrap_or(0);
-
-    let method3 = if atom != 0 {
-        xcb_connection
-            .get_property(false, root, atom, xproto::AtomEnum::WINDOW, 0, 1)
-            .unwrap()
-            .reply()
+    let atom3 = get_reply(
+        || format!("Failed to intern {atom_name}"),
+        xcb_connection.intern_atom(false, atom_name.as_bytes()),
+    );
+    let method3 = match atom3.log_with_level(Level::Debug) {
+        Some(reply) if reply.atom != x11rb::NONE => {
+            let atom = reply.atom;
+            get_reply(
+                || format!("Failed to get {atom_name}"),
+                xcb_connection.get_property(false, root, atom, xproto::AtomEnum::WINDOW, 0, 1),
+            )
             .map(|reply| reply.value_len > 0)
             .unwrap_or(false)
-    } else {
-        false
+        }
+        _ => return false,
     };
 
-    // TODO: Remove this
-    log::info!(
+    log::debug!(
         "Compositor detection: _NET_WM_CM_S?={}, _NET_WM_CM_OWNER={}, _NET_SUPPORTING_WM_CHECK={}",
         method1,
         method2,
@@ -1942,28 +2006,28 @@ fn check_gtk_frame_extents_supported(
     atoms: &XcbAtoms,
     root: xproto::Window,
 ) -> bool {
-    let supported_atoms = xcb_connection
-        .get_property(
+    let Some(supported_atoms) = get_reply(
+        || "Failed to get _NET_SUPPORTED",
+        xcb_connection.get_property(
             false,
             root,
             atoms._NET_SUPPORTED,
             xproto::AtomEnum::ATOM,
             0,
             1024,
-        )
-        .unwrap()
-        .reply()
-        .map(|reply| {
-            // Convert Vec<u8> to Vec<u32>
-            reply
-                .value
-                .chunks_exact(4)
-                .map(|chunk| u32::from_ne_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]))
-                .collect::<Vec<u32>>()
-        })
-        .unwrap_or_default();
+        ),
+    )
+    .log_with_level(Level::Debug) else {
+        return false;
+    };
+
+    let supported_atom_ids: Vec<u32> = supported_atoms
+        .value
+        .chunks_exact(4)
+        .filter_map(|chunk| chunk.try_into().ok().map(u32::from_ne_bytes))
+        .collect();
 
-    supported_atoms.contains(&atoms._GTK_FRAME_EXTENTS)
+    supported_atom_ids.contains(&atoms._GTK_FRAME_EXTENTS)
 }
 
 fn xdnd_is_atom_supported(atom: u32, atoms: &XcbAtoms) -> bool {
@@ -1980,17 +2044,19 @@ fn xdnd_get_supported_atom(
     supported_atoms: &XcbAtoms,
     target: xproto::Window,
 ) -> u32 {
-    let property = xcb_connection
-        .get_property(
+    if let Some(reply) = get_reply(
+        || "Failed to get XDnD supported atoms",
+        xcb_connection.get_property(
             false,
             target,
             supported_atoms.XdndTypeList,
             AtomEnum::ANY,
             0,
             1024,
-        )
-        .unwrap();
-    if let Ok(reply) = property.reply() {
+        ),
+    )
+    .log_with_level(Level::Warn)
+    {
         if let Some(atoms) = reply.value32() {
             for atom in atoms {
                 if xdnd_is_atom_supported(atom, &supported_atoms) {
@@ -2016,9 +2082,11 @@ fn xdnd_send_finished(
         sequence: 0,
         response_type: xproto::CLIENT_MESSAGE_EVENT,
     };
-    xcb_connection
-        .send_event(false, target, EventMask::default(), message)
-        .unwrap();
+    check_reply(
+        || "Failed to send XDnD finished event",
+        xcb_connection.send_event(false, target, EventMask::default(), message),
+    )
+    .log_err();
 }
 
 fn xdnd_send_status(
@@ -2036,22 +2104,24 @@ fn xdnd_send_status(
         sequence: 0,
         response_type: xproto::CLIENT_MESSAGE_EVENT,
     };
-    xcb_connection
-        .send_event(false, target, EventMask::default(), message)
-        .unwrap();
+    check_reply(
+        || "Failed to send XDnD status event",
+        xcb_connection.send_event(false, target, EventMask::default(), message),
+    )
+    .log_err();
 }
 
 /// Recomputes `pointer_device_states` by querying all pointer devices.
 /// When a device is present in `scroll_values_to_preserve`, its value for `ScrollAxisState.scroll_value` is used.
-fn get_new_pointer_device_states(
+fn current_pointer_device_states(
     xcb_connection: &XCBConnection,
     scroll_values_to_preserve: &BTreeMap<xinput::DeviceId, PointerDeviceState>,
-) -> BTreeMap<xinput::DeviceId, PointerDeviceState> {
-    let devices_query_result = xcb_connection
-        .xinput_xi_query_device(XINPUT_ALL_DEVICES)
-        .unwrap()
-        .reply()
-        .unwrap();
+) -> Option<BTreeMap<xinput::DeviceId, PointerDeviceState>> {
+    let devices_query_result = get_reply(
+        || "Failed to query XInput devices",
+        xcb_connection.xinput_xi_query_device(XINPUT_ALL_DEVICES),
+    )
+    .log_err()?;
 
     let mut pointer_device_states = BTreeMap::new();
     pointer_device_states.extend(

crates/gpui/src/platform/linux/x11/window.rs 🔗

@@ -1,4 +1,5 @@
 use anyhow::{Context as _, anyhow};
+use x11rb::connection::RequestConnection;
 
 use crate::platform::blade::{BladeContext, BladeRenderer, BladeSurfaceConfig};
 use crate::{
@@ -32,6 +33,7 @@ use std::{
 };
 
 use super::{X11Display, XINPUT_ALL_DEVICE_GROUPS, XINPUT_ALL_DEVICES};
+
 x11rb::atom_manager! {
     pub XcbAtoms: AtomsCookie {
         XA_ATOM,
@@ -318,43 +320,57 @@ impl rwh::HasDisplayHandle for X11Window {
     }
 }
 
-fn check_reply<C, F>(
+pub(crate) fn check_reply<E, F, C>(
     failure_context: F,
-    result: Result<VoidCookie<'_, Rc<XCBConnection>>, ConnectionError>,
+    result: Result<VoidCookie<'_, C>, ConnectionError>,
 ) -> anyhow::Result<()>
 where
-    C: Display + Send + Sync + 'static,
-    F: FnOnce() -> C,
+    E: Display + Send + Sync + 'static,
+    F: FnOnce() -> E,
+    C: RequestConnection,
 {
     result
-        .map_err(|connection_error| anyhow!(connection_error))
-        .and_then(|response| {
-            response
-                .check()
-                .map_err(|error_response| anyhow!(error_response))
-        })
+        .map_err(handle_connection_error)
+        .and_then(|response| response.check().map_err(|reply_error| anyhow!(reply_error)))
         .with_context(failure_context)
 }
 
-fn get_reply<C, F, O>(
+pub(crate) fn get_reply<E, F, C, O>(
     failure_context: F,
-    result: Result<Cookie<'_, Rc<XCBConnection>, O>, ConnectionError>,
+    result: Result<Cookie<'_, C, O>, ConnectionError>,
 ) -> anyhow::Result<O>
 where
-    C: Display + Send + Sync + 'static,
-    F: FnOnce() -> C,
+    E: Display + Send + Sync + 'static,
+    F: FnOnce() -> E,
+    C: RequestConnection,
     O: x11rb::x11_utils::TryParse,
 {
     result
-        .map_err(|connection_error| anyhow!(connection_error))
-        .and_then(|response| {
-            response
-                .reply()
-                .map_err(|error_response| anyhow!(error_response))
-        })
+        .map_err(handle_connection_error)
+        .and_then(|response| response.reply().map_err(|reply_error| anyhow!(reply_error)))
         .with_context(failure_context)
 }
 
+/// Convert X11 connection errors to `anyhow::Error` and panic for unrecoverable errors.
+pub(crate) fn handle_connection_error(err: ConnectionError) -> anyhow::Error {
+    match err {
+        ConnectionError::UnknownError => anyhow!("X11 connection: Unknown error"),
+        ConnectionError::UnsupportedExtension => anyhow!("X11 connection: Unsupported extension"),
+        ConnectionError::MaximumRequestLengthExceeded => {
+            anyhow!("X11 connection: Maximum request length exceeded")
+        }
+        ConnectionError::FdPassingFailed => {
+            panic!("X11 connection: File descriptor passing failed")
+        }
+        ConnectionError::ParseError(parse_error) => {
+            anyhow!(parse_error).context("Parse error in X11 response")
+        }
+        ConnectionError::InsufficientMemory => panic!("X11 connection: Insufficient memory"),
+        ConnectionError::IoError(err) => anyhow!(err).context("X11 connection: IOError"),
+        _ => anyhow!(err),
+    }
+}
+
 impl X11WindowState {
     pub fn new(
         handle: AnyWindowHandle,
@@ -581,7 +597,7 @@ impl X11WindowState {
                 ),
             )?;
 
-            xcb.flush().with_context(|| "X11 Flush failed.")?;
+            xcb.flush()?;
 
             let renderer = {
                 let raw_window = RawWindow {
@@ -641,8 +657,7 @@ impl X11WindowState {
                 || "X11 DestroyWindow failed while cleaning it up after setup failure.",
                 xcb.destroy_window(x_window),
             )?;
-            xcb.flush()
-                .with_context(|| "X11 Flush failed while cleaning it up after setup failure.")?;
+            xcb.flush()?;
         }
 
         setup_result
@@ -670,9 +685,7 @@ impl Drop for X11WindowHandle {
                 || "X11 DestroyWindow failed while dropping X11WindowHandle.",
                 self.xcb.destroy_window(self.id),
             )?;
-            self.xcb
-                .flush()
-                .with_context(|| "X11 Flush failed while dropping X11WindowHandle.")?;
+            self.xcb.flush()?;
             anyhow::Ok(())
         })
         .log_err();
@@ -691,10 +704,7 @@ impl Drop for X11Window {
                 || "X11 DestroyWindow failure.",
                 self.0.xcb.destroy_window(self.0.x_window),
             )?;
-            self.0
-                .xcb
-                .flush()
-                .with_context(|| "X11 Flush failed after calling DestroyWindow.")?;
+            self.0.xcb.flush()?;
 
             anyhow::Ok(())
         })
@@ -812,7 +822,7 @@ impl X11Window {
         let state = self.0.state.borrow();
 
         check_reply(
-            || "X11 UngrabPointer before move/resize of window ailed.",
+            || "X11 UngrabPointer before move/resize of window failed.",
             self.0.xcb.ungrab_pointer(x11rb::CURRENT_TIME),
         )?;
 
@@ -846,7 +856,11 @@ impl X11Window {
     }
 
     fn flush(&self) -> anyhow::Result<()> {
-        self.0.xcb.flush().with_context(|| "X11 Flush failed.")
+        self.0
+            .xcb
+            .flush()
+            .map_err(handle_connection_error)
+            .context("X11 flush failed")
     }
 }
 
@@ -889,9 +903,13 @@ impl X11WindowStatePtr {
         )?;
 
         if reply.value_len != 0 {
-            let atom = u32::from_ne_bytes(reply.value[0..4].try_into().unwrap());
-            let edge_constraints = EdgeConstraints::from_atom(atom);
-            state.edge_constraints.replace(edge_constraints);
+            if let Ok(bytes) = reply.value[0..4].try_into() {
+                let atom = u32::from_ne_bytes(bytes);
+                let edge_constraints = EdgeConstraints::from_atom(atom);
+                state.edge_constraints.replace(edge_constraints);
+            } else {
+                log::error!("Failed to parse GTK_EDGE_CONSTRAINTS");
+            }
         }
 
         Ok(())
@@ -1030,7 +1048,7 @@ impl X11WindowStatePtr {
         bounds
     }
 
-    pub fn configure(&self, bounds: Bounds<i32>) -> anyhow::Result<()> {
+    pub fn set_bounds(&self, bounds: Bounds<i32>) -> anyhow::Result<()> {
         let mut resize_args = None;
         let is_resize;
         {
@@ -1196,12 +1214,14 @@ impl PlatformWindow for X11Window {
     }
 
     fn mouse_position(&self) -> Point<Pixels> {
-        let reply = get_reply(
+        get_reply(
             || "X11 QueryPointer failed.",
             self.0.xcb.query_pointer(self.0.x_window),
         )
-        .unwrap();
-        Point::new((reply.root_x as u32).into(), (reply.root_y as u32).into())
+        .log_err()
+        .map_or(Point::new(Pixels(0.0), Pixels(0.0)), |reply| {
+            Point::new((reply.root_x as u32).into(), (reply.root_y as u32).into())
+        })
     }
 
     fn modifiers(&self) -> Modifiers {
@@ -1269,7 +1289,7 @@ impl PlatformWindow for X11Window {
                 xproto::Time::CURRENT_TIME,
             )
             .log_err();
-        self.flush().unwrap();
+        self.flush().log_err();
     }
 
     fn is_active(&self) -> bool {
@@ -1323,7 +1343,7 @@ impl PlatformWindow for X11Window {
                 &data,
             ),
         )
-        .unwrap();
+        .log_err();
     }
 
     fn map_window(&mut self) -> anyhow::Result<()> {
@@ -1359,7 +1379,7 @@ impl PlatformWindow for X11Window {
                 message,
             ),
         )
-        .unwrap();
+        .log_err();
     }
 
     fn zoom(&self) {
@@ -1370,7 +1390,7 @@ impl PlatformWindow for X11Window {
             state.atoms._NET_WM_STATE_MAXIMIZED_VERT,
             state.atoms._NET_WM_STATE_MAXIMIZED_HORZ,
         )
-        .unwrap();
+        .log_err();
     }
 
     fn toggle_fullscreen(&self) {
@@ -1381,7 +1401,7 @@ impl PlatformWindow for X11Window {
             state.atoms._NET_WM_STATE_FULLSCREEN,
             xproto::AtomEnum::NONE.into(),
         )
-        .unwrap();
+        .log_err();
     }
 
     fn is_fullscreen(&self) -> bool {
@@ -1444,9 +1464,11 @@ impl PlatformWindow for X11Window {
             || "X11 UngrabPointer failed.",
             self.0.xcb.ungrab_pointer(x11rb::CURRENT_TIME),
         )
-        .unwrap();
+        .log_err();
 
-        let coords = self.get_root_position(position).unwrap();
+        let Some(coords) = self.get_root_position(position).log_err() else {
+            return;
+        };
         let message = ClientMessageEvent::new(
             32,
             self.0.x_window,
@@ -1468,16 +1490,16 @@ impl PlatformWindow for X11Window {
                 message,
             ),
         )
-        .unwrap();
+        .log_err();
     }
 
     fn start_window_move(&self) {
         const MOVERESIZE_MOVE: u32 = 8;
-        self.send_moveresize(MOVERESIZE_MOVE).unwrap();
+        self.send_moveresize(MOVERESIZE_MOVE).log_err();
     }
 
     fn start_window_resize(&self, edge: ResizeEdge) {
-        self.send_moveresize(edge.to_moveresize()).unwrap();
+        self.send_moveresize(edge.to_moveresize()).log_err();
     }
 
     fn window_decorations(&self) -> crate::Decorations {
@@ -1552,7 +1574,7 @@ impl PlatformWindow for X11Window {
                     bytemuck::cast_slice::<u32, u8>(&insets),
                 ),
             )
-            .unwrap();
+            .log_err();
         }
     }
 
@@ -1574,7 +1596,7 @@ impl PlatformWindow for X11Window {
             WindowDecorations::Client => [1 << 1, 0, 0, 0, 0],
         };
 
-        check_reply(
+        let success = check_reply(
             || "X11 ChangeProperty for _MOTIF_WM_HINTS failed.",
             self.0.xcb.change_property(
                 xproto::PropMode::REPLACE,
@@ -1586,7 +1608,11 @@ impl PlatformWindow for X11Window {
                 bytemuck::cast_slice::<u32, u8>(&hints_data),
             ),
         )
-        .unwrap();
+        .log_err();
+
+        let Some(()) = success else {
+            return;
+        };
 
         match decorations {
             WindowDecorations::Server => {