Kill dispatching exceptions #1104

dmdashenkov · 2019-06-28T08:13:24Z

Before this PR, all the errors caused by message dispatching were thrown as exceptions. Exception handling was distributed between various onError methods as well as between different levels of abstraction, from buses to transactions.

Now, all the dispatching errors are converted into system events. Users may choose to subscribe to those events for diagnostics and recovery.

System Events

There are 4 new types of system events:

HandlerFailedUnexpectedly is the most common one. It is emitted at any generic error which occurs during a message handling, including runtime exceptions in handler methods, errors in the framework code, and errors in integrating user code and the framework code. Non-entity constraint violations also lead to HandlerFailedUnexpectedly events (while entity constraint violations cause ConstraintViolated events to be emitted).
RoutingFailed event is emitted when the user code which routes messages to entity instances fails with an exception. The main difference between this event and HandlerFailedUnexpectedly is that the framework does not know the ID of the target entity; thus, the data included in the event payload is a bit different.
CannotDispatchEventTwice and 4. CannotDispatchCommandTwice are events emitted when a duplicate event or command is dispatched to an entity. These events can be emitted at the Inbox level, i.e. before the delivery.

Message Propagation

Previously, when propagating a message through the system, the control flow heavily relied on the exceptions to be thrown when something does wrong. Now, a method dispatching a message would not throw an exception even if dispatching fails. That's why we introduce PropagationOutcome, a structure which aggregates an outcome of a message propagation, successful or otherwise.

A successful propagation may result in events, a rejection, or commands. Those results are posted into the respective buses on the MessageEndpoint level. Previously, commands used to be posted from within the entity level. Now, they are processed side-by-side with the events.

An unsuccessful propagation is described with a spine.core.Error. Errors are converted into system events and posted into the system Event Bus.

When many events are processed in a single transaction, one of them may fail and halt processing. Events which were not sent for processing because another event had failed result in interrupted outcomes.

Some messages may be forced on an entity by measures beyond normal propagation (such as catch-up processes). If an event does not belong to an entity, the entity may ignore it. Such processing is reflected in the PropagationOutcome as the ignored state.

`MessageDispatcher` contract

For quite a long time, MessageDispathcers returned the IDs of the target entities after dispatching a message. Those values were never used in production code and sometimes had to be artificially manufactured (e.g. in abstract message handlers). Now, MessageDispatcher.dispatch does not return anything.

MessageDispatcher.onError was deleted, since all the errors are now broadcast via system Event Bus.

# Conflicts: # license-report.md

codecov · 2019-07-01T09:03:16Z

Codecov Report

Merging #1104 into master will decrease coverage by 1.21%.
The diff coverage is 87.76%.

@@             Coverage Diff              @@
##             master    #1104      +/-   ##
============================================
- Coverage        92%   90.78%   -1.22%     
+ Complexity     3993     3928      -65     
============================================
  Files           539      540       +1     
  Lines         12850    12865      +15     
  Branches        711      723      +12     
============================================
- Hits          11822    11679     -143     
- Misses          807      959     +152     
- Partials        221      227       +6

…ssage handler

dmdashenkov · 2019-07-09T16:27:32Z

@armiol, @alexander-yevsyukov, PTAL.

I'm planning on improving the coverage in further PRs.

armiol · 2019-07-09T18:48:14Z

@dmdashenkov please rephrase this one:

 The events which were not processed because of the premature handling abortion result in the interrupted outcomes.

alexander-yevsyukov

The names CannotDispatchEventTwice and CannotDispatchCommandTwice sounds as if we want dispatch a signal twice but cannot do that because of some reason. Please think of names that reflect the fact that we do not want double delivery.

alexander-yevsyukov · 2019-07-10T07:21:28Z

PropagationOutcome also needs renaming. We already have a good common word for commands and events — Signal. Why don't we use it in this context? How about SignalOutcome or SignalResult, or SignalDispatchingResult?

If we also need to include entity states into this, it could be just DispatchingResult.

“Propagation” is not correct word for this context. We do not want this process be seen as creating more and more messages of the kind.

alexander-yevsyukov

Please see my comments.

alexander-yevsyukov · 2019-07-10T07:30:18Z

server/src/main/java/io/spine/server/BoundedContext.java

@@ -135,7 +131,7 @@ protected BoundedContext(BoundedContextBuilder builder) {
     */
    protected final void init() {
        eventBus.init(this);
-        tenantIndex.registerWith(this);
+        tenantIndex.init(this);


This reads as if TenantIndex initializes the `BoundedContext.

alexander-yevsyukov · 2019-07-10T07:30:39Z

server/src/main/java/io/spine/server/BoundedContext.java

@@ -201,7 +197,7 @@ public static BoundedContextBuilder multitenant(String name) {
    public <I, E extends Entity<I, ?>> void register(Repository<I, E> repository) {
        checkNotNull(repository);
        Security.allowOnlyFrameworkServer();
-        repository.injectContext(this);
+        initialize(repository);


How about just init()?

alexander-yevsyukov · 2019-07-10T07:37:38Z

server/src/main/java/io/spine/server/ContextAware.java

+     * @param context
+     *         the Context to which this instance belongs
+     */
+    void init(BoundedContext context);


Once again, it would be better to name it injectContext(), or register(), or (got forbid) setContext(). It would read better on the usage side.

alexander-yevsyukov · 2019-07-10T07:39:24Z

server/src/main/java/io/spine/server/VisibilityGuard.java

@@ -71,7 +72,7 @@ private void checkNotAlreadyRegistered(Class<? extends Message> stateClass) {
        if (alreadyRegistered != null) {
            throw newIllegalStateException(
                    "A repository for the state class %s already registered: %s",


Please add backticks for the code related parts of the message and the period at the end of the sentence.

…Command|Event]

… `BatchDispatch`

alexander-yevsyukov

Please see my comments.

alexander-yevsyukov · 2019-07-10T13:05:48Z

server/src/main/java/io/spine/server/BoundedContext.java

+     * If the given {@code contextPart} is {@link ContextAware},
+     * {@linkplain ContextAware#registerWith registers} it with this context.
+     */
+    private void registerWithThis(Object contextPart) {


Why not just register()?

alexander-yevsyukov · 2019-07-10T13:09:17Z

server/src/main/java/io/spine/server/VisibilityGuard.java

@@ -71,7 +71,7 @@ private void checkNotAlreadyRegistered(Class<? extends Message> stateClass) {
        RepositoryAccess alreadyRegistered = repositories.get(stateClass);
        if (alreadyRegistered != null) {
            throw newIllegalStateException(
-                    "A repository for the state class %s already registered: %s",
+                    "A repository for the state class %s already registered: `%s`",


Please finish with period.

alexander-yevsyukov · 2019-07-10T13:17:06Z

server/src/main/java/io/spine/server/entity/PropagationProcess.java

@@ -33,7 +33,7 @@



final class PropagationProcess

Don't we need to rename this class too?

alexander-yevsyukov · 2019-07-10T13:23:43Z

server/src/main/proto/spine/server/entity/dispatching.proto

@@ -36,7 +36,7 @@ import "spine/core/event.proto";

 // Report of a batch propagation of several signals.
 //
-message Propagation {
+message BatchDispatch {


Please rename to BatchDispatchOutcome.

alexander-yevsyukov · 2019-07-10T13:24:00Z

server/src/main/proto/spine/server/entity/dispatching.proto

@@ -36,7 +36,7 @@ import "spine/core/event.proto";

 // Report of a batch propagation of several signals.
 //


Please remove the empty line.

alexander-yevsyukov · 2019-07-10T13:41:00Z

server/src/main/java/io/spine/server/event/model/SubscriberMethod.java

-                                      EmptyClass,
-                                      MethodResult<Empty>> {
+                                      EmptyClass>
+        implements NonProducingMethod<EventSubscriber, EventClass, EventEnvelope> {


The name NonProducingMethod is a bit weird. VoidMethod may not be much better, but at least shorter.

alexander-yevsyukov · 2019-07-10T13:44:42Z

server/src/main/java/io/spine/server/model/CommandProducingMethod.java

+import io.spine.core.ActorContext;
+import io.spine.core.Command;
+import io.spine.server.entity.ProducedCommands;
+import io.spine.server.entity.Success;


Success does not seem to belong to entity package. Yes, most of the dispatching end up in entities, but we also have CommandHandler and EventHandler.

server/src/main/java/io/spine/server/model/MethodResult.java

alexander-yevsyukov · 2019-07-10T13:47:21Z

server/src/main/java/io/spine/server/model/NonProducingMethod.java

+                                        MessageEnvelope<?, ?, ?> handledSignal) {
+        if (result != null) {
+            String errorMessage = format(
+                    "Method `%s` should not produce any result. Produced: %s",


Please finish with dot.

alexander-yevsyukov · 2019-07-10T14:00:32Z

server/src/test/java/io/spine/server/model/contexts/tasks/CreationRetry.java

 /**
 * A {@link io.spine.server.command.Commander Commander} which retries the task
 * creation if it fails.
 */
 public final class CreationRetry extends AbstractCommander {

-    private static final Set<TaskId> rejectedTasks = new HashSet<>();
+    private static final Set<TaskId> rejectedTasks = newHashSet();


Google considers these static factory method as deprecated since Java 7.

…pine.server.dispatch`

…ess` to `BatchDispatch`

dmdashenkov · 2019-07-10T15:05:07Z

@alexander-yevsyukov, PTAL again. Let's discuss the leftover comments vocally.

alexander-yevsyukov

LGTM

dmdashenkov added 3 commits June 28, 2019 10:34

Fix the MessageId doc

a265fe2

Add logging when posting system events

eccf1e3

Merge branch 'master' into kill-exceptions

48ad974

# Conflicts: # license-report.md

dmdashenkov self-assigned this Jun 28, 2019

dmdashenkov added 4 commits June 28, 2019 11:42

Revert to a NOP observer

ea3cfc7

Add DUPLICATE_EVENT error code

c245d39

Fix alignment

2ed369c

Define signal propagation result types

adf1ef5

dmdashenkov added 21 commits July 1, 2019 14:22

Introduce PropagationOutcome as a DTO for transferring result of a me…

764b41d

…ssage handler

Fix compilation in tests

33566be

Delete entity unit test framework

b3c179b

Resolve unchecked warnings in BlackBoxBoundedContext

0ea9647

More type parameter improvements

9643679

Remove deprecated API usage

1635791

Post produced events if any

0e434e3

Fix formatting

eff16aa

Fix a mirror projection test

e24a023

Provide a valid Version

e67381e

Fix some of the tests which expect an exception

f01ca8b

Fix tests which check that command handlers return a non-empty result

c54a44d

Define additional diagnostic events

7ae7ff3

Post HandlerFailedUnexpectedly event

d2a1020

Post a system event on a duplicate signal dispatching attempt

4fd6f4e

Emit rejection events

b1a84b5

Include aggregate applier calls in the outcome report [WIP]

66db597

Include aggregate applier calls in the outcome report

1121b56

Update test util tests

e799c54

Clean up DeliveryTest

5994f6c

Dispatch HandlerFailedUnexpectedly events to CommandLog

7862ea2

dmdashenkov marked this pull request as ready for review July 9, 2019 16:26

alexander-yevsyukov requested changes Jul 10, 2019

View reviewed changes

dmdashenkov added 7 commits July 10, 2019 13:15

Rename CannotDispatch[Command|Event]Twice to CannotDispatchDuplicate[…

f51bae5

…Command|Event]

Rename ContextAware.init to registerWith

1da708f

Restore POM

c3a8380

Rename PropagationOutcome to DispatchOutcome and Propagation to…

d776d66

… `BatchDispatch`

Better error message

7ea3149

Fix a typo

212ec05

Simpler name for a parameter

8c3de5d

dmdashenkov requested a review from alexander-yevsyukov July 10, 2019 11:51

alexander-yevsyukov requested changes Jul 10, 2019

View reviewed changes

dmdashenkov added 9 commits July 10, 2019 17:31

Move dispatching definitions from io.spine.server.entity to `io.s…

6e5f27b

…pine.server.dispatch`

Rename registerWithThis into registerIfAware

103875d

Delete usage of Sets.newHashSet()

f644505

Better error messages

441ac8a

Add a dot at the end of an error message

2eaff68

Rename BatchDispatch to BatchDispatchOutcome and `PropagationProc…

6887524

…ess` to `BatchDispatch`

Remove empty lines

6caaa56

Explain why any CommandReceiver is an EventProducer

0d456be

Rename NonProducingMethod to VoidMethod

c840e5d

dmdashenkov requested a review from alexander-yevsyukov July 10, 2019 15:04

alexander-yevsyukov approved these changes Jul 10, 2019

View reviewed changes

dmdashenkov merged commit 37ad9e7 into master Jul 10, 2019

dmdashenkov deleted the kill-exceptions branch July 10, 2019 20:44

dmdashenkov mentioned this pull request Jul 15, 2019

Tests for error propagation #1113

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kill dispatching exceptions #1104

Kill dispatching exceptions #1104

dmdashenkov commented Jun 28, 2019 •

edited

Loading

codecov bot commented Jul 1, 2019 •

edited

Loading

dmdashenkov commented Jul 9, 2019

armiol commented Jul 9, 2019

alexander-yevsyukov left a comment

alexander-yevsyukov commented Jul 10, 2019 •

edited

Loading

alexander-yevsyukov left a comment

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov left a comment

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

alexander-yevsyukov Jul 10, 2019

dmdashenkov commented Jul 10, 2019

alexander-yevsyukov left a comment

		@@ -36,7 +36,7 @@ import "spine/core/event.proto";

		// Report of a batch propagation of several signals.
		//

Kill dispatching exceptions #1104

Kill dispatching exceptions #1104

Conversation

dmdashenkov commented Jun 28, 2019 • edited Loading

System Events

Message Propagation

MessageDispatcher contract

codecov bot commented Jul 1, 2019 • edited Loading

Codecov Report

dmdashenkov commented Jul 9, 2019

armiol commented Jul 9, 2019

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

alexander-yevsyukov commented Jul 10, 2019 • edited Loading

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmdashenkov commented Jul 10, 2019

alexander-yevsyukov left a comment

Choose a reason for hiding this comment

dmdashenkov commented Jun 28, 2019 •

edited

Loading

`MessageDispatcher` contract

codecov bot commented Jul 1, 2019 •

edited

Loading

alexander-yevsyukov commented Jul 10, 2019 •

edited

Loading