Added custom NumbersException class #15
Conversation
There was a problem hiding this comment.
The Messaging SDK has 2 exceptions. One ServiceException for non 2xx responses from the api server, and a ClientException for other stuff. Can you follow this same pattern if it makes sense for the numbers api?
https://github.com/Bandwidth/messaging-java-sdk/blob/master/src/main/java/com/bandwidth/sdk/messaging/exception/MessagingClientException.java
https://github.com/Bandwidth/messaging-java-sdk/blob/master/src/main/java/com/bandwidth/sdk/messaging/exception/MessagingServiceException.java
|
I'll just make the NumbersSerde in this PR and close #17 as well to make the travis scripts happy |
| @Value.Immutable | ||
| @JsonSerialize(as = ImmutableNumbersApiError.class) | ||
| @JsonDeserialize(as = ImmutableNumbersApiError.class) | ||
| public abstract class NumbersApiError { |
There was a problem hiding this comment.
I think the Numbers error's look different than our Messages api. We should probably just remove this (And NumbersApiFieldError)
There was a problem hiding this comment.
I think that's right. Based on the docs, all of the responses look like this
<BillingReportCreationResponse>
<ResponseStatus>
<ErrorCode>15501</ErrorCode>
<Description>Invalid date range. Start date 2014-05-21 must be before end date 2013-05-29</Description>
</ResponseStatus>
</BillingReportCreationResponse>
All have the same ResponseStatus fields, but the outer tag looks different for all of them
| public static void throwIfApiError(Response apiResponse) { | ||
| if (!isSuccessfulHttpStatusCode(apiResponse.getStatusCode())) { | ||
| try { | ||
| throw new NumbersServiceException( |
There was a problem hiding this comment.
The code that was copied here had a bug: https://github.com/Bandwidth/messaging-java-sdk/pull/40/files
|
|
||
| import com.fasterxml.jackson.core.type.TypeReference; | ||
| import com.fasterxml.jackson.dataformat.xml.XmlMapper; | ||
| import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; |
There was a problem hiding this comment.
This import isn't needed.
|
|
||
| import java.util.concurrent.CompletableFuture; | ||
|
|
||
| public class ExceptionUtils { |
There was a problem hiding this comment.
I see this used pattern in Messaging SDK and Number SDK (with only the Exception name being different). Might make sense to pull this out functionality to a shared lib.
|
Already done |
closes #14