Get rid of unwrap #246
Get rid of unwrap #246
Conversation
Filip-L
temporarily deployed
to
production-fidl
GitHub Actions
Inactive
| id: get_string_field(&app, "id").expect("ID must exist"), | ||
| owner: get_string_field(&app, "owner").expect("Owner must exist"), | ||
| repo: get_string_field(&app, "repo").expect("Repo must exist"), | ||
| pr_number: get_i64_field(&app, "pr_number").expect("PR number must exist"), | ||
| issue_number: get_i64_field(&app, "issue_number").expect("Issue number must exist"), |
There was a problem hiding this comment.
Is there any way to trigger a scenario where any of these would fail?
| } | ||
| let applications = app_data | ||
| .into_iter() | ||
| .map(|app| ApplicationModel { |
There was a problem hiding this comment.
Long-term (not this PR) it would be better to replace this with https://docs.rs/serde_json/latest/serde_json/fn.from_value.html
| @@ -38,8 +38,10 @@ pub async fn allocators() -> impl Responder { | |||
| pub async fn create_allocator_from_json(files: web::Json<ChangedAllocators>) -> impl Responder { | |||
| let ChangedAllocators { files_changed } = files.into_inner(); | |||
| match create_allocator_from_file(files_changed).await { | |||
| Ok(()) => HttpResponse::Ok() | |||
| .body(serde_json::to_string_pretty("All files processed successfully").unwrap()), | |||
| Ok(_) => match serde_json::to_string_pretty("All files processed successfully") { | |||
There was a problem hiding this comment.
to_string_pretty won't fail if passed a static string. It's a good candidate for .expect.
| Ok(app_file) => HttpResponse::Ok().body(serde_json::to_string_pretty(&app_file).unwrap()), | ||
| Ok(app_file) => match serde_json::to_string_pretty(&app_file) { | ||
| Ok(response) => HttpResponse::Ok().body(response), | ||
| Err(_) => HttpResponse::InternalServerError().body("Failed to parse success message"), |
There was a problem hiding this comment.
| Err(_) => HttpResponse::InternalServerError().body("Failed to parse success message"), | |
| Err(_) => HttpResponse::InternalServerError().body("Failed to serialize success message"), |
There was a problem hiding this comment.
Applies to all cases where it says parse instead of serialize when refering to to_string_pretty.
| @@ -170,7 +182,10 @@ pub async fn propose_storage_providers( | |||
| ) | |||
| .await | |||
| { | |||
| Ok(_) => HttpResponse::Ok().body(serde_json::to_string_pretty("Success").unwrap()), | |||
| Ok(_) => match serde_json::to_string_pretty("Success") { | |||
There was a problem hiding this comment.
to_string_pretty won't fail if passed a static string. It's a good candidate for .expect.
| @@ -523,8 +560,10 @@ pub async fn submit_kyc(info: web::Json<SubmitKYCInfo>) -> impl Responder { | |||
| Err(e) => return HttpResponse::BadRequest().body(e.to_string()), | |||
| }; | |||
| match ldn_application.submit_kyc(&info.into_inner()).await { | |||
| Ok(()) => HttpResponse::Ok() | |||
| .body(serde_json::to_string_pretty("Address verified with score").unwrap()), | |||
| Ok(_) => match serde_json::to_string_pretty("Address verified with score") { | |||
There was a problem hiding this comment.
to_string_pretty won't fail if passed a static string. It's a good candidate for .expect.
| @@ -546,7 +585,10 @@ pub async fn request_kyc(query: web::Query<VerifierActionsQueryParams>) -> impl | |||
| .request_kyc(&query.id, &query.owner, &query.repo) | |||
| .await | |||
| { | |||
| Ok(()) => HttpResponse::Ok().body(serde_json::to_string_pretty("Success").unwrap()), | |||
| Ok(_) => match serde_json::to_string_pretty("Success") { | |||
There was a problem hiding this comment.
to_string_pretty won't fail if passed a static string. It's a good candidate for .expect (won't repeat for any others, please review all these cases).
| e | ||
| )) | ||
| })?; | ||
| if let Some(allocation_amounts) = allocation_amount.quantity_options { |
There was a problem hiding this comment.
What if allocation_amount.quantity_options is None? Is it acceptable to ignore or should we return an error? Previously it would panic, which suggests we should return an error.
| let body = hyper::body::to_bytes(response).await.unwrap(); | ||
| let body = hyper::body::to_bytes(response) | ||
| .await | ||
| .map_err(|e| LDNError::Load(format!("Failed to parse to bytes: {}", e)))?; |
There was a problem hiding this comment.
| .map_err(|e| LDNError::Load(format!("Failed to parse to bytes: {}", e)))?; | |
| .map_err(|e| LDNError::Load(format!("Failed to serialize to bytes: {}", e)))?; |
| let last_commit: &CommitData = commits.last().unwrap(); | ||
| let body = hyper::body::to_bytes(response_body) | ||
| .await | ||
| .map_err(|e| LDNError::Load(format!("Failed to parse to bytes: {}", e)))?; |
There was a problem hiding this comment.
| .map_err(|e| LDNError::Load(format!("Failed to parse to bytes: {}", e)))?; | |
| .map_err(|e| LDNError::Load(format!("Failed to serialize to bytes: {}", e)))?; |
No description provided.