Return errors instead of panicking when interacting with the keychain

Closes #134

gpui/src/platform.rs

@@ -18,6 +18,7 @@ use crate::{
     text_layout::LineLayout,
     AnyAction, ClipboardItem, Menu, Scene,
 };
+use anyhow::Result;
 use async_task::Runnable;
 pub use event::Event;
 use std::{
@@ -46,8 +47,8 @@ pub trait Platform: Send + Sync {
     fn read_from_clipboard(&self) -> Option<ClipboardItem>;
     fn open_url(&self, url: &str);
 
-    fn write_credentials(&self, url: &str, username: &str, password: &[u8]);
-    fn read_credentials(&self, url: &str) -> Option<(String, Vec<u8>)>;
+    fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()>;
+    fn read_credentials(&self, url: &str) -> Result<Option<(String, Vec<u8>)>>;
 
     fn set_cursor_style(&self, style: CursorStyle);
 

gpui/src/platform/mac/platform.rs

@@ -5,6 +5,7 @@ use crate::{
     platform::{self, CursorStyle},
     AnyAction, ClipboardItem, Event, Menu, MenuItem,
 };
+use anyhow::{anyhow, Result};
 use block::ConcreteBlock;
 use cocoa::{
     appkit::{
@@ -469,7 +470,7 @@ impl platform::Platform for MacPlatform {
         }
     }
 
-    fn write_credentials(&self, url: &str, username: &str, password: &[u8]) {
+    fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()> {
         let url = CFString::from(url);
         let username = CFString::from(username);
         let password = CFData::from_buffer(password);
@@ -502,12 +503,13 @@ impl platform::Platform for MacPlatform {
             }
 
             if status != errSecSuccess {
-                panic!("{} password failed: {}", verb, status);
+                return Err(anyhow!("{} password failed: {}", verb, status));
             }
         }
+        Ok(())
     }
 
-    fn read_credentials(&self, url: &str) -> Option<(String, Vec<u8>)> {
+    fn read_credentials(&self, url: &str) -> Result<Option<(String, Vec<u8>)>> {
         let url = CFString::from(url);
         let cf_true = CFBoolean::true_value().as_CFTypeRef();
 
@@ -525,27 +527,27 @@ impl platform::Platform for MacPlatform {
             let status = SecItemCopyMatching(attrs.as_concrete_TypeRef(), &mut result);
             match status {
                 security::errSecSuccess => {}
-                security::errSecItemNotFound | security::errSecUserCanceled => return None,
-                _ => panic!("reading password failed: {}", status),
+                security::errSecItemNotFound | security::errSecUserCanceled => return Ok(None),
+                _ => return Err(anyhow!("reading password failed: {}", status)),
             }
 
             let result = CFType::wrap_under_create_rule(result)
                 .downcast::<CFDictionary>()
-                .expect("keychain item was not a dictionary");
+                .ok_or_else(|| anyhow!("keychain item was not a dictionary"))?;
             let username = result
                 .find(kSecAttrAccount as *const _)
-                .expect("account was missing from keychain item");
+                .ok_or_else(|| anyhow!("account was missing from keychain item"))?;
             let username = CFType::wrap_under_get_rule(*username)
                 .downcast::<CFString>()
-                .expect("account was not a string");
+                .ok_or_else(|| anyhow!("account was not a string"))?;
             let password = result
                 .find(kSecValueData as *const _)
-                .expect("password was missing from keychain item");
+                .ok_or_else(|| anyhow!("password was missing from keychain item"))?;
             let password = CFType::wrap_under_get_rule(*password)
                 .downcast::<CFData>()
-                .expect("password was not a string");
+                .ok_or_else(|| anyhow!("password was not a string"))?;
 
-            Some((username.to_string(), password.bytes().to_vec()))
+            Ok(Some((username.to_string(), password.bytes().to_vec())))
         }
     }
 

gpui/src/platform/test.rs

@@ -1,5 +1,6 @@
 use super::CursorStyle;
 use crate::{AnyAction, ClipboardItem};
+use anyhow::Result;
 use parking_lot::Mutex;
 use pathfinder_geometry::vector::Vector2F;
 use std::{
@@ -128,10 +129,12 @@ impl super::Platform for Platform {
 
     fn open_url(&self, _: &str) {}
 
-    fn write_credentials(&self, _: &str, _: &str, _: &[u8]) {}
+    fn write_credentials(&self, _: &str, _: &str, _: &[u8]) -> Result<()> {
+        Ok(())
+    }
 
-    fn read_credentials(&self, _: &str) -> Option<(String, Vec<u8>)> {
-        None
+    fn read_credentials(&self, _: &str) -> Result<Option<(String, Vec<u8>)>> {
+        Ok(None)
     }
 
     fn set_cursor_style(&self, style: CursorStyle) {

zed/src/rpc.rs

@@ -1,3 +1,4 @@
+use crate::util::ResultExt;
 use anyhow::{anyhow, Context, Result};
 use async_tungstenite::tungstenite::http::Request;
 use async_tungstenite::tungstenite::{Error as WebSocketError, Message as WebSocketMessage};
@@ -236,7 +237,11 @@ impl Client {
     ) -> Task<Result<(String, String)>> {
         let executor = executor.clone();
         executor.clone().spawn(async move {
-            if let Some((user_id, access_token)) = platform.read_credentials(&ZED_SERVER_URL) {
+            if let Some((user_id, access_token)) = platform
+                .read_credentials(&ZED_SERVER_URL)
+                .log_err()
+                .flatten()
+            {
                 log::info!("already signed in. user_id: {}", user_id);
                 return Ok((user_id, String::from_utf8(access_token).unwrap()));
             }
@@ -301,7 +306,9 @@ impl Client {
                 .decrypt_string(&access_token)
                 .context("failed to decrypt access token")?;
             platform.activate(true);
-            platform.write_credentials(&ZED_SERVER_URL, &user_id, access_token.as_bytes());
+            platform
+                .write_credentials(&ZED_SERVER_URL, &user_id, access_token.as_bytes())
+                .log_err();
             Ok((user_id.to_string(), access_token))
         })
     }