Skip to content

Commit 32129b6

Browse files
niklasad1VladLupashevskyi
authored andcommitted
ParityShell::open Return result (openethereum#8377)
* start * add error handling for winapi * fix typo * fix warnings and windows errors * formatting * Address review comments
1 parent d838357 commit 32129b6

File tree

3 files changed

+46
-13
lines changed

3 files changed

+46
-13
lines changed

parity/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ fn take_spec_name_override() -> Option<String> {
231231
#[cfg(windows)]
232232
fn global_cleanup() {
233233
// We need to cleanup all sockets before spawning another Parity process. This makes shure everything is cleaned up.
234-
// The loop is required because of internal refernce counter for winsock dll. We don't know how many crates we use do
234+
// The loop is required because of internal reference counter for winsock dll. We don't know how many crates we use do
235235
// initialize it. There's at least 2 now.
236236
for _ in 0.. 10 {
237237
unsafe { ::winapi::um::winsock2::WSACleanup(); }

parity/run.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ pub fn open_ui(ws_conf: &rpc::WsConfiguration, ui_conf: &rpc::UiConfiguration, l
145145

146146
let token = signer::generate_token_and_url(ws_conf, ui_conf, logger_config)?;
147147
// Open a browser
148-
url::open(&token.url);
148+
url::open(&token.url).map_err(|e| format!("{}", e))?;
149149
// Print a message
150150
println!("{}", token.message);
151151
Ok(())
@@ -157,10 +157,9 @@ pub fn open_dapp(dapps_conf: &dapps::Configuration, rpc_conf: &rpc::HttpConfigur
157157
}
158158

159159
let url = format!("http://{}:{}/{}/", rpc_conf.interface, rpc_conf.port, dapp);
160-
url::open(&url);
160+
url::open(&url).map_err(|e| format!("{}", e))?;
161161
Ok(())
162162
}
163-
164163
// node info fetcher for the local store.
165164
struct FullNodeInfo {
166165
miner: Option<Arc<Miner>>, // TODO: only TXQ needed, just use that after decoupling.

parity/url.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,67 @@
1616

1717
//! Cross-platform open url in default browser
1818
19+
use std;
20+
use std::os::raw::c_int;
21+
22+
#[allow(unused)]
23+
pub enum Error {
24+
ProcessError(std::io::Error),
25+
WindowsShellExecute(c_int),
26+
}
27+
28+
impl From<std::io::Error> for Error {
29+
fn from(err: std::io::Error) -> Self {
30+
Error::ProcessError(err)
31+
}
32+
}
33+
34+
impl std::fmt::Display for Error {
35+
fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
36+
match *self {
37+
Error::ProcessError(ref e) => write!(f, "{}", e),
38+
Error::WindowsShellExecute(e) => write!(f, "WindowsShellExecute error: {}", e),
39+
}
40+
}
41+
}
42+
1943
#[cfg(windows)]
20-
pub fn open(url: &str) {
44+
pub fn open(url: &str) -> Result<(), Error> {
2145
use std::ffi::CString;
2246
use std::ptr;
2347
use winapi::um::shellapi::ShellExecuteA;
2448
use winapi::um::winuser::SW_SHOWNORMAL as Normal;
2549

26-
unsafe {
50+
const WINDOWS_SHELL_EXECUTE_SUCCESS: c_int = 32;
51+
52+
let h_instance = unsafe {
2753
ShellExecuteA(ptr::null_mut(),
2854
CString::new("open").unwrap().as_ptr(),
2955
CString::new(url.to_owned().replace("\n", "%0A")).unwrap().as_ptr(),
3056
ptr::null(),
3157
ptr::null(),
32-
Normal);
58+
Normal) as c_int
59+
};
60+
61+
// https://msdn.microsoft.com/en-us/library/windows/desktop/bb762153(v=vs.85).aspx
62+
// `ShellExecute` returns a value greater than 32 on success
63+
if h_instance > WINDOWS_SHELL_EXECUTE_SUCCESS {
64+
Ok(())
65+
} else {
66+
Err(Error::WindowsShellExecute(h_instance))
3367
}
3468
}
3569

3670
#[cfg(any(target_os="macos", target_os="freebsd"))]
37-
pub fn open(url: &str) {
38-
use std;
39-
let _ = std::process::Command::new("open").arg(url).spawn();
71+
pub fn open(url: &str) -> Result<(), Error> {
72+
let _ = std::process::Command::new("open").arg(url).spawn()?;
73+
Ok(())
4074
}
4175

4276
#[cfg(target_os="linux")]
43-
pub fn open(url: &str) {
44-
use std;
45-
let _ = std::process::Command::new("xdg-open").arg(url).spawn();
77+
pub fn open(url: &str) -> Result<(), Error> {
78+
let _ = std::process::Command::new("xdg-open").arg(url).spawn()?;
79+
Ok(())
4680
}
4781

4882
#[cfg(target_os="android")]

0 commit comments

Comments
 (0)