-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
break(iot-dev): Rework thrown exceptions, add new device reconnection sample #1502
Conversation
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/FileUpload.java
Outdated
Show resolved
Hide resolved
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/ModuleClient.java
Outdated
Show resolved
Hide resolved
@@ -13,7 +14,7 @@ | |||
OK, | |||
BAD_FORMAT, | |||
UNAUTHORIZED, | |||
TOO_MANY_DEVICES, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrectly labeled when this enum was first created to represent 403 errors
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...connection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceClientManager.java
Outdated
Show resolved
Hide resolved
...nection-sample/src/main/java/samples/com/microsoft/azure/sdk/iot/DeviceConnectionSample.java
Outdated
Show resolved
Hide resolved
...lient/src/main/java/com/microsoft/azure/sdk/iot/device/exceptions/IotHubClientException.java
Outdated
Show resolved
Hide resolved
...lient/src/main/java/com/microsoft/azure/sdk/iot/device/exceptions/IotHubClientException.java
Outdated
Show resolved
Hide resolved
...lient/src/main/java/com/microsoft/azure/sdk/iot/device/exceptions/IotHubClientException.java
Outdated
Show resolved
Hide resolved
14e7370
to
008fb2a
Compare
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/FileUpload.java
Show resolved
Hide resolved
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/InternalClient.java
Show resolved
Hide resolved
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/IotHubStatusCode.java
Outdated
Show resolved
Hide resolved
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/IotHubStatusCode.java
Outdated
Show resolved
Hide resolved
device/iot-device-client/src/main/java/com/microsoft/azure/sdk/iot/device/IotHubStatusCode.java
Show resolved
Hide resolved
…/iot/device/IotHubStatusCode.java Co-authored-by: David R. Williamson <drwill@microsoft.com>
…ot-sdk-java into timtay/exceptions2
...ce-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/TransportException.java
Outdated
Show resolved
Hide resolved
...ce-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/TransportException.java
Outdated
Show resolved
Hide resolved
...ce-client/src/main/java/com/microsoft/azure/sdk/iot/device/transport/TransportException.java
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
Using admin privileges to squash and merge this without horton passing since I need to make changes to horton like we've done a few times now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
All of DeviceClient, ModuleClient and MultiplexingClient will now share an exception class
We'll still expose the IotHubStatusCode so users can catch for each specific error scenario that they are worried about and have a more general catch for scenarios they are less worried about. For instance:
In general, here is the before and after for non-open/close operations:
I want to stop returning IotHubStatusCodes to the user and expecting them to write logic to interpret them, so instead of returning the status code in the sync methods, we'll throw the IotHubClientException derived from the status code instead. Similarly, we'll include that exception in the async callback alongside the status code so that users can ignore the status code and just check the exception.
As a part of this PR, I've included my take on the new "device reconnection sample" that demonstrates best practices for keeping the connection alive and how to retry on each particular operation. It leans heavily on the proposed exception structure. It may be easiest to review the exception structure changes by reviewing this sample
I also renamed a few callback interfaces to keep the
subscribeToXYZ()
methods consistent with each otherI also added the sent message as a parameter in the callback for when a message was acknowledged by the service
Once this code is signed off, I plan on deleting the current device reconnection sample and replacing it with the new sample I wrote. That way, any user that already had a link to our device reconnection sample will still have a link to our recommended patterns.