Merge pull request #9 from seijikun/master

Attempt to fix #2: Hard parser crashes

Cargo.toml

···
 owo-colors = "3.5.0"
 serde = "1.0.152"
 serde_json = "1.0.91"
-socket2 = "0.4.7"
 surf = { version = "2.3.2", features = ["h1-client-rustls"], default-features = false}
 tokio = { version = "1.24.2", features = ["tokio-macros", "macros", "rt", "rt-multi-thread"] }
 url = "2.3.1"

examples/discover.rs

···
 
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let devices = discover_pnp_locations();
+    let devices = discover_pnp_locations().await?;
     tokio::pin!(devices);
 
     while let Some(device) = devices.next().await {

examples/media_renderer_client.rs

···
 
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let devices = discover_pnp_locations();
+    let devices = discover_pnp_locations().await?;
     tokio::pin!(devices);
 
     let mut kodi_device: Option<Device> = None;
···
     }
 
     let kodi_device = kodi_device.unwrap();
-    let device_client = DeviceClient::new(&kodi_device.location).connect().await?;
+    let device_client = DeviceClient::new(&kodi_device.location)?.connect().await?;
     let mut media_renderer = MediaRendererClient::new(device_client);
 
     let options = LoadOptions {

examples/media_server_client.rs

···
 
 #[tokio::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let devices = discover_pnp_locations();
+    let devices = discover_pnp_locations().await?;
     tokio::pin!(devices);
 
     let mut kodi_device: Option<Device> = None;
···
     }
 
     let kodi_device = kodi_device.unwrap();
-    let device_client = DeviceClient::new(&kodi_device.location).connect().await?;
+    let device_client = DeviceClient::new(&kodi_device.location)?.connect().await?;
     let media_server_client = MediaServerClient::new(device_client);
     let results = media_server_client
         .browse("0", "BrowseDirectChildren")

src/device_client.rs

···
     env,
     net::TcpListener,
     sync::{
-        mpsc::{self, Receiver, Sender},
-        Arc, Mutex,
+        Arc,
     },
     time::Duration,
 };
···
     types::{AVTransportEvent, Device, Event, Service},
     BROADCAST_EVENT,
 };
-use anyhow::Error;
+use anyhow::{anyhow, Result};
 use hyper::{
     server::conn::AddrStream,
     service::{make_service_fn, service_fn},
 };
 use hyper::{Body, Request, Response, Server};
 use surf::{Client, Config, Url};
+use tokio::sync::Mutex;
 use xml_builder::{XMLBuilder, XMLElement, XMLVersion};
 
 #[derive(Clone)]
···
 }
 
 impl DeviceClient {
-    pub fn new(url: &str) -> Self {
-        Self {
-            base_url: Url::parse(url).unwrap(),
+    pub fn new(url: &str) -> Result<Self> {
+        Ok(Self {
+            base_url: Url::parse(url)?,
             http_client: Config::new()
                 .set_timeout(Some(Duration::from_secs(5)))
-                .try_into()
-                .unwrap(),
+                .try_into()?,
             device: None,
             stop: Arc::new(Mutex::new(false)),
-        }
+        })
     }
 
-    pub async fn connect(&mut self) -> Result<Self, Error> {
+    pub async fn connect(&mut self) -> Result<Self> {
         self.device = Some(parse_location(self.base_url.as_str()).await?);
         Ok(Self {
             base_url: self.base_url.clone(),
···
         service_id: &str,
         action_name: &str,
         params: HashMap<String, String>,
-    ) -> Result<String, Error> {
+    ) -> Result<String> {
         if self.device.is_none() {
-            return Err(Error::msg("Device not connected"));
+            return Err(anyhow!("Device not connected"));
         }
         let service_id = resolve_service(service_id);
         let service = self.get_service_description(&service_id).await?;
···
                 self.call_action_internal(&service, action_name, params)
                     .await
             }
-            None => Err(Error::msg("Action not found")),
+            None => Err(anyhow!("Action not found")),
         }
     }
 
···
         service: &Service,
         action_name: &str,
         params: HashMap<String, String>,
-    ) -> Result<String, Error> {
-        let control_url = Url::parse(&service.control_url).unwrap();
+    ) -> Result<String> {
+        let control_url = Url::parse(&service.control_url)?;
 
         let mut xml = XMLBuilder::new()
             .version(XMLVersion::XML1_1)
···
 
         for (name, value) in params {
             let mut param = XMLElement::new(name.as_str());
-            param.add_text(value).unwrap();
-            action.add_child(param).unwrap();
+            param.add_text(value)?;
+            action.add_child(param)?;
         }
 
-        body.add_child(action).unwrap();
-        envelope.add_child(body).unwrap();
+        body.add_child(action)?;
+        envelope.add_child(body)?;
 
         xml.set_root_element(envelope);
 
         let mut writer: Vec<u8> = Vec::new();
-        xml.generate(&mut writer).unwrap();
-        let xml = String::from_utf8(writer).unwrap();
+        xml.generate(&mut writer)?;
+        let xml = String::from_utf8(writer)?;
 
         let soap_action = format!("\"{}#{}\"", service.service_type, action_name);
 
···
             .body_string(xml.clone())
             .send()
             .await
-            .map_err(|e| Error::msg(e.to_string()))?;
-        Ok(res
+            .map_err(|e| anyhow!(e.to_string()))?;
+        res
             .body_string()
             .await
-            .map_err(|e| Error::msg(e.to_string()))?)
+            .map_err(|e| anyhow!(e.to_string()))
     }
 
-    async fn get_service_description(&self, service_id: &str) -> Result<Service, Error> {
+    async fn get_service_description(&self, service_id: &str) -> Result<Service> {
         if let Some(device) = &self.device {
             let service = device
                 .services
                 .iter()
                 .find(|s| s.service_id == service_id)
-                .unwrap();
+                .ok_or_else(|| anyhow!("Service with requested service_id {} does not exist", service_id))?;
             return Ok(service.clone());
         }
-        Err(Error::msg("Device not connected"))
+        Err(anyhow!("Device not connected"))
     }
 
-    pub async fn subscribe(&mut self, service_id: &str) -> Result<(), Error> {
+    pub async fn subscribe(&mut self, service_id: &str) -> Result<()> {
         if self.device.is_none() {
-            return Err(Error::msg("Device not connected"));
+            return Err(anyhow!("Device not connected"));
         }
         let service_id = resolve_service(service_id);
         let service = self.get_service_description(&service_id).await?;
···
             .header("NT", "upnp:event")
             .header("TIMEOUT", "Second-1800")
             .header("USER-AGENT", user_agent)
-            .body(hyper::Body::empty())
-            .unwrap();
+            .body(hyper::Body::empty())?;
         client.request(req).await?;
         Ok(())
     }
 
-    pub async fn unsubscribe(&mut self, service_id: &str, sid: &str) -> Result<(), Error> {
+    pub async fn unsubscribe(&mut self, service_id: &str, sid: &str) -> Result<()> {
         if self.device.is_none() {
-            return Err(Error::msg("Device not connected"));
+            return Err(anyhow!("Device not connected"));
         }
         let service_id = resolve_service(service_id);
-        let service = self.get_service_description(&service_id).await.unwrap();
+        let service = self.get_service_description(&service_id).await?;
         let client = hyper::Client::new();
         let req = hyper::Request::builder()
             .method("UNSUBSCRIBE")
             .uri(service.event_sub_url.clone())
             .header("SID", sid)
-            .body(hyper::Body::empty())
-            .unwrap();
+            .body(hyper::Body::empty())?;
 
         client.request(req).await?;
 
···
         Ok(())
     }
 
-    async fn ensure_eventing_server(&mut self) -> Result<(String, u16), Error> {
+    async fn ensure_eventing_server(&mut self) -> Result<(String, u16)> {
         let addr: &str = "0.0.0.0:0";
-        let listener = TcpListener::bind(&addr).unwrap();
+        let listener = TcpListener::bind(addr)?;
 
         let service = make_service_fn(|_: &AddrStream| async {
             Ok::<_, hyper::Error>(service_fn(|req: Request<Body>| async move {
···
                 let current_track_metadata =
                     parse_current_track_metadata(last_change.as_str()).unwrap();
 
-                match transport_state {
-                    Some(state) => {
-                        let tx = BROADCAST_EVENT.lock().unwrap();
-                        let tx = tx.as_ref().clone();
-                        let ev = AVTransportEvent::TransportState {
-                            sid: sid.clone(),
-                            transport_state: state,
-                        };
-                        tx.unwrap().send(Event::AVTransport(ev)).unwrap();
-                    }
-                    None => {}
+                if let Some(state) = transport_state {
+                    let tx = BROADCAST_EVENT.lock().unwrap();
+                    let tx = tx.as_ref();
+                    let ev = AVTransportEvent::TransportState {
+                        sid: sid.clone(),
+                        transport_state: state,
+                    };
+                    tx.unwrap().send(Event::AVTransport(ev)).unwrap();
                 }
 
-                match play_mode {
-                    Some(mode) => {
-                        let tx = BROADCAST_EVENT.lock().unwrap();
-                        let tx = tx.as_ref().clone();
-                        let ev = AVTransportEvent::CurrentPlayMode {
-                            sid: sid.clone(),
-                            play_mode: mode,
-                        };
-                        tx.unwrap().send(Event::AVTransport(ev)).unwrap();
-                    }
-                    None => {}
+                if let Some(mode) = play_mode {
+                    let tx = BROADCAST_EVENT.lock().unwrap();
+                    let tx = tx.as_ref();
+                    let ev = AVTransportEvent::CurrentPlayMode {
+                        sid: sid.clone(),
+                        play_mode: mode,
+                    };
+                    tx.unwrap().send(Event::AVTransport(ev)).unwrap();
                 }
 
-                match av_transport_uri_metadata {
-                    Some(metadata) => {
-                        let tx = BROADCAST_EVENT.lock().unwrap();
-                        let tx = tx.as_ref().clone();
-                        let m = deserialize_metadata(metadata.as_str()).unwrap();
-                        let ev = AVTransportEvent::AVTransportURIMetaData {
-                            sid: sid.clone(),
-                            url: m.url,
-                            title: m.title,
-                            artist: m.artist,
-                            album: m.album,
-                            album_art_uri: m.album_art_uri,
-                            genre: m.genre,
-                        };
-                        tx.unwrap().send(Event::AVTransport(ev)).unwrap();
-                    }
-                    None => {}
+                if let Some(metadata) = av_transport_uri_metadata {
+                    let tx = BROADCAST_EVENT.lock().unwrap();
+                    let tx = tx.as_ref();
+                    let m = deserialize_metadata(metadata.as_str()).unwrap();
+                    let ev = AVTransportEvent::AVTransportURIMetaData {
+                        sid: sid.clone(),
+                        url: m.url,
+                        title: m.title,
+                        artist: m.artist,
+                        album: m.album,
+                        album_art_uri: m.album_art_uri,
+                        genre: m.genre,
+                    };
+                    tx.unwrap().send(Event::AVTransport(ev)).unwrap();
                 }
 
-                match current_track_metadata {
-                    Some(metadata) => {
-                        let m = deserialize_metadata(metadata.as_str()).unwrap();
-                        let tx = BROADCAST_EVENT.lock().unwrap();
-                        let tx = tx.as_ref().clone();
-                        let ev = AVTransportEvent::CurrentTrackMetadata {
-                            sid: sid.clone(),
-                            url: m.url,
-                            title: m.title,
-                            artist: m.artist,
-                            album: m.album,
-                            album_art_uri: m.album_art_uri,
-                            genre: m.genre,
-                        };
-                        tx.unwrap().send(Event::AVTransport(ev)).unwrap();
-                    }
-                    None => {}
+                if let Some(metadata) = current_track_metadata {
+                    let m = deserialize_metadata(metadata.as_str()).unwrap();
+                    let tx = BROADCAST_EVENT.lock().unwrap();
+                    let tx = tx.as_ref();
+                    let ev = AVTransportEvent::CurrentTrackMetadata {
+                        sid: sid.clone(),
+                        url: m.url,
+                        title: m.title,
+                        artist: m.artist,
+                        album: m.album,
+                        album_art_uri: m.album_art_uri,
+                        genre: m.genre,
+                    };
+                    tx.unwrap().send(Event::AVTransport(ev)).unwrap();
                 }
 
                 Ok::<_, hyper::Error>(Response::new(Body::empty()))
···
         });
 
         tokio::spawn(async move {
-            while !*stop.lock().unwrap() {
+            while !*stop.lock().await {
                 tokio::time::sleep(Duration::from_millis(100)).await;
             }
         });
···
         Ok((address, port))
     }
 
-    async fn release_eventing_server(&mut self) -> Result<(), Error> {
-        let mut stop = self.stop.lock().unwrap();
+    async fn release_eventing_server(&mut self) -> Result<()> {
+        let mut stop = self.stop.lock().await;
         *stop = true;
         Ok(())
     }
 }
 
 fn resolve_service(service_id: &str) -> String {
-    match service_id.contains(":") {
+    match service_id.contains(':') {
         true => service_id.to_string(),
         false => format!("urn:upnp-org:serviceId:{}", service_id),
     }

src/discovery.rs

···
-use anyhow::Error;
+use anyhow::{anyhow, Result};
 use async_stream::stream;
 use futures_util::Stream;
-use socket2::{Domain, Protocol, Socket, Type};
 use std::collections::HashMap;
-use std::mem::MaybeUninit;
-use std::net::{IpAddr, Ipv4Addr, SocketAddr};
+use std::net::{Ipv4Addr, SocketAddr};
 use std::str;
-use std::thread::sleep;
-use std::time::Duration;
+use tokio::net::UdpSocket;
 
 use crate::parser::parse_location;
 use crate::types::Device;
···
                                  ST: ssdp:all\r\n\
                                  \r\n";
 
-pub fn discover_pnp_locations() -> impl Stream<Item = Device> {
-    // Create a UDP socket
-    let socket = Socket::new(Domain::IPV4, Type::DGRAM, Some(Protocol::UDP)).unwrap();
+pub async fn discover_pnp_locations() -> Result<impl Stream<Item = Device>> {
+    let any: SocketAddr = ([0, 0, 0, 0], 0).into();
+    let socket = UdpSocket::bind(any).await?;
+    socket.join_multicast_v4(Ipv4Addr::new(239, 255, 255, 250), Ipv4Addr::new(0, 0, 0, 0))?;
 
     // Set the socket address to the multicast IP and port for UPnP device discovery
-    let socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(239, 255, 255, 250)), 1900).into();
-
-    // Join the UPnP multicast group
-    socket
-        .join_multicast_v4(
-            &Ipv4Addr::new(239, 255, 255, 250),
-            &Ipv4Addr::new(0, 0, 0, 0),
-        )
-        .unwrap();
+    let socket_addr: SocketAddr = ([239, 255, 255, 250], 1900).into();
 
     // Send the discovery request
     socket
         .send_to(DISCOVERY_REQUEST.as_bytes(), &socket_addr)
-        .unwrap();
+        .await?;
 
-    stream! {
+    Ok(stream! {
         loop {
-          // Receive the discovery response
-          let mut buf = [MaybeUninit::uninit(); 2048];
-          let (size, _) = socket.recv_from(&mut buf).unwrap();
-          // Convert the response to a string
-          let response =
-              str::from_utf8(unsafe { std::slice::from_raw_parts(buf.as_ptr() as *const u8, size) })
-                  .unwrap();
-          let headers = parse_raw_http_response(response).unwrap();
-          let location = *headers.get("location").unwrap();
-          yield parse_location(location).await.unwrap();
-          sleep(Duration::from_millis(500));
-      }
-    }
+            async fn get_next(socket: &UdpSocket) -> Result<String> {
+                // Receive the discovery response
+                let mut buf = [0; 2048];
+                let (size, _) = socket.recv_from(&mut buf).await?;
+                // Convert the response to a string
+                let response =
+                    str::from_utf8(unsafe { std::slice::from_raw_parts(buf.as_ptr() as *const u8, size) })?;
+                let headers = parse_raw_http_response(response)?;
+                let location = headers.get("location")
+                    .ok_or_else(|| anyhow!("Response header missing location"))?
+                    .to_string();
+                Ok(location)
+            }
+
+            if let Ok(location) = get_next(&socket).await {
+                if let Ok(device) = parse_location(&location).await {
+                    yield device;
+                }
+            }
+        }
+    })
 }
 
-fn parse_raw_http_response(response_str: &str) -> Result<HashMap<String, &str>, Error> {
+fn parse_raw_http_response(response_str: &str) -> Result<HashMap<String, &str>> {
     let mut headers = HashMap::new();
 
     match response_str.split("\r\n\r\n").next() {
···
             }
             Ok(headers)
         }
-        None => Err(Error::msg("Invalid HTTP response")),
+        None => Err(anyhow!("Invalid HTTP response")),
     }
 }

src/media_renderer.rs

···
-use std::{collections::HashMap, sync::mpsc, time::Duration};
+use std::{collections::HashMap, sync::mpsc};
 
 use anyhow::{Error, Ok};
 use async_stream::stream;

src/media_server.rs

···
 
         let ip = self.device_client.ip();
 
-        Ok(parse_browse_response(&response, &ip)?)
+        parse_browse_response(&response, &ip)
     }
 
     pub async fn get_sort_capabilities(&self) -> Result<(), Error> {

src/parser.rs

···
 use std::time::Duration;
 
 use crate::types::{Action, Argument, Container, Device, Item, Metadata, Service, TransportInfo};
-use anyhow::Error;
+use anyhow::{anyhow, Result};
 use elementtree::Element;
 use surf::{http::Method, Client, Config, Url};
 use xml::reader::XmlEvent;
 use xml::EventReader;
 
-pub async fn parse_location(location: &str) -> Result<Device, Error> {
+pub async fn parse_location(location: &str) -> Result<Device> {
     let client: Client = Config::new()
         .set_timeout(Some(Duration::from_secs(5)))
-        .try_into()
-        .unwrap();
-    let req = surf::Request::new(Method::Get, location.parse().unwrap());
-    let xml_root = client.recv_string(req).await.unwrap();
-
-    let mut device: Device = Device::default();
+        .try_into()?;
+    let req = surf::Request::new(Method::Get, location.parse()?);
+    let xml_root = client
+        .recv_string(req)
+        .await
+        .map_err(|e| anyhow!("Failed to retrieve xml from device endpoint: {}", e))?;
 
-    device.location = location.to_string();
+    let mut device = Device {
+        location: location.to_string(),
+        ..Default::default()
+    };
 
     device.device_type = parse_attribute(
         &xml_root,
···
     )?;
 
     let base_url = location.split('/').take(3).collect::<Vec<&str>>().join("/");
-    device.services = parse_services(&base_url, &xml_root).await;
+    device.services = parse_services(&base_url, &xml_root).await?;
 
     Ok(device)
 }
 
-fn parse_attribute(xml_root: &str, xml_name: &str) -> Result<String, Error> {
+fn parse_attribute(xml_root: &str, xml_name: &str) -> Result<String> {
     let root = Element::from_reader(xml_root.as_bytes())?;
     let mut xml_name = xml_name.split('/');
-    match root.find(xml_name.next().unwrap()) {
+    match root.find(
+        xml_name
+            .next()
+            .ok_or_else(|| anyhow!("xml_name ended unexpectedly"))?,
+    ) {
         Some(element) => {
-            let element = element.find(xml_name.next().unwrap());
+            let element = element.find(
+                xml_name
+                    .next()
+                    .ok_or_else(|| anyhow!("xml_name ended unexpectedly"))?,
+            );
             match element {
                 Some(element) => {
                     return Ok(element.text().to_string());
                 }
                 None => {
-                    return Ok("".to_string());
+                    Ok("".to_string())
                 }
             }
         }
···
     }
 }
 
-pub async fn parse_services(base_url: &str, xml_root: &str) -> Vec<Service> {
-    let root = Element::from_reader(xml_root.as_bytes()).unwrap();
+pub async fn parse_services(base_url: &str, xml_root: &str) -> Result<Vec<Service>> {
+    let root = Element::from_reader(xml_root.as_bytes())?;
     let device = root
         .find("{urn:schemas-upnp-org:device-1-0}device")
-        .unwrap();
+        .ok_or_else(|| anyhow!("Invalid response from device"))?;
 
     let mut services_with_actions: Vec<Service> = vec![];
     if let Some(service_list) = device.find("{urn:schemas-upnp-org:device-1-0}serviceList") {
-        let services = service_list.children();
+        let xml_services = service_list.children();
 
-        let services: Vec<Service> = services
-            .into_iter()
-            .map(|item| Service {
-                service_type: item
+        let mut services = Vec::new();
+        for xml_service in xml_services {
+            let mut service = Service {
+                service_type: xml_service
                     .find("{urn:schemas-upnp-org:device-1-0}serviceType")
-                    .unwrap()
+                    .ok_or_else(|| anyhow!("Service missing serviceType"))?
                     .text()
                     .to_string(),
-                service_id: item
+                service_id: xml_service
                     .find("{urn:schemas-upnp-org:device-1-0}serviceId")
-                    .unwrap()
+                    .ok_or_else(|| anyhow!("Service missing serviceId"))?
                     .text()
                     .to_string(),
-                control_url: item
+                control_url: xml_service
                     .find("{urn:schemas-upnp-org:device-1-0}controlURL")
-                    .unwrap()
+                    .ok_or_else(|| anyhow!("Service missing controlURL"))?
                     .text()
                     .to_string(),
-                event_sub_url: item
+                event_sub_url: xml_service
                     .find("{urn:schemas-upnp-org:device-1-0}eventSubURL")
-                    .unwrap()
+                    .ok_or_else(|| anyhow!("Service missing eventSubURL"))?
                     .text()
                     .to_string(),
-                scpd_url: item
+                scpd_url: xml_service
                     .find("{urn:schemas-upnp-org:device-1-0}SCPDURL")
-                    .unwrap()
+                    .ok_or_else(|| anyhow!("Service missing SCPDURL"))?
                     .text()
                     .to_string(),
                 actions: vec![],
-            })
-            .map(|mut service| {
-                service.control_url = build_absolute_url(base_url, &service.control_url);
-                service.event_sub_url = build_absolute_url(base_url, &service.event_sub_url);
-                service.scpd_url = build_absolute_url(base_url, &service.scpd_url);
-                service
-            })
-            .collect();
+            };
+
+            service.control_url = build_absolute_url(base_url, &service.control_url)?;
+            service.event_sub_url = build_absolute_url(base_url, &service.event_sub_url)?;
+            service.scpd_url = build_absolute_url(base_url, &service.scpd_url)?;
+
+            services.push(service);
+        }
 
         for service in &services {
             let mut service = service.clone();
-            service.actions = parse_service_description(&service.scpd_url).await;
+            service.actions = parse_service_description(&service.scpd_url).await?;
             services_with_actions.push(service);
         }
     }
 
-    services_with_actions
+    Ok(services_with_actions)
 }
 
-fn build_absolute_url(base_url: &str, relative_url: &str) -> String {
-    let base_url = Url::parse(base_url).unwrap();
-    base_url.join(relative_url).unwrap().to_string()
+fn build_absolute_url(base_url: &str, relative_url: &str) -> Result<String> {
+    let base_url = Url::parse(base_url)?;
+    Ok(base_url.join(relative_url)?.to_string())
 }
 
-pub async fn parse_service_description(scpd_url: &str) -> Vec<Action> {
+pub async fn parse_service_description(scpd_url: &str) -> Result<Vec<Action>> {
     let client: Client = Config::new()
         .set_timeout(Some(Duration::from_secs(5)))
-        .try_into()
-        .unwrap();
-    let req = surf::Request::new(Method::Get, scpd_url.parse().unwrap());
-    if let Ok(xml_root) = client.recv_string(req).await {
-        if let Ok(root) = Element::from_reader(xml_root.as_bytes()) {
-            let action_list = root.find("{urn:schemas-upnp-org:service-1-0}actionList");
+        .try_into()?;
+    let req = surf::Request::new(Method::Get, scpd_url.parse()?);
 
-            if action_list.is_none() {
-                return vec![];
-            }
+    let xml_root = client
+        .recv_string(req)
+        .await
+        .map_err(|e| anyhow!("Failed to retrieve xml response from device: {}", e))?;
+    let root = Element::from_reader(xml_root.as_bytes())?;
 
-            let action_list = action_list.unwrap().children();
-            let actions: Vec<Action> = action_list
-                .into_iter()
-                .map(|item| {
-                    let name = item
+    let action_list = match root.find("{urn:schemas-upnp-org:service-1-0}actionList") {
+        Some(action_list) => action_list,
+        None => return Ok(vec![]),
+    };
+
+    let mut actions = Vec::new();
+    for xml_action in action_list.children() {
+        let mut action = Action {
+            name: xml_action
+                .find("{urn:schemas-upnp-org:service-1-0}name")
+                .ok_or_else(|| anyhow!("Service::Action missing name"))?
+                .text()
+                .to_string(),
+            arguments: vec![],
+        };
+
+        if let Some(arguments) = xml_action.find("{urn:schemas-upnp-org:service-1-0}argumentList") {
+            for xml_argument in arguments.children() {
+                let argument = Argument {
+                    name: xml_argument
                         .find("{urn:schemas-upnp-org:service-1-0}name")
-                        .unwrap()
-                        .text();
-                    let arguments = item.find("{urn:schemas-upnp-org:service-1-0}argumentList");
-                    let arguments = arguments.unwrap().children();
-                    let arguments = arguments.into_iter().map(|item| {
-                        let name = item
-                            .find("{urn:schemas-upnp-org:service-1-0}name")
-                            .unwrap()
-                            .text();
-                        let direction = item
-                            .find("{urn:schemas-upnp-org:service-1-0}direction")
-                            .unwrap()
-                            .text();
-                        let related_state_variable = item
-                            .find("{urn:schemas-upnp-org:service-1-0}relatedStateVariable")
-                            .unwrap()
-                            .text();
-                        Argument {
-                            name: name.to_string(),
-                            direction: direction.to_string(),
-                            related_state_variable: related_state_variable.to_string(),
-                        }
-                    });
-                    Action {
-                        name: name.to_string(),
-                        arguments: arguments.collect(),
-                    }
-                })
-                .collect();
-            return actions;
+                        .ok_or_else(|| anyhow!("Service::Action::Argument missing name"))?
+                        .text()
+                        .to_string(),
+                    direction: xml_argument
+                        .find("{urn:schemas-upnp-org:service-1-0}direction")
+                        .ok_or_else(|| anyhow!("Service::Action::Argument missing direction"))?
+                        .text()
+                        .to_string(),
+                    related_state_variable: xml_argument
+                        .find("{urn:schemas-upnp-org:service-1-0}relatedStateVariable")
+                        .ok_or_else(|| {
+                            anyhow!("Service::Action::Argument missing relatedStateVariable")
+                        })?
+                        .text()
+                        .to_string(),
+                };
+                action.arguments.push(argument);
+            }
         }
+        actions.push(action);
     }
-    vec![]
+    Ok(actions)
 }
 
-pub fn parse_volume(xml_root: &str) -> Result<u8, Error> {
+pub fn parse_volume(xml_root: &str) -> Result<u8> {
     let parser = EventReader::from_str(xml_root);
     let mut in_current_volume = false;
     let mut current_volume: Option<u8> = None;
···
             }
             Ok(XmlEvent::Characters(volume)) => {
                 if in_current_volume {
-                    current_volume = Some(volume.parse().unwrap());
+                    current_volume = Some(volume.parse()?);
                 }
             }
             _ => {}
         }
     }
-    Ok(current_volume.unwrap())
+    current_volume.ok_or_else(|| anyhow!("Invalid response from device"))
 }
 
-pub fn parse_duration(xml_root: &str) -> Result<u32, Error> {
+pub fn parse_duration(xml_root: &str) -> Result<u32> {
     let parser = EventReader::from_str(xml_root);
     let mut in_duration = false;
     let mut duration: Option<String> = None;
···
             }
             Ok(XmlEvent::Characters(duration_str)) => {
                 if in_duration {
-                    let duration_str = duration_str.replace(":", "");
+                    let duration_str = duration_str.replace(':', "");
                     duration = Some(duration_str);
                 }
             }
···
         }
     }
 
-    let duration = duration.unwrap();
-    let hours = duration[0..2].parse::<u32>().unwrap();
-    let minutes = duration[2..4].parse::<u32>().unwrap();
-    let seconds = duration[4..6].parse::<u32>().unwrap();
+    let duration = duration.ok_or_else(|| anyhow!("Invalid response from device"))?;
+    let hours = duration[0..2].parse::<u32>()?;
+    let minutes = duration[2..4].parse::<u32>()?;
+    let seconds = duration[4..6].parse::<u32>()?;
     Ok(hours * 3600 + minutes * 60 + seconds)
 }
 
-pub fn parse_position(xml_root: &str) -> Result<u32, Error> {
+pub fn parse_position(xml_root: &str) -> Result<u32> {
     let parser = EventReader::from_str(xml_root);
     let mut in_position = false;
     let mut position: Option<String> = None;
···
             }
             Ok(XmlEvent::Characters(position_str)) => {
                 if in_position {
-                    let position_str = position_str.replace(":", "");
+                    let position_str = position_str.replace(':', "");
                     position = Some(position_str);
                 }
             }
···
         }
     }
 
-    let position = position.unwrap();
-    let hours = position[0..2].parse::<u32>().unwrap();
-    let minutes = position[2..4].parse::<u32>().unwrap();
-    let seconds = position[4..6].parse::<u32>().unwrap();
+    let position = position.ok_or_else(|| anyhow!("Invalid response from device"))?;
+    let hours = position[0..2].parse::<u32>()?;
+    let minutes = position[2..4].parse::<u32>()?;
+    let seconds = position[4..6].parse::<u32>()?;
     Ok(hours * 3600 + minutes * 60 + seconds)
 }
 
-pub fn parse_supported_protocols(xml_root: &str) -> Result<Vec<String>, Error> {
+pub fn parse_supported_protocols(xml_root: &str) -> Result<Vec<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut in_protocol = false;
     let mut protocols: String = "".to_string();
···
             _ => {}
         }
     }
-    Ok(protocols.split(",").map(|s| s.to_string()).collect())
+    Ok(protocols.split(',').map(|s| s.to_string()).collect())
 }
 
-pub fn parse_last_change(xml_root: &str) -> Result<Option<String>, Error> {
+pub fn parse_last_change(xml_root: &str) -> Result<Option<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut result = None;
     let mut in_last_change = false;
···
     Ok(result)
 }
 
-pub fn parse_current_play_mode(xml_root: &str) -> Result<Option<String>, Error> {
+pub fn parse_current_play_mode(xml_root: &str) -> Result<Option<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut current_play_mode: Option<String> = None;
-    for e in parser {
-        match e {
-            Ok(XmlEvent::StartElement {
-                name, attributes, ..
-            }) => {
-                if name.local_name == "CurrentPlayMode" {
-                    for attr in attributes {
-                        if attr.name.local_name == "val" {
-                            current_play_mode = Some(attr.value);
-                        }
+    for e in parser.into_iter().flatten() {
+        if let XmlEvent::StartElement { name, attributes, .. } = e {
+            if name.local_name == "CurrentPlayMode" {
+                for attr in attributes {
+                    if attr.name.local_name == "val" {
+                        current_play_mode = Some(attr.value);
                     }
                 }
             }
-            _ => {}
         }
     }
     Ok(current_play_mode)
 }
 
-pub fn parse_transport_state(xml_root: &str) -> Result<Option<String>, Error> {
+pub fn parse_transport_state(xml_root: &str) -> Result<Option<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut transport_state: Option<String> = None;
-    for e in parser {
-        match e {
-            Ok(XmlEvent::StartElement {
-                name, attributes, ..
-            }) => {
-                if name.local_name == "TransportState" {
-                    for attr in attributes {
-                        if attr.name.local_name == "val" {
-                            transport_state = Some(attr.value);
-                        }
+    for e in parser.into_iter().flatten() {
+        if let XmlEvent::StartElement { name, attributes, .. } = e {
+            if name.local_name == "TransportState" {
+                for attr in attributes {
+                    if attr.name.local_name == "val" {
+                        transport_state = Some(attr.value);
                     }
                 }
             }
-            _ => {}
         }
     }
     Ok(transport_state)
 }
 
-pub fn parse_av_transport_uri_metadata(xml_root: &str) -> Result<Option<String>, Error> {
+pub fn parse_av_transport_uri_metadata(xml_root: &str) -> Result<Option<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut av_transport_uri_metadata: Option<String> = None;
-    for e in parser {
-        match e {
-            Ok(XmlEvent::StartElement {
-                name, attributes, ..
-            }) => {
-                if name.local_name == "AVTransportURIMetaData" {
-                    for attr in attributes {
-                        if attr.name.local_name == "val" {
-                            av_transport_uri_metadata = Some(attr.value);
-                        }
+    for e in parser.into_iter().flatten() {
+        if let XmlEvent::StartElement { name, attributes, .. } = e {
+            if name.local_name == "AVTransportURIMetaData" {
+                for attr in attributes {
+                    if attr.name.local_name == "val" {
+                        av_transport_uri_metadata = Some(attr.value);
                     }
                 }
             }
-            _ => {}
         }
     }
     Ok(av_transport_uri_metadata)
 }
 
-pub fn parse_current_track_metadata(xml_root: &str) -> Result<Option<String>, Error> {
+pub fn parse_current_track_metadata(xml_root: &str) -> Result<Option<String>> {
     let parser = EventReader::from_str(xml_root);
     let mut current_track_metadata: Option<String> = None;
-    for e in parser {
-        match e {
-            Ok(XmlEvent::StartElement {
-                name, attributes, ..
-            }) => {
-                if name.local_name == "CurrentTrackMetaData" {
-                    for attr in attributes {
-                        if attr.name.local_name == "val" {
-                            current_track_metadata = Some(attr.value);
-                        }
+    for e in parser.into_iter().flatten() {
+        if let XmlEvent::StartElement { name, attributes, .. } = e {
+            if name.local_name == "CurrentTrackMetaData" {
+                for attr in attributes {
+                    if attr.name.local_name == "val" {
+                        current_track_metadata = Some(attr.value);
                     }
                 }
             }
-            _ => {}
         }
     }
     Ok(current_track_metadata)
 }
 
-pub fn deserialize_metadata(xml: &str) -> Result<Metadata, Error> {
+pub fn deserialize_metadata(xml: &str) -> Result<Metadata> {
     let parser = EventReader::from_str(xml);
     let mut in_title = false;
     let mut in_artist = false;
···
     })
 }
 
-pub fn parse_browse_response(xml: &str, ip: &str) -> Result<(Vec<Container>, Vec<Item>), Error> {
+pub fn parse_browse_response(xml: &str, ip: &str) -> Result<(Vec<Container>, Vec<Item>)> {
     let parser = EventReader::from_str(xml);
     let mut in_result = false;
     let mut result: (Vec<Container>, Vec<Item>) = (Vec::new(), Vec::new());
···
     Ok(result)
 }
 
-pub fn deserialize_content_directory(
-    xml: &str,
-    ip: &str,
-) -> Result<(Vec<Container>, Vec<Item>), Error> {
+pub fn deserialize_content_directory(xml: &str, ip: &str) -> Result<(Vec<Container>, Vec<Item>)> {
     let parser = EventReader::from_str(xml);
     let mut in_container = false;
     let mut in_item = false;
···
                             items.last_mut().unwrap().protocol_info = attr.value.clone();
                         }
                         if attr.name.local_name == "size" {
-                            items.last_mut().unwrap().size =
-                                Some(attr.value.parse::<u64>().unwrap());
+                            items.last_mut().unwrap().size = Some(attr.value.parse::<u64>()?);
                         }
                         if attr.name.local_name == "duration" {
                             items.last_mut().unwrap().duration = Some(attr.value.clone());
···
     Ok((containers, items))
 }
 
-pub fn parse_transport_info(xml: &str) -> Result<TransportInfo, Error> {
+pub fn parse_transport_info(xml: &str) -> Result<TransportInfo> {
     let parser = EventReader::from_str(xml);
     let mut in_transport_state = false;
     let mut in_transport_status = false;