feat: update SdkException and subtypes #166
Conversation
Adds errorCode and transportDetails properties.
pgautier404
force-pushed
the
feat/update-error-types
branch
from
99e4559
to
16af95d
Compare
| public AlreadyExistsException(string message) : base(message, MomentoErrorCode.ALREADY_EXISTS_ERROR) | ||
| { | ||
| } | ||
| public AlreadyExistsException(string message, MomentoErrorTransportDetails transportDetails) : base(message, MomentoErrorCode.ALREADY_EXISTS_ERROR, transportDetails) |
There was a problem hiding this comment.
I know the name MomentoErrorTransportDetails is from my spec doc, but curious how folks feel about it. Open to shortening it if it seems to verbose.
There was a problem hiding this comment.
I'm fine with it. I prefer self documenting to terse.
| CLIENT_RESOURCE_EXHAUSTED, | ||
| FAILED_PRECONDITION_ERROR, | ||
| UNKNOWN_ERROR | ||
| } |
There was a problem hiding this comment.
i know you may have this in a branch already and it's fine for it to be in a PR, but we should see if we can add some comment strings here that make these a little more clear.
There was a problem hiding this comment.
This is indeed on a branch that will be rolling out as a PR as soon as I merge this one.
| public MomentoErrorCode errorCode; | ||
| public MomentoErrorTransportDetails? transportDetails = null; | ||
|
|
||
| protected SdkException(string message, MomentoErrorCode errorCode) : base(message) |
There was a problem hiding this comment.
This is a super nitpick but I feel like it's more typical/idiomatic to have the enum be the first arg in a signature like this one.
There was a problem hiding this comment.
That's fair. Fixed in fd2de77.
| } | ||
| protected SdkException(string message, Exception e) : base(message, e) | ||
| protected SdkException(string message, MomentoErrorCode errorCode, MomentoErrorTransportDetails? transportDetails, Exception e) : base(message, e) |
There was a problem hiding this comment.
since these are all protected and the implementations are simple, I'd vote for just having a single constructor that takes all of the args.
There was a problem hiding this comment.
Also fixed in fd2de77.
Adds errorCode and transportDetails properties.