Expect removed from various places #197
Conversation
| .bucket(bucket_name) | ||
| .send() | ||
| .await | ||
| .context(format!("Failed to create bucket: {} in us-east-1", bucket_name))?; |
There was a problem hiding this comment.
Can we try to change this
if self.bucket_location_constraint.as_str() == "us-east-1" {
and compare the enums provided by AWS.
Accordingly change the context to not hardcode us-east-1
Note: I tried doing this in a recent PR but the code was failing after
maybe you can try and this time it might work ?
WDYT @Mohiiit ?
| @@ -231,7 +231,8 @@ fn parse_worker_message(message: &Delivery) -> Result<Option<WorkerTriggerMessag | |||
| .borrow_payload() | |||
| .ok_or_else(|| ConsumptionError::Other(OtherError::from("Empty payload".to_string())))?; | |||
| let message_string = String::from_utf8_lossy(payload).to_string().trim_matches('\"').to_string(); | |||
| let trigger_type = WorkerTriggerType::from_str(message_string.as_str()).expect("trigger type unwrapping failed"); | |||
| let trigger_type = WorkerTriggerType::from_str(message_string.as_str()) | |||
| .map_err(|e| ConsumptionError::Other(OtherError::from(e)))?; | |||
There was a problem hiding this comment.
Let's add a wrap_err here as well,
seems like a good practice to follow
WDYT @Mohiiit ?
| @@ -85,8 +85,7 @@ impl AtlanticClient { | |||
| ) | |||
| .send() | |||
| .await | |||
| .map_err(AtlanticError::AddJobFailure) | |||
| .expect("Failed to add job"); | |||
| .map_err(AtlanticError::AddJobFailure)?; | |||
There was a problem hiding this comment.
Same, can add wrap_err here
WDYT on adding wrap_err throughout the code @Mohiiit ?
There was a problem hiding this comment.
I don't think it makes sense here, because wrap_err is used to provide extra context to the error, in here the only context is that the add job failure happened which is already handled by the AtlanticError here
but I will research about the best practices here and will come back here
Removed expect and added wrap for the error