{"action":"created","issue":{"url":"https://api.github.com/repos/plietar/librespot/issues/127","repository_url":"https://api.github.com/repos/plietar/librespot","labels_url":"https://api.github.com/repos/plietar/librespot/issues/127/labels{/name}","comments_url":"https://api.github.com/repos/plietar/librespot/issues/127/comments","events_url":"https://api.github.com/repos/plietar/librespot/issues/127/events","html_url":"https://github.com/plietar/librespot/issues/127","id":188923617,"number":127,"title":"unwraps in network code","user":{"login":"crepererum","id":1529400,"avatar_url":"https://avatars.githubusercontent.com/u/1529400?v=3","gravatar_id":"","url":"https://api.github.com/users/crepererum","html_url":"https://github.com/crepererum","followers_url":"https://api.github.com/users/crepererum/followers","following_url":"https://api.github.com/users/crepererum/following{/other_user}","gists_url":"https://api.github.com/users/crepererum/gists{/gist_id}","starred_url":"https://api.github.com/users/crepererum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/crepererum/subscriptions","organizations_url":"https://api.github.com/users/crepererum/orgs","repos_url":"https://api.github.com/users/crepererum/repos","events_url":"https://api.github.com/users/crepererum/events{/privacy}","received_events_url":"https://api.github.com/users/crepererum/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2016-11-12T17:32:07Z","updated_at":"2017-01-13T06:47:33Z","closed_at":null,"body":"The network-related code (e.g. in `session.rs`) uses unwrap, even in places where failure is kinda likely. For example:\r\n```\r\nthread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: IoError(Error { repr: Os { code: 104, message: \"Connection reset by peer\" } })', /buil\r\ndslave/rust-buildbot/slave/stable-dist-rustc-cross-host-linux/build/src/libcore/result.rs:799\r\nstack backtrace:\r\n   1:       0x557f3db8d3 - std::sys::backtrace::tracing::imp::write::hd28a1f8221c946e5\r\n   2:       0x557f3dfd9b - std::panicking::default_hook::{{closure}}::hcb904beb56706ec8\r\n   3:       0x557f3de75b - std::panicking::default_hook::hd59cb475a55dc643\r\n   4:       0x557f3dedcf - std::panicking::rust_panic_with_hook::hea2bde53d708eb9a\r\n   5:       ne557f3dec9b - std::panicking::begin_panic::hc4949303f890336e\r\n   6:       0x557f3debeb - std::panicking::begin_panic_fmt::he756d57ad1c1864c\r\n   7:       0x557f3deb8b - rust_begin_unwind\r\n   8:       0x557f40825b - core::panicking::panic_fmt::ha615cb8b8c64841b\r\n   9:       0x557f1a9123 - core::result::unwrap_failed::hf088160fd5523cfd\r\n  10:       0x557f1f211b - librespot::session::Session::recv::h1029c0727cfdd2f5\r\n  11:       0x557f1f1bb7 - librespot::session::Session::poll::h8d6b905a3d50b9e1\r\n  12:       0x557f16f5db - librespot::main::ha3ae70879a6a6aa7\r\n  13:       0x557f3e512f - __rust_maybe_catch_panic\r\n  14:       0x557f3ddedb - std::rt::lang_start::hcb77440d8735d978\r\n  15:       0x7fa1c4a363 - __libc_start_main\r\n  2::       0x557f167363 - <unknown>\r\n```\r\n\r\nConnection resets are a normal thing, esp. in Wifi-environments. So the `recv` method should probably return a Result-type instead and the layer above (`poll`) should handle failed connection attempts, e.g. by reconnect/retry."},"comment":{"url":"https://api.github.com/repos/plietar/librespot/issues/comments/272373547","html_url":"https://github.com/plietar/librespot/issues/127#issuecomment-272373547","issue_url":"https://api.github.com/repos/plietar/librespot/issues/127","id":272373547,"user":{"login":"crepererum","id":1529400,"avatar_url":"https://avatars.githubusercontent.com/u/1529400?v=3","gravatar_id":"","url":"https://api.github.com/users/crepererum","html_url":"https://github.com/crepererum","followers_url":"https://api.github.com/users/crepererum/followers","following_url":"https://api.github.com/users/crepererum/following{/other_user}","gists_url":"https://api.github.com/users/crepererum/gists{/gist_id}","starred_url":"https://api.github.com/users/crepererum/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/crepererum/subscriptions","organizations_url":"https://api.github.com/users/crepererum/orgs","repos_url":"https://api.github.com/users/crepererum/repos","events_url":"https://api.github.com/users/crepererum/events{/privacy}","received_events_url":"https://api.github.com/users/crepererum/received_events","type":"User","site_admin":false},"created_at":"2017-01-13T06:47:33Z","updated_at":"2017-01-13T06:47:33Z","body":"I guess the reset means that either the Spotify server directly or some node in between cuts the TCP connection. This ain't uncommon, expecially when the server suffers under high loads. Network problems are (IMHO) such normal that they shouldn't produce a panic. But as @plietar already mentioned, proof of concept code does hit always follow this rule. "}}
{"actor":{"display_login":"crepererum"}}