[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>
This commit is contained in:
Jacob Laursen 2023-09-09 12:05:38 +02:00 committed by Kai Kreuzer
parent 4be53bcf6f
commit 9f753e879a
5 changed files with 69 additions and 72 deletions

View File

@ -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);

View File

@ -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;
}

View File

@ -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);

View File

@ -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);

View File

@ -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.