Commit 5ea053b71b385e53dc92728ea0486e39b3851b7c
1 parent
831f6201
blacklisted Script error should show initial exception
Showing
2 changed files
with
55 additions
and
40 deletions
application/src/main/java/org/thingsboard/server/service/script/AbstractNashornJsSandboxService.java
@@ -17,13 +17,13 @@ package org.thingsboard.server.service.script; | @@ -17,13 +17,13 @@ package org.thingsboard.server.service.script; | ||
17 | 17 | ||
18 | import com.google.common.util.concurrent.Futures; | 18 | import com.google.common.util.concurrent.Futures; |
19 | import com.google.common.util.concurrent.ListenableFuture; | 19 | import com.google.common.util.concurrent.ListenableFuture; |
20 | -import com.google.common.util.concurrent.ListeningExecutorService; | ||
21 | -import com.google.common.util.concurrent.MoreExecutors; | ||
22 | import delight.nashornsandbox.NashornSandbox; | 20 | import delight.nashornsandbox.NashornSandbox; |
23 | import delight.nashornsandbox.NashornSandboxes; | 21 | import delight.nashornsandbox.NashornSandboxes; |
24 | import jdk.nashorn.api.scripting.NashornScriptEngineFactory; | 22 | import jdk.nashorn.api.scripting.NashornScriptEngineFactory; |
23 | +import lombok.Data; | ||
25 | import lombok.EqualsAndHashCode; | 24 | import lombok.EqualsAndHashCode; |
26 | import lombok.Getter; | 25 | import lombok.Getter; |
26 | +import lombok.RequiredArgsConstructor; | ||
27 | import lombok.extern.slf4j.Slf4j; | 27 | import lombok.extern.slf4j.Slf4j; |
28 | import org.thingsboard.server.common.data.id.EntityId; | 28 | import org.thingsboard.server.common.data.id.EntityId; |
29 | 29 | ||
@@ -45,10 +45,9 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -45,10 +45,9 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
45 | private NashornSandbox sandbox; | 45 | private NashornSandbox sandbox; |
46 | private ScriptEngine engine; | 46 | private ScriptEngine engine; |
47 | private ExecutorService monitorExecutorService; | 47 | private ExecutorService monitorExecutorService; |
48 | - private ListeningExecutorService evalExecutorService; | ||
49 | 48 | ||
50 | private final Map<UUID, String> functionsMap = new ConcurrentHashMap<>(); | 49 | private final Map<UUID, String> functionsMap = new ConcurrentHashMap<>(); |
51 | - private final Map<BlackListKey, AtomicInteger> blackListedFunctions = new ConcurrentHashMap<>(); | 50 | + private final Map<BlackListKey, BlackListInfo> blackListedFunctions = new ConcurrentHashMap<>(); |
52 | 51 | ||
53 | private final Map<String, ScriptInfo> scriptKeyToInfo = new ConcurrentHashMap<>(); | 52 | private final Map<String, ScriptInfo> scriptKeyToInfo = new ConcurrentHashMap<>(); |
54 | private final Map<UUID, ScriptInfo> scriptIdToInfo = new ConcurrentHashMap<>(); | 53 | private final Map<UUID, ScriptInfo> scriptIdToInfo = new ConcurrentHashMap<>(); |
@@ -58,7 +57,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -58,7 +57,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
58 | if (useJsSandbox()) { | 57 | if (useJsSandbox()) { |
59 | sandbox = NashornSandboxes.create(); | 58 | sandbox = NashornSandboxes.create(); |
60 | monitorExecutorService = Executors.newFixedThreadPool(getMonitorThreadPoolSize()); | 59 | monitorExecutorService = Executors.newFixedThreadPool(getMonitorThreadPoolSize()); |
61 | - evalExecutorService = MoreExecutors.listeningDecorator(Executors.newFixedThreadPool(10)); | ||
62 | sandbox.setExecutor(monitorExecutorService); | 60 | sandbox.setExecutor(monitorExecutorService); |
63 | sandbox.setMaxCPUTime(getMaxCpuTime()); | 61 | sandbox.setMaxCPUTime(getMaxCpuTime()); |
64 | sandbox.allowNoBraces(false); | 62 | sandbox.allowNoBraces(false); |
@@ -74,9 +72,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -74,9 +72,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
74 | if (monitorExecutorService != null) { | 72 | if (monitorExecutorService != null) { |
75 | monitorExecutorService.shutdownNow(); | 73 | monitorExecutorService.shutdownNow(); |
76 | } | 74 | } |
77 | - if (evalExecutorService != null) { | ||
78 | - evalExecutorService.shutdownNow(); | ||
79 | - } | ||
80 | } | 75 | } |
81 | 76 | ||
82 | protected abstract boolean useJsSandbox(); | 77 | protected abstract boolean useJsSandbox(); |
@@ -124,26 +119,35 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -124,26 +119,35 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
124 | public ListenableFuture<Object> invokeFunction(UUID scriptId, EntityId entityId, Object... args) { | 119 | public ListenableFuture<Object> invokeFunction(UUID scriptId, EntityId entityId, Object... args) { |
125 | String functionName = functionsMap.get(scriptId); | 120 | String functionName = functionsMap.get(scriptId); |
126 | if (functionName == null) { | 121 | if (functionName == null) { |
127 | - return Futures.immediateFailedFuture(new RuntimeException("No compiled script found for scriptId: [" + scriptId + "]!")); | ||
128 | - } | ||
129 | - if (!isBlackListed(scriptId)) { | ||
130 | - try { | ||
131 | - Object result; | ||
132 | - if (useJsSandbox()) { | ||
133 | - result = sandbox.getSandboxedInvocable().invokeFunction(functionName, args); | ||
134 | - } else { | ||
135 | - result = ((Invocable) engine).invokeFunction(functionName, args); | ||
136 | - } | ||
137 | - return Futures.immediateFuture(result); | ||
138 | - } catch (Exception e) { | ||
139 | - BlackListKey blackListKey = new BlackListKey(scriptId, entityId); | ||
140 | - blackListedFunctions.computeIfAbsent(blackListKey, key -> new AtomicInteger(0)).incrementAndGet(); | ||
141 | - return Futures.immediateFailedFuture(e); | ||
142 | - } | 122 | + String message = "No compiled script found for scriptId: [" + scriptId + "]!"; |
123 | + log.warn(message); | ||
124 | + return Futures.immediateFailedFuture(new RuntimeException(message)); | ||
125 | + } | ||
126 | + | ||
127 | + BlackListInfo blackListInfo = blackListedFunctions.get(new BlackListKey(scriptId, entityId)); | ||
128 | + if (blackListInfo != null && blackListInfo.getCount() >= getMaxErrors()) { | ||
129 | + RuntimeException throwable = new RuntimeException("Script is blacklisted due to maximum error count " + getMaxErrors() + "!", blackListInfo.getCause()); | ||
130 | + throwable.printStackTrace(); | ||
131 | + return Futures.immediateFailedFuture(throwable); | ||
132 | + } | ||
133 | + | ||
134 | + try { | ||
135 | + return invoke(functionName, args); | ||
136 | + } catch (Exception e) { | ||
137 | + BlackListKey blackListKey = new BlackListKey(scriptId, entityId); | ||
138 | + blackListedFunctions.computeIfAbsent(blackListKey, key -> new BlackListInfo()).incrementWithReason(e); | ||
139 | + return Futures.immediateFailedFuture(e); | ||
140 | + } | ||
141 | + } | ||
142 | + | ||
143 | + private ListenableFuture<Object> invoke(String functionName, Object... args) throws ScriptException, NoSuchMethodException { | ||
144 | + Object result; | ||
145 | + if (useJsSandbox()) { | ||
146 | + result = sandbox.getSandboxedInvocable().invokeFunction(functionName, args); | ||
143 | } else { | 147 | } else { |
144 | - return Futures.immediateFailedFuture( | ||
145 | - new RuntimeException("Script is blacklisted due to maximum error count " + getMaxErrors() + "!")); | 148 | + result = ((Invocable) engine).invokeFunction(functionName, args); |
146 | } | 149 | } |
150 | + return Futures.immediateFuture(result); | ||
147 | } | 151 | } |
148 | 152 | ||
149 | @Override | 153 | @Override |
@@ -182,15 +186,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -182,15 +186,6 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
182 | } | 186 | } |
183 | 187 | ||
184 | 188 | ||
185 | - private boolean isBlackListed(UUID scriptId) { | ||
186 | - if (blackListedFunctions.containsKey(scriptId)) { | ||
187 | - AtomicInteger errorCount = blackListedFunctions.get(scriptId); | ||
188 | - return errorCount.get() >= getMaxErrors(); | ||
189 | - } else { | ||
190 | - return false; | ||
191 | - } | ||
192 | - } | ||
193 | - | ||
194 | private String generateJsScript(JsScriptType scriptType, String functionName, String scriptBody, String... argNames) { | 189 | private String generateJsScript(JsScriptType scriptType, String functionName, String scriptBody, String... argNames) { |
195 | switch (scriptType) { | 190 | switch (scriptType) { |
196 | case RULE_NODE_SCRIPT: | 191 | case RULE_NODE_SCRIPT: |
@@ -233,13 +228,33 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | @@ -233,13 +228,33 @@ public abstract class AbstractNashornJsSandboxService implements JsSandboxServic | ||
233 | 228 | ||
234 | @EqualsAndHashCode | 229 | @EqualsAndHashCode |
235 | @Getter | 230 | @Getter |
231 | + @RequiredArgsConstructor | ||
236 | private static class BlackListKey { | 232 | private static class BlackListKey { |
237 | private final UUID scriptId; | 233 | private final UUID scriptId; |
238 | private final EntityId entityId; | 234 | private final EntityId entityId; |
239 | 235 | ||
240 | - public BlackListKey(UUID scriptId, EntityId entityId) { | ||
241 | - this.scriptId = scriptId; | ||
242 | - this.entityId = entityId; | 236 | + } |
237 | + | ||
238 | + @Data | ||
239 | + private static class BlackListInfo { | ||
240 | + private final AtomicInteger count; | ||
241 | + private Exception ex; | ||
242 | + | ||
243 | + BlackListInfo() { | ||
244 | + this.count = new AtomicInteger(0); | ||
245 | + } | ||
246 | + | ||
247 | + void incrementWithReason(Exception e) { | ||
248 | + count.incrementAndGet(); | ||
249 | + ex = e; | ||
250 | + } | ||
251 | + | ||
252 | + int getCount() { | ||
253 | + return count.get(); | ||
254 | + } | ||
255 | + | ||
256 | + Exception getCause() { | ||
257 | + return ex; | ||
243 | } | 258 | } |
244 | } | 259 | } |
245 | } | 260 | } |
@@ -171,10 +171,10 @@ public class RuleNodeJsScriptEngine implements org.thingsboard.rule.engine.api.S | @@ -171,10 +171,10 @@ public class RuleNodeJsScriptEngine implements org.thingsboard.rule.engine.api.S | ||
171 | if (e.getCause() instanceof ScriptException) { | 171 | if (e.getCause() instanceof ScriptException) { |
172 | throw (ScriptException)e.getCause(); | 172 | throw (ScriptException)e.getCause(); |
173 | } else { | 173 | } else { |
174 | - throw new ScriptException("Failed to execute js script: " + e.getMessage()); | 174 | + throw new ScriptException(e); |
175 | } | 175 | } |
176 | } catch (Exception e) { | 176 | } catch (Exception e) { |
177 | - throw new ScriptException("Failed to execute js script: " + e.getMessage()); | 177 | + throw new ScriptException(e); |
178 | } | 178 | } |
179 | } | 179 | } |
180 | 180 |