Skip to content

Commit cd69046

Browse files
author
Zhang Wenhao
committed
<fix>[core]: enforce propagateExceptionTo before lambda handlers
Add assertion in done(Runnable) and error(Consumer<ErrorCode>) to require propagateExceptionTo() being called first. This prevents uncaught exceptions from being silently swallowed when using lambda-style handlers. - Allow first element of asyncBackups to be null for cases where no async backup is needed. propagateExceptionTo will remove all null elements. - Remove redundant isEmpty ternary guards since the new assertion guarantees asyncBackups is non-empty Related: ZSV-11310 Change-Id: I696870737a6f6b79747a63686c73656363686a6a
1 parent 4db6aab commit cd69046

1 file changed

Lines changed: 17 additions & 8 deletions

File tree

core/src/main/java/org/zstack/core/workflow/SimpleFlowChain.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.concurrent.ConcurrentHashMap;
2727
import java.util.function.Consumer;
2828
import java.util.function.Function;
29+
import java.util.stream.Collectors;
2930

3031
import static org.zstack.core.Platform.inerr;
3132

@@ -260,9 +261,11 @@ public SimpleFlowChain error(FlowErrorHandler handler) {
260261

261262
@SuppressWarnings("rawtypes")
262263
public SimpleFlowChain error(Consumer<ErrorCode> handler) {
263-
AsyncBackup firstAsyncBackup = this.asyncBackups.isEmpty() ? null : this.asyncBackups.get(0);
264-
AsyncBackup[] otherAsyncBackups = this.asyncBackups.isEmpty() ? new AsyncBackup[0] :
265-
this.asyncBackups.subList(1, this.asyncBackups.size()).toArray(new AsyncBackup[0]);
264+
DebugUtils.Assert(!asyncBackups.isEmpty(),
265+
"propagateExceptionTo() must be called before error(Consumer<ErrorCode>)."
266+
+ " Call .propagateExceptionTo(completion) or .propagateExceptionTo((AsyncBackup) null) if you don't need async backup");
267+
AsyncBackup firstAsyncBackup = this.asyncBackups.get(0);
268+
AsyncBackup[] otherAsyncBackups = this.asyncBackups.subList(1, this.asyncBackups.size()).toArray(new AsyncBackup[0]);
266269

267270
DebugUtils.Assert(handler != null, "handler of errorHandler should not be null");
268271
return error(new FlowErrorHandler(firstAsyncBackup, otherAsyncBackups) {
@@ -320,12 +323,16 @@ public Map getData() {
320323

321324
public SimpleFlowChain propagateExceptionTo(AsyncBackup... backups) {
322325
DebugUtils.Assert(backups != null, "backups in methods propagateExceptionTo() must be not null");
323-
DebugUtils.Assert(Arrays.stream(backups).noneMatch(Objects::isNull),
324-
"backups in propagateExceptionTo() should not contain null elements");
325326
DebugUtils.Assert(doneHandler == null, "propagateExceptionTo() must be called before SimpleFlowChain.done()");
326327
DebugUtils.Assert(errorHandler == null, "propagateExceptionTo() must be called before SimpleFlowChain.error()");
327328
DebugUtils.Assert(finallyHandler == null, "propagateExceptionTo() must be called before SimpleFlowChain.Finally()");
329+
328330
this.asyncBackups.addAll(Arrays.asList(backups));
331+
this.asyncBackups.removeIf(Objects::isNull);
332+
333+
if (this.asyncBackups.isEmpty()) {
334+
this.asyncBackups.add(null);
335+
}
329336
return this;
330337
}
331338

@@ -338,9 +345,11 @@ public SimpleFlowChain done(FlowDoneHandler handler) {
338345

339346
@SuppressWarnings("rawtypes")
340347
public SimpleFlowChain done(Runnable runnable) {
341-
AsyncBackup firstAsyncBackup = this.asyncBackups.isEmpty() ? null : this.asyncBackups.get(0);
342-
AsyncBackup[] otherAsyncBackups = this.asyncBackups.isEmpty() ? new AsyncBackup[0] :
343-
this.asyncBackups.subList(1, this.asyncBackups.size()).toArray(new AsyncBackup[0]);
348+
DebugUtils.Assert(!asyncBackups.isEmpty(),
349+
"propagateExceptionTo() must be called before done(Runnable)."
350+
+ " Call .propagateExceptionTo(completion) or .propagateExceptionTo((AsyncBackup) null) if you don't need async backup");
351+
AsyncBackup firstAsyncBackup = this.asyncBackups.get(0);
352+
AsyncBackup[] otherAsyncBackups = this.asyncBackups.subList(1, this.asyncBackups.size()).toArray(new AsyncBackup[0]);
344353

345354
DebugUtils.Assert(runnable != null, "runnable of doneHandler should not be null");
346355
return done(new FlowDoneHandler(firstAsyncBackup, otherAsyncBackups) {

0 commit comments

Comments
 (0)