[hue] Fix and improve error logging and status descriptions for API v2 (#15512)

* Provide detailed error information on failed commands
* Log as info when command succeeds
* Revert collect(Collectors.toList()) refactoring
* Provide exception message in status description

Fixes #15511

---------

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>

diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/connection/Clip2Bridge.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/connection/Clip2Bridge.java
index 102b5170ed6cb9f1411b8223e8ea47fd74690295..6c2f8d84317fcde39424e1370e03d1875927b895 100644 (file)
--- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/connection/Clip2Bridge.java
+++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/connection/Clip2Bridge.java
@@ -1081,10 +1081,11 @@ public class Clip2Bridge implements Closeable {
      * accessing the session while it is being recreated.
      *
      * @param resource the resource to put.
+     * @return the resource, which may contain errors.
      * @throws ApiException if something fails.
      * @throws InterruptedException
      */
-    public void putResource(Resource resource) throws ApiException, InterruptedException {
+    public Resources putResource(Resource resource) throws ApiException, InterruptedException {
         Stream stream = null;
         try (Throttler throttler = new Throttler(MAX_CONCURRENT_STREAMS);
                 SessionSynchronizer sessionSynchronizer = new SessionSynchronizer(false)) {
@@ -1106,17 +1107,17 @@ public class Clip2Bridge implements Closeable {
             String contentType = contentStreamListener.getContentType();
             int status = contentStreamListener.getStatus();
             LOGGER.trace("HTTP/2 {} (Content-Type: {}) << {}", status, contentType, contentJson);
-            if (status != HttpStatus.OK_200) {
+            if (!HttpStatus.isSuccess(status)) {
                 throw new ApiException(String.format("Unexpected HTTP status '%d'", status));
             }
             if (!MediaType.APPLICATION_JSON.equals(contentType)) {
                 throw new ApiException("Unexpected Content-Type: " + contentType);
             }
+            if (contentJson.isEmpty()) {
+                throw new ApiException("Response payload is empty");
+            }
             try {
-                Resources resources = Objects.requireNonNull(jsonParser.fromJson(contentJson, Resources.class));
-                if (LOGGER.isDebugEnabled()) {
-                    resources.getErrors().forEach(error -> LOGGER.debug("putResource() resources error:{}", error));
-                }
+                return Objects.requireNonNull(jsonParser.fromJson(contentJson, Resources.class));
             } catch (JsonParseException e) {
                 LOGGER.debug("putResource() parsing error json:{}", contentJson, e);
                 throw new ApiException("Parsing error", e);

diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resources.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resources.java
index a20618893ba97f9c50bb24416a757c049e20b312..ce554136df11171d4d9ab0e90dc1df84744d79ac 100644 (file)
--- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resources.java
+++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/dto/clip2/Resources.java
@@ -32,6 +32,10 @@ public class Resources {
         return errors.stream().map(Error::getDescription).collect(Collectors.toList());
     }
 
+    public boolean hasErrors() {
+        return !errors.isEmpty();
+    }
+
     public List<Resource> getResources() {
         return data;
     }

diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java
index d7903bce6b275532ab8ecc016dfa90bdac1d9e2f..aabdf973577c68c331ff8f31e81476b657061d60 100644 (file)
--- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java
+++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2BridgeHandler.java
@@ -161,73 +161,50 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
     private synchronized void checkConnection() {
         logger.debug("checkConnection()");
 
-        // check connection to the hub
-        ThingStatusDetail thingStatus;
+        boolean retryApplicationKey = false;
+        boolean retryConnection = false;
+
         try {
             checkAssetsLoaded();
             getClip2Bridge().testConnectionState();
-            thingStatus = ThingStatusDetail.NONE;
-        } catch (HttpUnauthorizedException e) {
-            logger.debug("checkConnection() {}", e.getMessage(), e);
-            thingStatus = ThingStatusDetail.CONFIGURATION_ERROR;
+            updateSelf(); // go online
+        } catch (HttpUnauthorizedException unauthorizedException) {
+            logger.debug("checkConnection() {}", unauthorizedException.getMessage(), unauthorizedException);
+            if (applKeyRetriesRemaining > 0) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                        "@text/offline.api2.conf-error.press-pairing-button");
+                try {
+                    registerApplicationKey();
+                    retryApplicationKey = true;
+                } catch (HttpUnauthorizedException e) {
+                    retryApplicationKey = true;
+                } catch (ApiException e) {
+                    setStatusOfflineWithCommunicationError(e);
+                } catch (IllegalStateException e) {
+                    updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                            "@text/offline.api2.conf-error.read-only");
+                } catch (AssetNotLoadedException e) {
+                    updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                            "@text/offline.api2.conf-error.assets-not-loaded");
+                } catch (InterruptedException e) {
+                    return;
+                }
+            } else {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                        "@text/offline.api2.conf-error.not-authorized");
+            }
         } catch (ApiException e) {
             logger.debug("checkConnection() {}", e.getMessage(), e);
-            thingStatus = ThingStatusDetail.COMMUNICATION_ERROR;
+            setStatusOfflineWithCommunicationError(e);
+            retryConnection = connectRetriesRemaining > 0;
         } catch (AssetNotLoadedException e) {
             logger.debug("checkConnection() {}", e.getMessage(), e);
-            thingStatus = ThingStatusDetail.HANDLER_INITIALIZING_ERROR;
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                    "@text/offline.api2.conf-error.assets-not-loaded");
         } catch (InterruptedException e) {
             return;
         }
 
-        // update the thing status
-        boolean retryApplicationKey = false;
-        boolean retryConnection = false;
-        switch (thingStatus) {
-            case CONFIGURATION_ERROR:
-                if (applKeyRetriesRemaining > 0) {
-                    updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                            "@text/offline.api2.conf-error.press-pairing-button");
-                    try {
-                        registerApplicationKey();
-                        retryApplicationKey = true;
-                    } catch (HttpUnauthorizedException e) {
-                        retryApplicationKey = true;
-                    } catch (ApiException e) {
-                        updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                                "@text/offline.communication-error");
-                    } catch (IllegalStateException e) {
-                        updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                                "@text/offline.api2.conf-error.read-only");
-                    } catch (AssetNotLoadedException e) {
-                        updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                                "@text/offline.api2.conf-error.assets-not-loaded");
-                    } catch (InterruptedException e) {
-                        return;
-                    }
-                } else {
-                    updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                            "@text/offline.api2.conf-error.not-authorized");
-                }
-                break;
-
-            case COMMUNICATION_ERROR:
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                        "@text/offline.communication-error");
-                retryConnection = connectRetriesRemaining > 0;
-                break;
-
-            case HANDLER_INITIALIZING_ERROR:
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                        "@text/offline.api2.conf-error.assets-not-loaded");
-                break;
-
-            case NONE:
-            default:
-                updateSelf(); // go online
-                break;
-        }
-
         int milliSeconds;
         if (retryApplicationKey) {
             // short delay used during attempts to create or validate an application key
@@ -250,6 +227,18 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
         checkConnectionTask = scheduler.schedule(() -> checkConnection(), milliSeconds, TimeUnit.MILLISECONDS);
     }
 
+    private void setStatusOfflineWithCommunicationError(Exception e) {
+        Throwable cause = e.getCause();
+        String causeMessage = cause == null ? null : cause.getMessage();
+        if (causeMessage == null || causeMessage.isEmpty()) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                    "@text/offline.api2.comm-error.exception [\"" + e.getMessage() + "\"]");
+        } else {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                    "@text/offline.api2.comm-error.exception [\"" + e.getMessage() + " -> " + causeMessage + "\"]");
+        }
+    }
+
     /**
      * If a child thing has been added, and the bridge is online, update the child's data.
      */
@@ -467,8 +456,7 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
                 }
             } catch (IOException e) {
                 logger.trace("initializeAssets() communication error on '{}'", ipAddress, e);
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                        "@text/offline.api2.comm-error.exception [\"" + e.getMessage() + "\"]");
+                setStatusOfflineWithCommunicationError(e);
                 return;
             }
 
@@ -491,8 +479,7 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
                 clip2Bridge = new Clip2Bridge(httpClientFactory, this, ipAddress, applicationKey);
             } catch (ApiException e) {
                 logger.trace("initializeAssets() communication error on '{}'", ipAddress, e);
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                        "@text/offline.api2.comm-error.exception [\"" + e.getMessage() + "\"]");
+                setStatusOfflineWithCommunicationError(e);
                 return;
             }
 
@@ -555,14 +542,15 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
      * Execute an HTTP PUT to send a Resource object to the server.
      *
      * @param resource the resource to put.
+     * @return the resource, which may contain errors.
      * @throws ApiException if a communication error occurred.
      * @throws AssetNotLoadedException if one of the assets is not loaded.
      * @throws InterruptedException
      */
-    public void putResource(Resource resource) throws ApiException, AssetNotLoadedException, InterruptedException {
+    public Resources putResource(Resource resource) throws ApiException, AssetNotLoadedException, InterruptedException {
         logger.debug("putResource() {}", resource);
         checkAssetsLoaded();
-        getClip2Bridge().putResource(resource);
+        return getClip2Bridge().putResource(resource);
     }
 
     /**
@@ -684,8 +672,7 @@ public class Clip2BridgeHandler extends BaseBridgeHandler {
             getClip2Bridge().open();
         } catch (ApiException e) {
             logger.trace("updateSelf() {}", e.getMessage(), e);
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                    "@text/offline.api2.comm-error.exception [\"" + e.getMessage() + "\"]");
+            setStatusOfflineWithCommunicationError(e);
             onConnectionOffline();
         } catch (AssetNotLoadedException e) {
             logger.trace("updateSelf() {}", e.getMessage(), e);

diff --git a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java
index 84586791b069d4a7c236da775505778a8560677c..76d7b4c5c8e85e6bb62cdc01f486e3fa4bfe96c9 100644 (file)
--- a/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java
+++ b/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/handler/Clip2ThingHandler.java
@@ -44,6 +44,7 @@ import org.openhab.binding.hue.internal.dto.clip2.MirekSchema;
 import org.openhab.binding.hue.internal.dto.clip2.ProductData;
 import org.openhab.binding.hue.internal.dto.clip2.Resource;
 import org.openhab.binding.hue.internal.dto.clip2.ResourceReference;
+import org.openhab.binding.hue.internal.dto.clip2.Resources;
 import org.openhab.binding.hue.internal.dto.clip2.enums.ActionType;
 import org.openhab.binding.hue.internal.dto.clip2.enums.EffectType;
 import org.openhab.binding.hue.internal.dto.clip2.enums.RecallAction;
@@ -507,7 +508,11 @@ public class Clip2ThingHandler extends BaseThingHandler {
         logger.debug("{} -> handleCommand() put resource {}", resourceId, putResource);
 
         try {
-            getBridgeHandler().putResource(putResource);
+            Resources resources = getBridgeHandler().putResource(putResource);
+            if (resources.hasErrors()) {
+                logger.info("Command '{}' for thing '{}', channel '{}' succeeded with errors: {}", command,
+                        thing.getUID(), channelUID, String.join("; ", resources.getErrors()));
+            }
         } catch (ApiException | AssetNotLoadedException e) {
             if (logger.isDebugEnabled()) {
                 logger.debug("{} -> handleCommand() error {}", resourceId, e.getMessage(), e);

diff --git a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/i18n/hue.properties b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/i18n/hue.properties
index 1835b97ab78213dff86bb9316a02f91b5ca0eca4..b79fd4ddbd2ac5064b263662ef3f3223374a5456 100644 (file)
--- a/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/i18n/hue.properties
+++ b/bundles/org.openhab.binding.hue/src/main/resources/OH-INF/i18n/hue.properties
@@ -232,7 +232,7 @@ offline.group-removed = Hue Bridge reports group as removed.
 # api v2 offline configuration error descriptions
 
 offline.api2.comm-error.zigbee-connectivity-issue = Zigbee connectivity issue.
-offline.api2.comm-error.exception = An unexpected exception ''{0}'' occurred.
+offline.api2.comm-error.exception = An unexpected exception occurred: {0}
 offline.api2.conf-error.certificate-load = Certificate loading failed. Please check your configuration settings (network address, type of certificate).
 offline.api2.conf-error.assets-not-loaded = Bridge/Thing handler assets not loaded.
 offline.api2.conf-error.press-pairing-button = Not authenticated. Press pairing button on the Hue Bridge or set a valid application key in configuration.