From 69964a2413933c8b97d4dafeb1746dee47d28429 Mon Sep 17 00:00:00 2001 From: Dmytro Skarzhynets Date: Mon, 7 Jul 2025 17:07:30 +0300 Subject: [PATCH 1/4] Make script compilation errors unrecoverable during rule node initialization --- .../service/script/RuleNodeScriptEngine.java | 8 ++++--- .../server/actors/TbActorMailbox.java | 5 +++-- .../script/api/TbScriptException.java | 21 +++++++++++++++++-- .../script/api/js/NashornJsInvokeService.java | 7 ++++++- .../api/tbel/DefaultTbelInvokeService.java | 16 ++++++++------ .../common/util/RecoveryAware.java} | 4 ++-- .../rule/engine/api/TbNodeException.java | 7 ++----- 7 files changed, 47 insertions(+), 21 deletions(-) rename common/{message/src/main/java/org/thingsboard/server/common/msg/TbActorError.java => util/src/main/java/org/thingsboard/common/util/RecoveryAware.java} (89%) diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java index 8f19aeb0a3..d99f1654f3 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java @@ -22,6 +22,7 @@ import lombok.extern.slf4j.Slf4j; import org.thingsboard.rule.engine.api.ScriptEngine; import org.thingsboard.script.api.ScriptInvokeService; import org.thingsboard.script.api.ScriptType; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.server.common.data.id.CustomerId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.msg.TbMsg; @@ -32,7 +33,6 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutionException; - @Slf4j public abstract class RuleNodeScriptEngine implements ScriptEngine { @@ -51,7 +51,10 @@ public abstract class RuleNodeScriptEngine imp if (e instanceof ExecutionException) { t = e.getCause(); } - throw new IllegalArgumentException("Can't compile script: " + t.getMessage(), t); + if (t instanceof TbScriptException scriptException) { + throw scriptException; + } + throw new RuntimeException("Unexpected error when creating script engine: " + t.getMessage(), t); } } @@ -81,7 +84,6 @@ public abstract class RuleNodeScriptEngine imp return Futures.transformAsync(executeScriptAsync(msg), this::executeToStringTransform, MoreExecutors.directExecutor()); } - @Override public ListenableFuture executeFilterAsync(TbMsg msg) { return Futures.transformAsync(executeScriptAsync(msg), diff --git a/common/actor/src/main/java/org/thingsboard/server/actors/TbActorMailbox.java b/common/actor/src/main/java/org/thingsboard/server/actors/TbActorMailbox.java index 6cd28fa98d..c20726d765 100644 --- a/common/actor/src/main/java/org/thingsboard/server/actors/TbActorMailbox.java +++ b/common/actor/src/main/java/org/thingsboard/server/actors/TbActorMailbox.java @@ -19,8 +19,8 @@ import lombok.Getter; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.thingsboard.server.common.data.EntityType; +import org.thingsboard.common.util.RecoveryAware; import org.thingsboard.server.common.msg.MsgType; -import org.thingsboard.server.common.msg.TbActorError; import org.thingsboard.server.common.msg.TbActorMsg; import org.thingsboard.server.common.msg.TbActorStopReason; @@ -35,6 +35,7 @@ import java.util.function.Supplier; @Getter @RequiredArgsConstructor public final class TbActorMailbox implements TbActorCtx { + private static final boolean HIGH_PRIORITY = true; private static final boolean NORMAL_PRIORITY = false; @@ -100,7 +101,7 @@ public final class TbActorMailbox implements TbActorCtx { if (t instanceof TbActorException && t.getCause() != null) { t = t.getCause(); } - return t instanceof TbActorError && ((TbActorError) t).isUnrecoverable(); + return t instanceof RecoveryAware recoveryAware && recoveryAware.isUnrecoverable(); } private void enqueue(TbActorMsg msg, boolean highPriority) { diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/TbScriptException.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/TbScriptException.java index 77888db255..347490b3fb 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/TbScriptException.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/TbScriptException.java @@ -16,13 +16,24 @@ package org.thingsboard.script.api; import lombok.Getter; +import org.thingsboard.common.util.RecoveryAware; +import java.io.Serial; import java.util.UUID; -public class TbScriptException extends RuntimeException { +public class TbScriptException extends RuntimeException implements RecoveryAware { + + @Serial private static final long serialVersionUID = -1958193538782818284L; - public static enum ErrorCode {COMPILATION, TIMEOUT, RUNTIME, OTHER} + public enum ErrorCode { + + COMPILATION, + TIMEOUT, + RUNTIME, + OTHER + + } @Getter private final UUID scriptId; @@ -37,4 +48,10 @@ public class TbScriptException extends RuntimeException { this.errorCode = errorCode; this.body = body; } + + @Override + public boolean isUnrecoverable() { + return errorCode == ErrorCode.COMPILATION; + } + } diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/js/NashornJsInvokeService.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/js/NashornJsInvokeService.java index 3507dd87e6..4aedab8081 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/js/NashornJsInvokeService.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/js/NashornJsInvokeService.java @@ -20,6 +20,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import delight.nashornsandbox.NashornSandbox; import delight.nashornsandbox.NashornSandboxes; +import delight.nashornsandbox.exceptions.ScriptCPUAbuseException; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import lombok.Getter; @@ -153,8 +154,12 @@ public class NashornJsInvokeService extends AbstractJsInvokeService { } scriptInfoMap.put(scriptId, scriptInfo); return scriptId; - } catch (Exception e) { + } catch (ScriptException e) { throw new TbScriptException(scriptId, TbScriptException.ErrorCode.COMPILATION, jsScript, e); + } catch (ScriptCPUAbuseException e) { + throw new TbScriptException(scriptId, TbScriptException.ErrorCode.TIMEOUT, jsScript, e); + } catch (Exception e) { + throw new TbScriptException(scriptId, TbScriptException.ErrorCode.OTHER, jsScript, e); } }); } diff --git a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/DefaultTbelInvokeService.java b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/DefaultTbelInvokeService.java index 25a7ede547..e2195a6bbf 100644 --- a/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/DefaultTbelInvokeService.java +++ b/common/script/script-api/src/main/java/org/thingsboard/script/api/tbel/DefaultTbelInvokeService.java @@ -27,6 +27,7 @@ import jakarta.annotation.PreDestroy; import lombok.Getter; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; +import org.mvel2.CompileException; import org.mvel2.ExecutionContext; import org.mvel2.MVEL; import org.mvel2.ParserContext; @@ -52,11 +53,11 @@ import java.io.Serializable; import java.nio.charset.StandardCharsets; import java.util.Calendar; import java.util.Collections; -import java.util.Map; import java.util.Optional; import java.util.Random; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -66,9 +67,9 @@ import java.util.concurrent.locks.ReentrantLock; @Service public class DefaultTbelInvokeService extends AbstractScriptInvokeService implements TbelInvokeService { - protected final Map scriptIdToHash = new ConcurrentHashMap<>(); - protected final Map scriptMap = new ConcurrentHashMap<>(); - protected Cache compiledScriptsCache; + private final ConcurrentMap scriptIdToHash = new ConcurrentHashMap<>(); + private final ConcurrentMap scriptMap = new ConcurrentHashMap<>(); + private Cache compiledScriptsCache; private SandboxedParserConfiguration parserConfig; private final Optional apiUsageStateClient; @@ -204,8 +205,10 @@ public class DefaultTbelInvokeService extends AbstractScriptInvokeService implem lock.unlock(); } return scriptId; - } catch (Exception e) { + } catch (CompileException e) { throw new TbScriptException(scriptId, TbScriptException.ErrorCode.COMPILATION, scriptBody, e); + } catch (Exception e) { + throw new TbScriptException(scriptId, TbScriptException.ErrorCode.OTHER, scriptBody, e); } }); } @@ -246,7 +249,7 @@ public class DefaultTbelInvokeService extends AbstractScriptInvokeService implem } } - private Serializable compileScript(String scriptBody) { + private static Serializable compileScript(String scriptBody) throws CompileException { return MVEL.compileExpression(scriptBody, new ParserContext()); } @@ -269,4 +272,5 @@ public class DefaultTbelInvokeService extends AbstractScriptInvokeService implem protected StatsType getStatsType() { return StatsType.TBEL_INVOKE; } + } diff --git a/common/message/src/main/java/org/thingsboard/server/common/msg/TbActorError.java b/common/util/src/main/java/org/thingsboard/common/util/RecoveryAware.java similarity index 89% rename from common/message/src/main/java/org/thingsboard/server/common/msg/TbActorError.java rename to common/util/src/main/java/org/thingsboard/common/util/RecoveryAware.java index 8c322d8eb5..e1553bec36 100644 --- a/common/message/src/main/java/org/thingsboard/server/common/msg/TbActorError.java +++ b/common/util/src/main/java/org/thingsboard/common/util/RecoveryAware.java @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.thingsboard.server.common.msg; +package org.thingsboard.common.util; -public interface TbActorError { +public interface RecoveryAware { boolean isUnrecoverable(); diff --git a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/TbNodeException.java b/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/TbNodeException.java index 874a37792d..7d5a7443d3 100644 --- a/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/TbNodeException.java +++ b/rule-engine/rule-engine-api/src/main/java/org/thingsboard/rule/engine/api/TbNodeException.java @@ -16,12 +16,9 @@ package org.thingsboard.rule.engine.api; import lombok.Getter; -import org.thingsboard.server.common.msg.TbActorError; +import org.thingsboard.common.util.RecoveryAware; -/** - * Created by ashvayka on 19.01.18. - */ -public class TbNodeException extends Exception implements TbActorError { +public class TbNodeException extends Exception implements RecoveryAware { @Getter private final boolean unrecoverable; From 2055dc83befd08c0ec4384f2bc80a6817d8db26d Mon Sep 17 00:00:00 2001 From: Dmytro Skarzhynets Date: Tue, 8 Jul 2025 14:29:55 +0300 Subject: [PATCH 2/4] Cleanup script engine classes --- .../script/RuleNodeJsScriptEngine.java | 152 ++++++++-------- .../service/script/RuleNodeScriptEngine.java | 68 +++----- .../script/RuleNodeTbelScriptEngine.java | 162 +++++++++--------- 3 files changed, 175 insertions(+), 207 deletions(-) diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java index b7490af487..e0b8351ff6 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java @@ -17,18 +17,16 @@ package org.thingsboard.server.service.script; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; -import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import lombok.extern.slf4j.Slf4j; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.script.api.RuleNodeScriptFactory; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.script.api.js.JsInvokeService; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.msg.TbMsg; import org.thingsboard.server.common.msg.TbMsgMetaData; -import javax.script.ScriptException; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -36,85 +34,12 @@ import java.util.List; import java.util.Map; import java.util.Set; - -@Slf4j public class RuleNodeJsScriptEngine extends RuleNodeScriptEngine { public RuleNodeJsScriptEngine(TenantId tenantId, JsInvokeService scriptInvokeService, String script, String... argNames) { super(tenantId, scriptInvokeService, script, argNames); } - @Override - public ListenableFuture executeJsonAsync(TbMsg msg) { - return executeScriptAsync(msg); - } - - @Override - protected ListenableFuture> executeUpdateTransform(TbMsg msg, JsonNode json) { - if (json.isObject()) { - return Futures.immediateFuture(Collections.singletonList(unbindMsg(json, msg))); - } else if (json.isArray()) { - List res = new ArrayList<>(json.size()); - json.forEach(jsonObject -> res.add(unbindMsg(jsonObject, msg))); - return Futures.immediateFuture(res); - } - log.warn("Wrong result type: {}", json.getNodeType()); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + json.getNodeType())); - } - - @Override - protected ListenableFuture executeGenerateTransform(TbMsg prevMsg, JsonNode result) { - if (!result.isObject()) { - log.warn("Wrong result type: {}", result.getNodeType()); - Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + result.getNodeType())); - } - return Futures.immediateFuture(unbindMsg(result, prevMsg)); - } - - @Override - protected JsonNode convertResult(Object result) { - return JacksonUtil.toJsonNode(result != null ? result.toString() : null); - } - - @Override - protected ListenableFuture executeToStringTransform(JsonNode result) { - if (result.isTextual()) { - return Futures.immediateFuture(result.asText()); - } - log.warn("Wrong result type: {}", result.getNodeType()); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + result.getNodeType())); - } - - @Override - protected ListenableFuture executeFilterTransform(JsonNode json) { - if (json.isBoolean()) { - return Futures.immediateFuture(json.asBoolean()); - } - log.warn("Wrong result type: {}", json.getNodeType()); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + json.getNodeType())); - } - - @Override - protected ListenableFuture> executeSwitchTransform(JsonNode result) { - if (result.isTextual()) { - return Futures.immediateFuture(Collections.singleton(result.asText())); - } - if (result.isArray()) { - Set nextStates = new HashSet<>(); - for (JsonNode val : result) { - if (!val.isTextual()) { - log.warn("Wrong result type: {}", val.getNodeType()); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + val.getNodeType())); - } else { - nextStates.add(val.asText()); - } - } - return Futures.immediateFuture(nextStates); - } - log.warn("Wrong result type: {}", result.getNodeType()); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + result.getNodeType())); - } - @Override protected Object[] prepareArgs(TbMsg msg) { String[] args = new String[3]; @@ -128,6 +53,71 @@ public class RuleNodeJsScriptEngine extends RuleNodeScriptEngine executeUpdateTransform(TbMsg msg, JsonNode json) { + if (json.isObject()) { + return Collections.singletonList(unbindMsg(json, msg)); + } else if (json.isArray()) { + List res = new ArrayList<>(json.size()); + json.forEach(jsonObject -> res.add(unbindMsg(jsonObject, msg))); + return res; + } + throw wrongResultType(json); + } + + @Override + protected TbMsg executeGenerateTransform(TbMsg prevMsg, JsonNode result) { + if (!result.isObject()) { + throw wrongResultType(result); + } + return unbindMsg(result, prevMsg); + } + + @Override + protected boolean executeFilterTransform(JsonNode json) { + if (json.isBoolean()) { + return json.asBoolean(); + } + throw wrongResultType(json); + } + + @Override + protected Set executeSwitchTransform(JsonNode result) { + if (result.isTextual()) { + return Collections.singleton(result.asText()); + } + if (result.isArray()) { + Set nextStates = new HashSet<>(); + for (JsonNode val : result) { + if (!val.isTextual()) { + throw wrongResultType(val); + } else { + nextStates.add(val.asText()); + } + } + return nextStates; + } + throw wrongResultType(result); + } + + @Override + public ListenableFuture executeJsonAsync(TbMsg msg) { + return executeScriptAsync(msg); + } + + @Override + protected String executeToStringTransform(JsonNode result) { + if (result.isTextual()) { + return result.asText(); + } + throw wrongResultType(result); + } + + @Override + protected JsonNode convertResult(Object result) { + return JacksonUtil.toJsonNode(result != null ? result.toString() : null); + } + private static TbMsg unbindMsg(JsonNode msgData, TbMsg msg) { String data = null; Map metadata = null; @@ -138,19 +128,23 @@ public class RuleNodeJsScriptEngine extends RuleNodeScriptEngine() { - }); + metadata = JacksonUtil.convertValue(msgMetadata, new TypeReference<>() {}); } if (msgData.has(RuleNodeScriptFactory.MSG_TYPE)) { messageType = msgData.get(RuleNodeScriptFactory.MSG_TYPE).asText(); } String newData = data != null ? data : msg.getData(); TbMsgMetaData newMetadata = metadata != null ? new TbMsgMetaData(metadata) : msg.getMetaData().copy(); - String newMessageType = !StringUtils.isEmpty(messageType) ? messageType : msg.getType(); + String newMessageType = StringUtils.isNotEmpty(messageType) ? messageType : msg.getType(); return msg.transform() .type(newMessageType) .metaData(newMetadata) .data(newData) .build(); } + + private TbScriptException wrongResultType(JsonNode result) { + return new TbScriptException(scriptId, TbScriptException.ErrorCode.RUNTIME, null, new ClassCastException("Wrong result type: " + result.getNodeType())); + } + } diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java index d99f1654f3..ec9c2fd983 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeScriptEngine.java @@ -17,7 +17,6 @@ package org.thingsboard.server.service.script; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.MoreExecutors; import lombok.extern.slf4j.Slf4j; import org.thingsboard.rule.engine.api.ScriptEngine; import org.thingsboard.script.api.ScriptInvokeService; @@ -27,25 +26,26 @@ import org.thingsboard.server.common.data.id.CustomerId; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.msg.TbMsg; -import javax.script.ScriptException; import java.util.List; import java.util.Set; import java.util.UUID; import java.util.concurrent.ExecutionException; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; + @Slf4j public abstract class RuleNodeScriptEngine implements ScriptEngine { private final T scriptInvokeService; - private final UUID scriptId; + protected final UUID scriptId; private final TenantId tenantId; public RuleNodeScriptEngine(TenantId tenantId, T scriptInvokeService, String script, String... argNames) { this.tenantId = tenantId; this.scriptInvokeService = scriptInvokeService; try { - this.scriptId = this.scriptInvokeService.eval(tenantId, ScriptType.RULE_NODE_SCRIPT, script, argNames).get(); + scriptId = this.scriptInvokeService.eval(tenantId, ScriptType.RULE_NODE_SCRIPT, script, argNames).get(); } catch (Exception e) { Throwable t = e; if (e instanceof ExecutionException) { @@ -63,73 +63,53 @@ public abstract class RuleNodeScriptEngine imp @Override public ListenableFuture> executeUpdateAsync(TbMsg msg) { ListenableFuture result = executeScriptAsync(msg); - return Futures.transformAsync(result, - json -> executeUpdateTransform(msg, json), - MoreExecutors.directExecutor()); + return Futures.transform(result, json -> executeUpdateTransform(msg, json), directExecutor()); } - protected abstract ListenableFuture> executeUpdateTransform(TbMsg msg, R result); + protected abstract List executeUpdateTransform(TbMsg msg, R result); @Override public ListenableFuture executeGenerateAsync(TbMsg prevMsg) { - return Futures.transformAsync(executeScriptAsync(prevMsg), - result -> executeGenerateTransform(prevMsg, result), - MoreExecutors.directExecutor()); + return Futures.transform(executeScriptAsync(prevMsg), result -> executeGenerateTransform(prevMsg, result), directExecutor()); } - protected abstract ListenableFuture executeGenerateTransform(TbMsg prevMsg, R result); - - @Override - public ListenableFuture executeToStringAsync(TbMsg msg) { - return Futures.transformAsync(executeScriptAsync(msg), this::executeToStringTransform, MoreExecutors.directExecutor()); - } + protected abstract TbMsg executeGenerateTransform(TbMsg prevMsg, R result); @Override public ListenableFuture executeFilterAsync(TbMsg msg) { - return Futures.transformAsync(executeScriptAsync(msg), - this::executeFilterTransform, - MoreExecutors.directExecutor()); + return Futures.transform(executeScriptAsync(msg), this::executeFilterTransform, directExecutor()); } - protected abstract ListenableFuture executeToStringTransform(R result); - - protected abstract ListenableFuture executeFilterTransform(R result); - - protected abstract ListenableFuture> executeSwitchTransform(R result); + protected abstract boolean executeFilterTransform(R result); @Override public ListenableFuture> executeSwitchAsync(TbMsg msg) { - return Futures.transformAsync(executeScriptAsync(msg), - this::executeSwitchTransform, - MoreExecutors.directExecutor()); //usually runs in a callbackExecutor + return Futures.transform(executeScriptAsync(msg), this::executeSwitchTransform, directExecutor()); // usually runs on a callbackExecutor } + protected abstract Set executeSwitchTransform(R result); + + @Override + public ListenableFuture executeToStringAsync(TbMsg msg) { + return Futures.transform(executeScriptAsync(msg), this::executeToStringTransform, directExecutor()); + } + + protected abstract String executeToStringTransform(R result); + ListenableFuture executeScriptAsync(TbMsg msg) { log.trace("execute script async, msg {}", msg); Object[] inArgs = prepareArgs(msg); return executeScriptAsync(msg.getCustomerId(), inArgs[0], inArgs[1], inArgs[2]); } - ListenableFuture executeScriptAsync(CustomerId customerId, Object... args) { - return Futures.transformAsync(scriptInvokeService.invokeScript(tenantId, customerId, this.scriptId, args), - o -> { - try { - return Futures.immediateFuture(convertResult(o)); - } catch (Exception e) { - if (e.getCause() instanceof ScriptException) { - return Futures.immediateFailedFuture(e.getCause()); - } else if (e.getCause() instanceof RuntimeException) { - return Futures.immediateFailedFuture(new ScriptException(e.getCause().getMessage())); - } else { - return Futures.immediateFailedFuture(new ScriptException(e)); - } - } - }, MoreExecutors.directExecutor()); + private ListenableFuture executeScriptAsync(CustomerId customerId, Object... args) { + return Futures.transform(scriptInvokeService.invokeScript(tenantId, customerId, scriptId, args), this::convertResult, directExecutor()); } public void destroy() { - scriptInvokeService.release(this.scriptId); + scriptInvokeService.release(scriptId); } protected abstract R convertResult(Object result); + } diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeTbelScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeTbelScriptEngine.java index 5e197d0e5d..991be63f81 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeTbelScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeTbelScriptEngine.java @@ -19,17 +19,15 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.MoreExecutors; -import lombok.extern.slf4j.Slf4j; import org.thingsboard.common.util.JacksonUtil; import org.thingsboard.script.api.RuleNodeScriptFactory; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.script.api.tbel.TbelInvokeService; import org.thingsboard.server.common.data.StringUtils; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.common.msg.TbMsg; import org.thingsboard.server.common.msg.TbMsgMetaData; -import javax.script.ScriptException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -40,86 +38,14 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; -@Slf4j public class RuleNodeTbelScriptEngine extends RuleNodeScriptEngine { public RuleNodeTbelScriptEngine(TenantId tenantId, TbelInvokeService scriptInvokeService, String script, String... argNames) { super(tenantId, scriptInvokeService, script, argNames); } - @Override - protected ListenableFuture executeFilterTransform(Object result) { - if (result instanceof Boolean) { - return Futures.immediateFuture((Boolean) result); - } - return wrongResultType(result); - } - - @Override - protected ListenableFuture> executeUpdateTransform(TbMsg msg, Object result) { - if (result instanceof Map) { - return Futures.immediateFuture(Collections.singletonList(unbindMsg((Map) result, msg))); - } else if (result instanceof Collection) { - List res = new ArrayList<>(); - for (Object resObject : (Collection) result) { - if (resObject instanceof Map) { - res.add(unbindMsg((Map) resObject, msg)); - } else { - return wrongResultType(resObject); - } - } - return Futures.immediateFuture(res); - } - return wrongResultType(result); - } - - @Override - protected ListenableFuture executeGenerateTransform(TbMsg prevMsg, Object result) { - if (result instanceof Map) { - return Futures.immediateFuture(unbindMsg((Map) result, prevMsg)); - } - return wrongResultType(result); - } - - @Override - protected ListenableFuture executeToStringTransform(Object result) { - if (result instanceof String) { - return Futures.immediateFuture((String) result); - } else { - return Futures.immediateFuture(JacksonUtil.toString(result)); - } - } - - @Override - protected ListenableFuture> executeSwitchTransform(Object result) { - if (result instanceof String) { - return Futures.immediateFuture(Collections.singleton((String) result)); - } else if (result instanceof Collection) { - Set res = new HashSet<>(); - for (Object resObject : (Collection) result) { - if (resObject instanceof String) { - res.add((String) resObject); - } else { - return wrongResultType(resObject); - } - } - return Futures.immediateFuture(res); - } - return wrongResultType(result); - } - - @Override - public ListenableFuture executeJsonAsync(TbMsg msg) { - return Futures.transform(executeScriptAsync(msg), JacksonUtil::valueToTree, MoreExecutors.directExecutor()); - - } - - @Override - protected Object convertResult(Object result) { - return result; - } - @Override protected Object[] prepareArgs(TbMsg msg) { Object[] args = new Object[3]; @@ -133,6 +59,74 @@ public class RuleNodeTbelScriptEngine extends RuleNodeScriptEngine executeUpdateTransform(TbMsg msg, Object result) { + if (result instanceof Map msgData) { + return Collections.singletonList(unbindMsg(msgData, msg)); + } else if (result instanceof Collection resultCollection) { + List res = new ArrayList<>(resultCollection.size()); + for (Object resObject : resultCollection) { + if (resObject instanceof Map msgData) { + res.add(unbindMsg(msgData, msg)); + } else { + throw wrongResultType(resObject); + } + } + return res; + } + throw wrongResultType(result); + } + + @Override + protected TbMsg executeGenerateTransform(TbMsg prevMsg, Object result) { + if (result instanceof Map msgData) { + return unbindMsg(msgData, prevMsg); + } + throw wrongResultType(result); + } + + @Override + protected boolean executeFilterTransform(Object result) { + if (result instanceof Boolean b) { + return b; + } + throw wrongResultType(result); + } + + @Override + protected Set executeSwitchTransform(Object result) { + if (result instanceof String str) { + return Collections.singleton(str); + } + if (result instanceof Collection resultCollection) { + Set res = new HashSet<>(resultCollection.size()); + for (Object resObject : resultCollection) { + if (resObject instanceof String str) { + res.add(str); + } else { + throw wrongResultType(resObject); + } + } + return res; + } + throw wrongResultType(result); + } + + @Override + public ListenableFuture executeJsonAsync(TbMsg msg) { + return Futures.transform(executeScriptAsync(msg), JacksonUtil::valueToTree, directExecutor()); + } + + @Override + protected Object convertResult(Object result) { + return result; + } + + @Override + protected String executeToStringTransform(Object result) { + return result instanceof String str ? str : JacksonUtil.toString(result); + } + private static TbMsg unbindMsg(Map msgData, TbMsg msg) { String data = null; Map metadata = null; @@ -142,12 +136,12 @@ public class RuleNodeTbelScriptEngine extends RuleNodeScriptEngine) msgMetadataObj).entrySet().stream().filter(e -> e.getValue() != null) + if (msgMetadataObj instanceof Map msgMetadataObjAsMap) { + metadata = msgMetadataObjAsMap.entrySet().stream() + .filter(e -> e.getValue() != null) .collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString())); } else { - metadata = JacksonUtil.convertValue(msgMetadataObj, new TypeReference<>() { - }); + metadata = JacksonUtil.convertValue(msgMetadataObj, new TypeReference<>() {}); } } if (msgData.containsKey(RuleNodeScriptFactory.MSG_TYPE)) { @@ -155,7 +149,7 @@ public class RuleNodeTbelScriptEngine extends RuleNodeScriptEngine ListenableFuture wrongResultType(Object result) { + private TbScriptException wrongResultType(Object result) { String className = toClassName(result); - log.warn("Wrong result type: {}", className); - return Futures.immediateFailedFuture(new ScriptException("Wrong result type: " + className)); + return new TbScriptException(scriptId, TbScriptException.ErrorCode.RUNTIME, null, new ClassCastException("Wrong result type: " + className)); } private static String toClassName(Object result) { return result != null ? result.getClass().getSimpleName() : "null"; } + } From a0e8b014297c0569644ba3bef67ee794731c225f Mon Sep 17 00:00:00 2001 From: Dmytro Skarzhynets Date: Tue, 8 Jul 2025 15:32:24 +0300 Subject: [PATCH 3/4] Add tests for compilation errors when evaluating scripts --- .../script/NashornJsInvokeServiceTest.java | 22 +++++++++ .../script/RemoteJsInvokeServiceTest.java | 45 ++++++++++++++--- .../service/script/TbelInvokeServiceTest.java | 22 +++++++++ .../script/api/TbScriptExceptionTest.java | 49 +++++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 common/script/script-api/src/test/java/org/thingsboard/script/api/TbScriptExceptionTest.java diff --git a/application/src/test/java/org/thingsboard/server/service/script/NashornJsInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/NashornJsInvokeServiceTest.java index 0942cef75d..b8ab48b38d 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/NashornJsInvokeServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/script/NashornJsInvokeServiceTest.java @@ -25,11 +25,13 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.test.context.TestPropertySource; import org.thingsboard.common.util.TbStopWatch; import org.thingsboard.script.api.ScriptType; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.script.api.js.NashornJsInvokeService; import org.thingsboard.server.common.data.id.TenantId; import org.thingsboard.server.controller.AbstractControllerTest; import org.thingsboard.server.dao.service.DaoSqlTest; +import javax.script.ScriptException; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -39,6 +41,7 @@ import java.util.concurrent.TimeoutException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.thingsboard.server.common.data.msg.TbMsgType.POST_TELEMETRY_REQUEST; @@ -59,6 +62,25 @@ class NashornJsInvokeServiceTest extends AbstractControllerTest { @Value("${js.local.max_errors}") private int maxJsErrors; + @Test + void givenUncompilableScript_whenEvaluating_thenThrowsErrorWithCompilationErrorCode() { + // GIVEN + var uncompilableScript = "return msg.temperature?.value;"; + + // WHEN-THEN + assertThatThrownBy(() -> evalScript(uncompilableScript)) + .isInstanceOf(ExecutionException.class) + .cause() + .isInstanceOf(TbScriptException.class) + .asInstanceOf(type(TbScriptException.class)) + .satisfies(ex -> { + assertThat(ex.getScriptId()).isNotNull(); + assertThat(ex.getErrorCode()).isEqualTo(TbScriptException.ErrorCode.COMPILATION); + assertThat(ex.getBody()).contains(uncompilableScript); + assertThat(ex.getCause()).isInstanceOf(ScriptException.class); + }); + } + @Test void givenSimpleScriptTestPerformance() throws ExecutionException, InterruptedException { int iterations = 1000; diff --git a/application/src/test/java/org/thingsboard/server/service/script/RemoteJsInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/RemoteJsInvokeServiceTest.java index 363f21fa10..36990d9768 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/RemoteJsInvokeServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/script/RemoteJsInvokeServiceTest.java @@ -23,9 +23,9 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.springframework.test.util.ReflectionTestUtils; import org.thingsboard.script.api.ScriptType; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.server.common.data.ApiUsageState; import org.thingsboard.server.common.data.id.TenantId; -import org.thingsboard.server.common.stats.DefaultStatsFactory; import org.thingsboard.server.common.stats.StatsCounter; import org.thingsboard.server.common.stats.StatsFactory; import org.thingsboard.server.common.stats.TbApiUsageReportClient; @@ -42,8 +42,11 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ExecutionException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; @@ -60,7 +63,6 @@ class RemoteJsInvokeServiceTest { private RemoteJsInvokeService remoteJsInvokeService; private TbQueueRequestTemplate, TbProtoQueueMsg> jsRequestTemplate; - @BeforeEach public void beforeEach() { TbApiUsageStateClient apiUsageStateClient = mock(TbApiUsageStateClient.class); @@ -74,7 +76,7 @@ class RemoteJsInvokeServiceTest { remoteJsInvokeService.requestTemplate = jsRequestTemplate; StatsFactory statsFactory = mock(StatsFactory.class); when(statsFactory.createStatsCounter(any(), any())).thenReturn(mock(StatsCounter.class)); - ReflectionTestUtils.setField(remoteJsInvokeService, "statsFactory",statsFactory); + ReflectionTestUtils.setField(remoteJsInvokeService, "statsFactory", statsFactory); remoteJsInvokeService.init(); } @@ -84,7 +86,36 @@ class RemoteJsInvokeServiceTest { } @Test - public void whenInvokingFunction_thenDoNotSendScriptBody() throws Exception { + void givenUncompilableScript_whenEvaluating_thenThrowsErrorWithCompilationErrorCode() { + // GIVEN + doAnswer(methodCall -> Futures.immediateFuture(new TbProtoJsQueueMsg<>(UUID.randomUUID(), RemoteJsResponse.newBuilder() + .setCompileResponse(JsInvokeProtos.JsCompileResponse.newBuilder() + .setSuccess(false) + .setErrorCode(JsInvokeProtos.JsInvokeErrorCode.COMPILATION_ERROR) + .setErrorDetails("SyntaxError: Unexpected token 'const'") + .setScriptHash(methodCall.>getArgument(0).getValue().getCompileRequest().getScriptHash()) + .build()) + .build()))) + .when(jsRequestTemplate).send(argThat(jsQueueMsg -> jsQueueMsg.getValue().hasCompileRequest())); + + var uncompilableScript = "let const = 'this is not allowed';"; + + // WHEN-THEN + assertThatThrownBy(() -> remoteJsInvokeService.eval(TenantId.SYS_TENANT_ID, ScriptType.RULE_NODE_SCRIPT, uncompilableScript).get()) + .isInstanceOf(ExecutionException.class) + .cause() + .isInstanceOf(TbScriptException.class) + .asInstanceOf(type(TbScriptException.class)) + .satisfies(ex -> { + assertThat(ex.getScriptId()).isNotNull(); + assertThat(ex.getErrorCode()).isEqualTo(TbScriptException.ErrorCode.COMPILATION); + assertThat(ex.getBody()).contains(uncompilableScript); + assertThat(ex.getCause()).isInstanceOf(RuntimeException.class).hasMessage("SyntaxError: Unexpected token 'const'"); + }); + } + + @Test + void whenInvokingFunction_thenDoNotSendScriptBody() throws Exception { mockJsEvalResponse(); String scriptBody = "return { a: 'b'};"; UUID scriptId = remoteJsInvokeService.eval(TenantId.SYS_TENANT_ID, ScriptType.RULE_NODE_SCRIPT, scriptBody).get(); @@ -110,7 +141,7 @@ class RemoteJsInvokeServiceTest { } @Test - public void whenInvokingFunctionAndRemoteJsExecutorRemovedScript_thenHandleNotFoundErrorAndMakeInvokeRequestWithScriptBody() throws Exception { + void whenInvokingFunctionAndRemoteJsExecutorRemovedScript_thenHandleNotFoundErrorAndMakeInvokeRequestWithScriptBody() throws Exception { mockJsEvalResponse(); String scriptBody = "return { a: 'b'};"; UUID scriptId = remoteJsInvokeService.eval(TenantId.SYS_TENANT_ID, ScriptType.RULE_NODE_SCRIPT, scriptBody).get(); @@ -156,7 +187,7 @@ class RemoteJsInvokeServiceTest { } @Test - public void whenDoingEval_thenSaveScriptByHashOfTenantIdAndScriptBody() throws Exception { + void whenDoingEval_thenSaveScriptByHashOfTenantIdAndScriptBody() throws Exception { mockJsEvalResponse(); TenantId tenantId1 = TenantId.fromUUID(UUID.randomUUID()); @@ -187,7 +218,7 @@ class RemoteJsInvokeServiceTest { } @Test - public void whenReleasingScript_thenCheckForHashUsages() throws Exception { + void whenReleasingScript_thenCheckForHashUsages() throws Exception { mockJsEvalResponse(); String scriptBody = "return { a: 'b'};"; UUID scriptId1 = remoteJsInvokeService.eval(TenantId.SYS_TENANT_ID, ScriptType.RULE_NODE_SCRIPT, scriptBody).get(); diff --git a/application/src/test/java/org/thingsboard/server/service/script/TbelInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/TbelInvokeServiceTest.java index 732f31f044..fc66f806d7 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/TbelInvokeServiceTest.java +++ b/application/src/test/java/org/thingsboard/server/service/script/TbelInvokeServiceTest.java @@ -20,10 +20,12 @@ import com.github.benmanes.caffeine.cache.Cache; import org.junit.Assert; import org.junit.Ignore; import org.junit.jupiter.api.Test; +import org.mvel2.CompileException; import org.springframework.beans.factory.annotation.Value; import org.springframework.test.context.TestPropertySource; import org.springframework.test.util.ReflectionTestUtils; import org.thingsboard.common.util.JacksonUtil; +import org.thingsboard.script.api.TbScriptException; import org.thingsboard.script.api.tbel.TbelScript; import java.io.Serializable; @@ -37,6 +39,7 @@ import java.util.concurrent.TimeUnit; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.InstanceOfAssertFactories.type; @TestPropertySource(properties = { "tbel.max_script_body_size=100", @@ -50,6 +53,25 @@ class TbelInvokeServiceTest extends AbstractTbelInvokeTest { @Value("${tbel.max_errors}") private int maxJsErrors; + @Test + void givenUncompilableScript_whenEvaluating_thenThrowsErrorWithCompilationErrorCode() { + // GIVEN + var uncompilableScript = "return msg.property !== undefined;"; + + // WHEN-THEN + assertThatThrownBy(() -> evalScript(uncompilableScript)) + .isInstanceOf(ExecutionException.class) + .cause() + .isInstanceOf(TbScriptException.class) + .asInstanceOf(type(TbScriptException.class)) + .satisfies(ex -> { + assertThat(ex.getScriptId()).isNotNull(); + assertThat(ex.getErrorCode()).isEqualTo(TbScriptException.ErrorCode.COMPILATION); + assertThat(ex.getBody()).isEqualTo(uncompilableScript); + assertThat(ex.getCause()).isInstanceOf(CompileException.class); + }); + } + @Test void givenSimpleScriptTestPerformance() throws ExecutionException, InterruptedException { int iterations = 100000; diff --git a/common/script/script-api/src/test/java/org/thingsboard/script/api/TbScriptExceptionTest.java b/common/script/script-api/src/test/java/org/thingsboard/script/api/TbScriptExceptionTest.java new file mode 100644 index 0000000000..330b895504 --- /dev/null +++ b/common/script/script-api/src/test/java/org/thingsboard/script/api/TbScriptExceptionTest.java @@ -0,0 +1,49 @@ +/** + * Copyright © 2016-2025 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.script.api; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; + +import static org.assertj.core.api.Assertions.assertThat; + +class TbScriptExceptionTest { + + @Test + void givenCompilationError_whenCheckingIsUnrecoverable_thenReturnsTrue() { + // GIVEN + var exception = new TbScriptException(null, TbScriptException.ErrorCode.COMPILATION, null, null); + + // WHEN-THEN + assertThat(exception.isUnrecoverable()).isTrue(); + } + + @ParameterizedTest + @EnumSource( + value = TbScriptException.ErrorCode.class, + mode = EnumSource.Mode.EXCLUDE, + names = "COMPILATION" + ) + void givenRecoverableErrorCodes_whenCheckingIsUnrecoverable_thenReturnsFalse(TbScriptException.ErrorCode errorCode) { + // GIVEN + var exception = new TbScriptException(null, errorCode, null, null); + + // WHEN-THEN + assertThat(exception.isUnrecoverable()).isFalse(); + } + +} From 480b89c11c291174857b0fa0a01980f6da82cde1 Mon Sep 17 00:00:00 2001 From: Sergey Matvienko Date: Sat, 12 Jul 2025 10:09:09 +0200 Subject: [PATCH 4/4] Reduced log severity to debug for Scheduling reconnect message to avoid log flood under high-load. Logs are made more informative. --- .../src/main/java/org/thingsboard/mqtt/MqttClientImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/netty-mqtt/src/main/java/org/thingsboard/mqtt/MqttClientImpl.java b/netty-mqtt/src/main/java/org/thingsboard/mqtt/MqttClientImpl.java index 801470284b..69d2b6e192 100644 --- a/netty-mqtt/src/main/java/org/thingsboard/mqtt/MqttClientImpl.java +++ b/netty-mqtt/src/main/java/org/thingsboard/mqtt/MqttClientImpl.java @@ -192,14 +192,14 @@ final class MqttClientImpl implements MqttClient { } private void scheduleConnectIfRequired(String host, int port, boolean reconnect) { - log.trace("[{}] Scheduling connect to server, isReconnect - {}", channel != null ? channel.id() : "UNKNOWN", reconnect); + log.trace("[{}][{}][{}] Scheduling connect to server, isReconnect - {}", host, port, channel != null ? channel.id() : "UNKNOWN", reconnect); if (clientConfig.isReconnect() && !disconnected) { if (reconnect) { this.reconnect = true; } final long nextReconnectDelay = reconnectStrategy.getNextReconnectDelay(); - log.info("[{}] Scheduling reconnect in [{}] sec", channel != null ? channel.id() : "UNKNOWN", nextReconnectDelay); + log.debug("[{}][{}][{}] Scheduling reconnect in [{}] sec", host, port, channel != null ? channel.id() : "UNKNOWN", nextReconnectDelay); eventLoop.schedule((Runnable) () -> connect(host, port, reconnect), nextReconnectDelay, TimeUnit.SECONDS); } }