[Issue Report] Remove all .unwrap()s
This project contains A LOT of unwraps, which can be easily avoided and removed, making the code much rustier. I'd like to do that after #423 gets merged.
To handle all possible errors, we would need a proper Result type; Result<T, Box<dyn Error>> won't do. We could use anyhow, but I'd suggest color_eyre because of the fancier look when an error happens. Let me know what you think
Here's a list of all unwraps or expects I found using ripgrep:
tests\gzip.rs
10: let headers = request.headers_mut().unwrap();
15: http::HeaderValue::from_str(accept_encoding).unwrap(),
21: .request(request.body(Body::empty()).unwrap())
23: .unwrap()
tests\cors.rs
9: client.get(url.parse().unwrap()).await.unwrap()
benches\file_explorer.rs
13: HTTP_CLIENT.get(uri.parse().unwrap()).await.unwrap();
17: let rt = Runtime::new().unwrap();
25: let rt = Runtime::new().unwrap();
34: let rt = Runtime::new().unwrap();
tests\basic_auth.rs
10: client.get(url.parse().unwrap()).await.unwrap()
18: let headers = request.headers_mut().unwrap();
22: HeaderValue::from_str(credentials.as_http_header().as_str()).unwrap(),
27: .request(request.body(Body::empty()).unwrap())
29: .unwrap()
src\utils\url_encode.rs
16: let component = component.to_str().unwrap();
46: let file_path = PathBuf::from_str(file_path).unwrap();
59: let file_path = file_path.to_str().unwrap();
src\cli.rs
81: host: "127.0.0.1".parse().unwrap(),
85: root_dir: PathBuf::from_str("./").unwrap(),
88: tls_cert: PathBuf::from_str("cert.pem").unwrap(),
89: tls_key: PathBuf::from_str("key.rsa").unwrap(),
119: host: "0.0.0.0".parse().unwrap(),
136: host: "192.168.0.1".parse().unwrap(),
148: root_dir: PathBuf::from_str("~/User/sources/http-server").unwrap(),
235: tls_cert: PathBuf::from_str("~/secrets/cert").unwrap(),
236: tls_key: PathBuf::from_str("~/secrets/key").unwrap(),
tests\defacto.rs
9: client.get(url.parse().unwrap()).await.unwrap()
src\addon\proxy.rs
25: let upstream = Uri::from_str(upstream).unwrap();
41: HeaderValue::from_str(self.upstream.authority().unwrap().as_str()).unwrap(),
46: tokio::spawn(async move { client.request(outogoing).await.unwrap() })
48: .unwrap()
66: let via_header = HeaderValue::from_str(&via_header_str).unwrap();
69: let current_via_header = current_via_header.to_str().unwrap();
84: HeaderValue::from_str(proxies_list.as_str()).unwrap(),
139: .unwrap();
152: HeaderValue::from_str(self.upstream.authority().unwrap().as_str()).unwrap(),
156: headers.remove(USER_AGENT).unwrap();
163: let scheme = self.upstream.scheme_str().unwrap();
164: let authority = self.upstream.authority().unwrap().as_str();
176: .unwrap()
206: let via_header_value = headers.get(&HeaderName::from_static("via")).unwrap();
207: let via_header_value = via_header_value.to_str().unwrap();
220: HeaderValue::from_str("HTTP/1.1 GoodProxy").unwrap(),
232: let via_header_value = headers.get(&HeaderName::from_static("via")).unwrap();
233: let via_header_value = via_header_value.to_str().unwrap();
247: (CONNECTION, HeaderValue::from_str("keep-alive").unwrap()),
src\addon\cors.rs
98: HeaderValue::from_str("true").unwrap(),
107: HeaderValue::from_str(allow_headers.as_str()).unwrap(),
116: HeaderValue::from_str(allow_methods.as_str()).unwrap(),
123: HeaderValue::from_str(allow_origin.as_str()).unwrap(),
132: HeaderValue::from_str(expose_headers.as_str()).unwrap(),
139: HeaderValue::from_str(max_age.to_string().as_str()).unwrap(),
148: HeaderValue::from_str(request_headers.as_str()).unwrap(),
155: HeaderValue::from_str(request_method.as_str()).unwrap(),
375: assert_eq!(cors, Cors::try_from(config).unwrap());
src\addon\compression\gzip.rs
93: let mut buffer_cursor = aggregate(body).await.unwrap();
106: HeaderValue::from_str("gzip").unwrap(),
140: HeaderValue::from_str("gzip, deflate").unwrap(),
151: let response = response_builder.body(response_body).unwrap();
160: let compressed = compress(raw).unwrap();
175: .unwrap();
183: .unwrap(),
198: let mut buffer_cursor = aggregate(body).await.unwrap();
207: .unwrap();
213: let mut buffer_cursor = aggregate(compressed_body).await.unwrap();
src\utils\error.rs
19: .unwrap(),
21: .unwrap()
src\addon\file_server\query_params.rs
71: let have = QueryParams::from_str(uri_string).unwrap();
src\config\util\tls.rs
35: path.to_str().unwrap()
38: let cert_bytes = &rustls_pemfile::certs(&mut buf_reader).unwrap()[0];
45: .with_context(|| format!("Unable to find the TLS keys on: {}", path.to_str().unwrap()))?;
49: let path = path.to_str().unwrap();
54: let path = path.to_str().unwrap();
src\config\mod.rs
47: let root_dir = current_dir().unwrap();
73: let root_dir = if cli_arguments.root_dir.to_str().unwrap() == "./" {
74: current_dir().unwrap()
76: let root_dir = cli_arguments.root_dir.to_str().unwrap();
112: cli_arguments.username.unwrap(),
113: cli_arguments.password.unwrap(),
126: let proxy_url = cli_arguments.proxy.unwrap();
src\server\mod.rs
48: let https_config = config.tls.clone().unwrap();
73: server_task.await.unwrap();
92: if self.config.address.ip() == Ipv4Addr::from_str("0.0.0.0").unwrap() {
122: let server = https_server_builder.make_server(address).await.unwrap();
137: if self.config.address.ip() == Ipv4Addr::from_str("0.0.0.0").unwrap() {
src\config\file.rs
57: let path = PathBuf::from_str(value).unwrap();
58: let canon = fs::canonicalize(path).unwrap();
88: let config = ConfigFile::parse_toml(file_contents).unwrap();
89: let mut root_dir = std::env::current_dir().unwrap();
112: ConfigFile::parse_toml(file_contents).unwrap();
129: let root_dir = Some(std::env::current_dir().unwrap());
131: cert: PathBuf::from_str("cert_123.pem").unwrap(),
132: key: PathBuf::from_str("key_123.pem").unwrap(),
135: let config = ConfigFile::parse_toml(file_contents).unwrap();
140: assert_eq!(config.tls.unwrap(), tls);
157: let root_dir = Some(std::env::current_dir().unwrap());
159: cert: PathBuf::from_str("cert_123.pem").unwrap(),
160: key: PathBuf::from_str("key_123.pem").unwrap(),
163: let config = ConfigFile::parse_toml(file_contents).unwrap();
168: assert_eq!(config.tls.unwrap(), tls);
205: let config = ConfigFile::parse_toml(file_contents).unwrap();
206: let root_dir = Some(std::env::current_dir().unwrap());
211: assert_eq!(config.cors.unwrap(), cors);
232: let root_dir = Some(std::env::current_dir().unwrap());
253: let config = ConfigFile::parse_toml(file_contents).unwrap();
258: assert_eq!(config.cors.unwrap(), cors);
270: let config = ConfigFile::parse_toml(file_contents).unwrap();
272: assert!(config.compression.unwrap().gzip);
285: let config = ConfigFile::parse_toml(file_contents).unwrap();
289: let basic_auth = config.basic_auth.unwrap();
302: let config = ConfigFile::parse_toml(file_contents).unwrap();
316: let config = ConfigFile::parse_toml(file_contents).unwrap();
320: let proxy = config.proxy.unwrap();
332: let config = ConfigFile::parse_toml(file_contents).unwrap();
335: assert!(config.graceful_shutdown.unwrap());
src\addon\file_server\scoped_file_system.rs
153: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
167: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
175: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
183: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
196: normalized.to_str().unwrap(),
203: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
204: let resolved_entry = sfs.resolve(PathBuf::from("assets/logo.svg")).await.unwrap();
215: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
216: let resolved_entry = sfs.resolve(PathBuf::from("assets/")).await.unwrap();
223: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
224: let resolved_entry = sfs.resolve(PathBuf::from("assets")).await.unwrap();
231: let sfs = ScopedFileSystem::new(PathBuf::from("")).unwrap();
src\server\handler\mod.rs
49: let middleware = Middleware::try_from(config).unwrap();
60: let middleware = Middleware::try_from(config).unwrap();
src\addon\file_server\mod.rs
45: let scoped_file_system = ScopedFileSystem::new(config.root_dir.clone()).unwrap();
59: let template = std::str::from_utf8(template).unwrap();
63: .unwrap();
224: .unwrap();
254: .unwrap()
377: let path = entry_path.strip_prefix(root_dir).unwrap();
393: let root_dir = PathBuf::from_str("/Users/bob/sources/http-server").unwrap();
396: .unwrap();
421: let sanitized_path = FileServer::parse_path(req_uri).unwrap().0;
422: let wanted_path = PathBuf::from_str(want[idx]).unwrap();
430: let root_dir = PathBuf::from_str("/Users/bob/sources/http-server").unwrap();
433: .unwrap();
434: let breadcrumbs = FileServer::breadcrumbs_from_path(&root_dir, &entry_path).unwrap();
src\server\handler\file_server.rs
32: let response = self.file_server.resolve(req_path).await.unwrap();
src\server\middleware\cors.rs
27: let cors = Cors::try_from(cors_config).unwrap();
Totally agree! Go for it!
I think having unwraps in tests is fine at least for the needs on this project because in tests we are validating specific scenarios.
If test runs on panic due to unwrap, its a good signal of either:
- Invalid assumption
- Invalid state
Both determines failure in any case so I would keep unwraps in tests for simplicity sake.
I cant say the same with end user code/source. No unwraps/cases of panic should exist there.
@EstebanBorai Please review #423 first to avoid collisions
I think having unwraps in tests is fine at least for the needs on this project because in tests we are validating specific scenarios.
I agree
New codebase which still a WIP, we can reopen when v1 is released.