From 2d4793a8299bb7bfae79e60fa9700a7592063a76 Mon Sep 17 00:00:00 2001 From: jimtng <2554958+jimtng@users.noreply.github.com> Date: Fri, 23 Jan 2026 02:16:20 +1000 Subject: [PATCH] [YAML Thing Provider] Log warning on Thing config errors (#5282) Catch IAE during Thing configuration loading to prevent the entire file from failing. Errors are now logged as warnings instead of throwing a full stack trace. Signed-off-by: Jimmy Tanagra --- .../yaml/internal/things/YamlChannelDTO.java | 9 +++ .../yaml/internal/things/YamlThingDTO.java | 10 +++ .../internal/things/YamlThingProvider.java | 77 ++++++++++--------- 3 files changed, 60 insertions(+), 36 deletions(-) diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlChannelDTO.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlChannelDTO.java index ebe7423e7..4eaa65586 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlChannelDTO.java +++ b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlChannelDTO.java @@ -18,6 +18,7 @@ import java.util.Objects; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.config.core.Configuration; import org.openhab.core.model.yaml.internal.util.YamlElementUtils; import org.openhab.core.thing.type.ChannelKind; import org.openhab.core.thing.type.ChannelTypeUID; @@ -42,6 +43,14 @@ public class YamlChannelDTO { public boolean isValid(@NonNull List<@NonNull String> errors, @NonNull List<@NonNull String> warnings) { boolean ok = true; + if (config != null) { + try { + new Configuration(config); + } catch (IllegalArgumentException e) { + errors.add("invalid data in \"config\" field: %s".formatted(e.getMessage())); + ok = false; + } + } if (type != null) { try { new ChannelTypeUID("dummy", type); diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingDTO.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingDTO.java index 3810e7fa3..530879271 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingDTO.java +++ b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingDTO.java @@ -19,6 +19,7 @@ import java.util.Objects; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; +import org.openhab.core.config.core.Configuration; import org.openhab.core.model.yaml.YamlElement; import org.openhab.core.model.yaml.YamlElementName; import org.openhab.core.model.yaml.internal.util.YamlElementUtils; @@ -92,6 +93,15 @@ public class YamlThingDTO implements YamlElement, Cloneable { ok = false; } } + if (config != null) { + try { + new Configuration(config); + } catch (IllegalArgumentException e) { + addToList(errors, + "invalid thing \"%s\": invalid data in \"config\" field: %s".formatted(uid, e.getMessage())); + ok = false; + } + } if (channels != null) { for (Map.Entry<@NonNull String, @NonNull YamlChannelDTO> entry : channels.entrySet()) { String channelId = entry.getKey(); diff --git a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java index 53edc74a9..459bc7b00 100644 --- a/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java +++ b/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java @@ -350,49 +350,54 @@ public class YamlThingProvider extends AbstractProvider } private @Nullable Thing mapThing(YamlThingDTO thingDto) { - ThingUID thingUID = new ThingUID(thingDto.uid); - String[] segments = thingUID.getAsString().split(AbstractUID.SEPARATOR); - ThingTypeUID thingTypeUID = new ThingTypeUID(thingUID.getBindingId(), segments[1]); + try { + ThingUID thingUID = new ThingUID(thingDto.uid); + String[] segments = thingUID.getAsString().split(AbstractUID.SEPARATOR); + ThingTypeUID thingTypeUID = new ThingTypeUID(thingUID.getBindingId(), segments[1]); - ThingType thingType = thingTypeRegistry.getThingType(thingTypeUID, localeProvider.getLocale()); - ThingUID bridgeUID = thingDto.bridge != null ? new ThingUID(thingDto.bridge) : null; - Configuration configuration = new Configuration(thingDto.config); + ThingType thingType = thingTypeRegistry.getThingType(thingTypeUID, localeProvider.getLocale()); + ThingUID bridgeUID = thingDto.bridge != null ? new ThingUID(thingDto.bridge) : null; + Configuration configuration = new Configuration(thingDto.config); - ThingBuilder thingBuilder = thingDto.isBridge() ? BridgeBuilder.create(thingTypeUID, thingUID) - : ThingBuilder.create(thingTypeUID, thingUID); - thingBuilder - .withLabel(thingDto.label != null ? thingDto.label : (thingType != null ? thingType.getLabel() : null)); - thingBuilder.withLocation(thingDto.location); - thingBuilder.withBridge(bridgeUID); - thingBuilder.withConfiguration(configuration); + ThingBuilder thingBuilder = thingDto.isBridge() ? BridgeBuilder.create(thingTypeUID, thingUID) + : ThingBuilder.create(thingTypeUID, thingUID); + thingBuilder.withLabel( + thingDto.label != null ? thingDto.label : (thingType != null ? thingType.getLabel() : null)); + thingBuilder.withLocation(thingDto.location); + thingBuilder.withBridge(bridgeUID); + thingBuilder.withConfiguration(configuration); - List channels = createChannels(thingTypeUID, thingUID, - thingDto.channels != null ? thingDto.channels : Map.of(), - thingType != null ? thingType.getChannelDefinitions() : List.of()); - thingBuilder.withChannels(channels); + List channels = createChannels(thingTypeUID, thingUID, + thingDto.channels != null ? thingDto.channels : Map.of(), + thingType != null ? thingType.getChannelDefinitions() : List.of()); + thingBuilder.withChannels(channels); - Thing thing = thingBuilder.build(); + Thing thing = thingBuilder.build(); - Thing thingFromHandler = null; - ThingHandlerFactory handlerFactory = thingHandlerFactories.stream() - .filter(thf -> isThingHandlerFactoryReady(thf) && thf.supportsThingType(thingTypeUID)).findFirst() - .orElse(null); - if (handlerFactory != null) { - thingFromHandler = handlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID); - if (thingFromHandler != null) { - mergeThing(thingFromHandler, thing); - logger.debug("Successfully loaded thing \'{}\'", thingUID); - } else { - // Possible cause: Asynchronous loading of the XML files - // Add the data to the queue in order to retry it later - logger.debug( - "ThingHandlerFactory \'{}\' claimed it can handle \'{}\' type but actually did not. Queued for later refresh.", - handlerFactory.getClass().getSimpleName(), thingTypeUID); - queueRetryThingCreation(handlerFactory, thingTypeUID, configuration, thingUID, bridgeUID); + Thing thingFromHandler = null; + ThingHandlerFactory handlerFactory = thingHandlerFactories.stream() + .filter(thf -> isThingHandlerFactoryReady(thf) && thf.supportsThingType(thingTypeUID)).findFirst() + .orElse(null); + if (handlerFactory != null) { + thingFromHandler = handlerFactory.createThing(thingTypeUID, configuration, thingUID, bridgeUID); + if (thingFromHandler != null) { + mergeThing(thingFromHandler, thing); + logger.debug("Successfully loaded thing \'{}\'", thingUID); + } else { + // Possible cause: Asynchronous loading of the XML files + // Add the data to the queue in order to retry it later + logger.debug( + "ThingHandlerFactory \'{}\' claimed it can handle \'{}\' type but actually did not. Queued for later refresh.", + handlerFactory.getClass().getSimpleName(), thingTypeUID); + queueRetryThingCreation(handlerFactory, thingTypeUID, configuration, thingUID, bridgeUID); + } } - } - return thingFromHandler != null ? thingFromHandler : thing; + return thingFromHandler != null ? thingFromHandler : thing; + } catch (IllegalArgumentException e) { + logger.warn("Error creating thing '{}', thing will be ignored: {}", thingDto.uid, e.getMessage()); + return null; + } } private List createChannels(ThingTypeUID thingTypeUID, ThingUID thingUID,