attestation-service: Refactor errors in attestation module

[kartikjoshi21]

Replace anyhow error crate with thiserror crate
Fixes: #231

[mkulke]

a couple of smaller remarks, but we're getting there

[mkulke]

almost there, I think we need to keep the error messages specific when we refactor from anyhow! or .context(). pasted some examples in the recent comments to show how this can be achieved.

[kartikjoshi21]

almost there, I think we need to keep the error messages specific when we refactor from anyhow! or .context(). pasted some examples in the recent comments to show how this can be achieved.

Updated the PR to include the context. Thanks @mkulke @Xynnn007

[mkulke]

looks good, small nits from my side. @Xynnn007 comments should are left to be addressed, mostly adding {0} to extend the error message, where appropriate.

[mkulke]

lgtm, thanks for accommodating the comments

[fitzthum]

Looks pretty good.

My only question is about how you chose the names for error enums. It seems like you are creating an enum for each file, but this might not be optimal. For instance we end up with the same errors in multiple enums (like the config error). It also means that the names of the enums aren't super meaningful. The RestfulError enum contains a bunch of stuff that isn't realy related to Rest but is just part of that file. ServiceError is a really general term.

I wonder if there is a way to break up the errors into different logical groups, e.g. ConfigError, ProtocolError, CryptoError, and then share these enums between files.

I guess this isn't too big of a deal, though. If @Xynnn007 and @mkulke are happy with the PR then we can go ahead as is.

[mkulke]

Looks pretty good.

My only question is about how you chose the names for error enums. It seems like you are creating an enum for each file, but this might not be optimal. For instance we end up with the same errors in multiple enums (like the config error). It also means that the names of the enums aren't super meaningful. The RestfulError enum contains a bunch of stuff that isn't realy related to Rest but is just part of that file. ServiceError is a really general term.

I wonder if there is a way to break up the errors into different logical groups, e.g. ConfigError, ProtocolError, CryptoError, and then share these enums between files.

ah indeed, in some of those cases a per-file error doesn't make much sense, I agree. e.g. in the case of RestfulError & GrpcError, maybe they could be split into ProtocolError and ,idk, "ServiceError"?

[Xynnn007]

Thanks @kartikjoshi21