Skip to content

Commit ff7eccc

Browse files
slivingstonslivingston
committed
refactor to use fewer unwrap()
1 parent 7f99341 commit ff7eccc

1 file changed

Lines changed: 86 additions & 33 deletions

File tree

‎src/client.rs‎

Lines changed: 86 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ impl ClientError {
6363
}
6464
}
6565

66+
fn parse_error_message(payload: serde_json::Value) -> String {
67+
payload["error_message"]
68+
.as_str()
69+
.expect("Error message from api.rerobots.net should be a string")
70+
.to_string()
71+
}
72+
6673
#[cfg(not(test))]
6774
fn get_origin() -> String {
6875
option_env!("REROBOTS_ORIGIN")
@@ -96,7 +103,8 @@ impl TokenClaims {
96103
pub fn new(api_token: &str) -> Result<TokenClaims, &str> {
97104
let alg = PKeyWithDigest {
98105
digest: MessageDigest::sha256(),
99-
key: PKey::public_key_from_pem(PUBLIC_KEY).unwrap(),
106+
key: PKey::public_key_from_pem(PUBLIC_KEY)
107+
.expect("PEM text should be parsed into public key"),
100108
};
101109

102110
let result: Result<Token<Header, Claims, _>, jwt::error::Error> =
@@ -109,10 +117,19 @@ impl TokenClaims {
109117
},
110118
};
111119
let claims = parsed_tok.claims();
112-
let subject = claims.registered.subject.as_ref().unwrap();
120+
let subject = claims
121+
.registered
122+
.subject
123+
.as_ref()
124+
.ok_or("Token has subject claim")?;
113125
let expiration = claims.registered.expiration;
114126
let organization = if claims.private.contains_key("org") {
115-
Some(claims.private["org"].as_str().unwrap().into())
127+
Some(
128+
claims.private["org"]
129+
.as_str()
130+
.expect("org claim is string")
131+
.into(),
132+
)
116133
} else {
117134
None
118135
};
@@ -129,7 +146,10 @@ impl TokenClaims {
129146
match self.expiration {
130147
Some(exp) => {
131148
let now = std::time::SystemTime::now();
132-
let utime = now.duration_since(std::time::UNIX_EPOCH).unwrap().as_secs();
149+
let utime = now
150+
.duration_since(std::time::UNIX_EPOCH)
151+
.expect("Duration since Unix epoch should be computable")
152+
.as_secs();
133153
exp < utime
134154
}
135155
None => false,
@@ -150,7 +170,9 @@ impl std::fmt::Display for TokenClaims {
150170
write!(
151171
f,
152172
"expiration: {}",
153-
Utc.timestamp_opt(exp as i64, 0).unwrap()
173+
Utc.timestamp_opt(exp as i64, 0)
174+
.single()
175+
.expect("Expiration (in seconds) should be parsed as timestamp")
154176
)
155177
}
156178
None => write!(f, "expiration: (never)"),
@@ -161,8 +183,13 @@ impl std::fmt::Display for TokenClaims {
161183
fn create_client(token: Option<String>) -> Result<awc::Client, Box<dyn std::error::Error>> {
162184
let authheader = match token {
163185
Some(tok) => Some(format!("Bearer {tok}")),
164-
None => std::env::var_os("REROBOTS_API_TOKEN")
165-
.map(|tok| format!("Bearer {}", tok.into_string().unwrap())),
186+
None => std::env::var_os("REROBOTS_API_TOKEN").map(|tok| {
187+
format!(
188+
"Bearer {}",
189+
tok.into_string()
190+
.expect("REROBOTS_API_TOKEN variable should be valid string")
191+
)
192+
}),
166193
};
167194

168195
let client = awc::Client::builder();
@@ -202,7 +229,7 @@ pub fn api_search(
202229
Ok(payload)
203230
} else if resp.status() == 400 {
204231
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
205-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
232+
ClientError::newbox(parse_error_message(payload))
206233
} else {
207234
ClientError::newbox(format!("server indicated error: {}", resp.status()))
208235
}
@@ -232,7 +259,7 @@ pub fn api_instances(
232259
Ok(payload)
233260
} else if resp.status() == 400 {
234261
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
235-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
262+
ClientError::newbox(parse_error_message(payload))
236263
} else {
237264
ClientError::newbox(format!("server indicated error: {}", resp.status()))
238265
}
@@ -248,13 +275,18 @@ fn select_instance<S: ToString>(
248275
Some(inid) => Ok(inid.to_string()),
249276
None => {
250277
let payload = api_instances(token, false)?;
251-
let instances = payload["workspace_instances"].as_array().unwrap();
278+
let instances = payload["workspace_instances"]
279+
.as_array()
280+
.ok_or("workspace_instances should be array")?;
252281
if instances.is_empty() {
253282
ClientError::newbox("no active instances")
254283
} else if instances.len() > 1 {
255284
ClientError::newbox("ambiguous command because more than one active instance")
256285
} else {
257-
Ok(instances[0].as_str().unwrap().to_string())
286+
Ok(instances[0]
287+
.as_str()
288+
.ok_or("Elements of workspace_instances should be strings")?
289+
.to_string())
258290
}
259291
}
260292
}
@@ -285,7 +317,7 @@ pub fn api_instance_info<S: ToString>(
285317
ClientError::newbox("instance not found")
286318
} else if resp.status() == 400 {
287319
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
288-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
320+
ClientError::newbox(parse_error_message(payload))
289321
} else {
290322
ClientError::newbox(format!("server indicated error: {}", resp.status()))
291323
}
@@ -312,12 +344,15 @@ pub fn get_instance_sshkey<S: ToString>(
312344
if resp.status() == 200 {
313345
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
314346
debug!("resp to GET {}: {}", path, serde_json::to_string(&payload)?);
315-
Ok(payload["key"].as_str().unwrap().to_string())
347+
Ok(payload["key"]
348+
.as_str()
349+
.ok_or("key should be string")?
350+
.to_string())
316351
} else if resp.status() == 404 {
317352
ClientError::newbox("instance not found")
318353
} else if resp.status() == 400 {
319354
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
320-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
355+
ClientError::newbox(parse_error_message(payload))
321356
} else {
322357
ClientError::newbox(format!("server indicated error: {}", resp.status()))
323358
}
@@ -357,14 +392,12 @@ pub fn api_wdeployment_info<S: std::fmt::Display>(
357392
);
358393
payload
359394
.as_object_mut()
360-
.unwrap()
395+
.expect("Response is JSON object")
361396
.insert("cap".to_string(), rules_payload);
362397
} else if resp.status() == 400 {
363398
let payload: serde_json::Value =
364399
serde_json::from_slice(resp.body().await?.as_ref())?;
365-
return ClientError::newbox(String::from(
366-
payload["error_message"].as_str().unwrap(),
367-
));
400+
return ClientError::newbox(parse_error_message(payload));
368401
} else {
369402
return ClientError::newbox(format!(
370403
"server indicated error: {}",
@@ -378,7 +411,7 @@ pub fn api_wdeployment_info<S: std::fmt::Display>(
378411
ClientError::newbox("workspace deployment not found")
379412
} else if resp.status() == 400 {
380413
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
381-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
414+
ClientError::newbox(parse_error_message(payload))
382415
} else {
383416
ClientError::newbox(format!("server indicated error: {}", resp.status()))
384417
}
@@ -408,7 +441,7 @@ pub fn api_terminate_instance(
408441
ClientError::newbox("instance not found")
409442
} else if resp.status() == 400 {
410443
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
411-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
444+
ClientError::newbox(parse_error_message(payload))
412445
} else {
413446
ClientError::newbox(format!("server indicated error: {}", resp.status()))
414447
}
@@ -448,7 +481,7 @@ pub fn api_launch_instance(
448481
Ok(payload)
449482
} else if resp.status() == 400 {
450483
let payload: serde_json::Value = serde_json::from_slice(resp.body().await?.as_ref())?;
451-
ClientError::newbox(String::from(payload["error_message"].as_str().unwrap()))
484+
ClientError::newbox(parse_error_message(payload))
452485
} else {
453486
ClientError::newbox(format!("server indicated error: {}", resp.status()))
454487
}
@@ -462,8 +495,10 @@ mod tests {
462495
use super::api_search;
463496
use super::TokenClaims;
464497

498+
type TestResult = Result<(), Box<dyn std::error::Error>>;
499+
465500
#[test]
466-
fn empty_search() {
501+
fn empty_search() -> TestResult {
467502
let _m = mock("GET", "/deployments?info=t")
468503
.with_status(200)
469504
.with_header("content-type", "application/json")
@@ -476,13 +511,16 @@ mod tests {
476511
)
477512
.create();
478513

479-
let res = api_search(None, None, None).unwrap();
480-
let wds = res["workspace_deployments"].as_array().unwrap();
481-
assert_eq!(wds.len(), 0)
514+
let res = api_search(None, None, None)?;
515+
let wds = res["workspace_deployments"]
516+
.as_array()
517+
.ok_or("workspace_deployments should be array")?;
518+
assert_eq!(wds.len(), 0);
519+
Ok(())
482520
}
483521

484522
#[test]
485-
fn search_with_1() {
523+
fn search_with_1() -> TestResult {
486524
let _m = mock("GET", "/deployments?info=t")
487525
.with_status(200)
488526
.with_header("content-type", "application/json")
@@ -512,27 +550,42 @@ mod tests {
512550
)
513551
.create();
514552

515-
let res = api_search(None, None, None).unwrap();
516-
let wds = res["workspace_deployments"].as_array().unwrap();
553+
let res = api_search(None, None, None)?;
554+
let wds = res["workspace_deployments"]
555+
.as_array()
556+
.ok_or("workspace_deployments should be array")?;
517557
assert_eq!(wds.len(), 1);
518558
assert_eq!(wds[0], "82051afa-b331-4b82-8bd4-9eea9ad78241");
519-
let info = res["info"][wds[0].as_str().unwrap()].as_object().unwrap();
559+
let info = res["info"][wds[0]
560+
.as_str()
561+
.ok_or("Elements of workspace_deployments should be strings")?]
562+
.as_object()
563+
.ok_or("info should be JSON object")?;
520564
assert_eq!(info["type"], "fixed_misty2");
565+
566+
Ok(())
521567
}
522568

523569
#[test]
524-
fn detect_expired_token() {
570+
fn detect_expired_token() -> TestResult {
525571
const EXPIRED_TOKEN: &str = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ0ZXN0X3VzZXIiLCJpc3MiOiJyZXJvYm90cy5uZXQiLCJhdWQiOiJyZXJvYm90cy5uZXQiLCJleHAiOjE2Nzk2ODQ0MjQsIm5iZiI6MTY3OTU5ODAyNH0.R9Z4Y5tVHJiPTGfEjUljIGmzor4SAmUmdXyuvQBF2oc6QVHFfxGD-QnVaDfyB6Q2WbBiMWsvDgsI2X56t_-Cd7tZio-VQLL-AEwu1uTnOnt3aXwYB211M7b5ZEFrNN5BNS00FqnMpOQ5DfWKzYUqzvVV4gbxdPhLD2PUpMvT6-F-9NgtR_Z5VEeR-rzRI1-A0KYP9tWHh8GeEPb4D449wtcp-a-Hy6GHOFGGJupSkiB1aJ0T-b1CPlEDN9uwgEq4N_2hHMXwYiyc5Qpo5PABAB_BhM0CP43ca2M9n67om9oQZHqnkon_RWJDSjNAGCn8aZGSfKv0E2pahXfqjhWn6Eakb_R4VDNFBIy6xAl1i0NT-YDdF8-4kLA0sxIoLnk812LvmoifsFsmuv1cdSAAccYdyMjwyQDNCMjFCSJSR6pwZhpfsaUB1frTXWaFteA8yGf8bkL59i3yWherji7yfRY-aepVSH2Hjw_bHxVIPq3mulrhW0XI8qk6uPS1CB5F4Thqqvf_G-qIx0HJWAzF6zTkoAjhOa-BUIuxGo2w17fxK2RhzoOfMZWSfQqifKdCxhuGNOTpyD7nsK9OQP9_S1krOLSvavWuPfTV5GN-NhmRSAcg8Qcv1UkGguZaAFlHlGOzlw4PI_9qGhIxPj7t-PjHyETdH4IrdilpQkXgZuw";
526-
let tc = TokenClaims::new(EXPIRED_TOKEN).unwrap();
572+
let tc = TokenClaims::new(EXPIRED_TOKEN)?;
527573
assert!(tc.is_expired());
528574
assert_eq!(tc.subject, "test_user");
529575
assert_eq!(tc.organization, None);
530-
assert_eq!(tc.expiration.unwrap(), 1679684424);
576+
assert_eq!(tc.expiration, Some(1679684424));
531577

532578
let mut tok = String::from(EXPIRED_TOKEN);
533579
tok.push('F');
534-
let error = TokenClaims::new(&tok).unwrap_err();
580+
let error = match TokenClaims::new(&tok) {
581+
Ok(_) => {
582+
panic!("Error should be detected when parsing broken token text")
583+
}
584+
Err(e) => e,
585+
};
535586
assert_eq!(error, "not a valid signature");
587+
588+
Ok(())
536589
}
537590

538591
#[test]

0 commit comments

Comments
 (0)