refactor: return None when entity is not found
#632
Conversation
| pub async fn get_account_header_by_id( | ||
| &self, | ||
| account_id: AccountId, | ||
| ) -> Result<(AccountHeader, AccountStatus), ClientError> { | ||
| ) -> Result<Option<(AccountHeader, AccountStatus)>, ClientError> { |
There was a problem hiding this comment.
If AccountDataNotFound was the only error we should then return Option<(AccountHeader, AccountStatus)> instead of wrapping result. Though it would require a change in the Store trait.
This is not the only place with the same pattern (e.g. get_account_auth). Should we document some error that each method might return? Or in case that we don't have the error case anymore, should we remove the wrapping Result?
There was a problem hiding this comment.
In terms of defined errors I don't think that there are any, but a StoreError::DatabaseError might be possible if there is an internal database error
| client | ||
| .get_account_header_by_id(account_id) | ||
| .await? | ||
| .ok_or(format!("Account with ID {account_id} not found"))?; |
There was a problem hiding this comment.
nit: This appears enough times that it might make sense to do a util function that returns this in case we want to change it in the future
| .store | ||
| .get_account(*account_id) | ||
| .await? | ||
| .ok_or(StoreError::AccountDataNotFound(*account_id))? |
There was a problem hiding this comment.
Should this be a NoteScreenerError instead? Basically if we say that accounts not being found is not a store problem, then I think errors related to it should be in higher-level modules
closes #510