chore(core)!: Chain stack traces for state machines

Errors originating within state machines lose context when thrown since there is an asynchronous gap between adding an event to a state machine and when it eventually gets processed. For example, consider the following exception thrown from a state machine: ``` ERROR | MyStateMachine | Emitted error: Exception #0 MyStateMachine.doWork (file:///Users/nydillon/dev/amplify-dart/packages/amplify_core/test/state_machine/my_state_machine.dart:82:7) <asynchronous suspension> #1 MyStateMachine.resolve (file:///Users/nydillon/dev/amplify-dart/packages/amplify_core/test/state_machine/my_state_machine.dart:100:9) <asynchronous suspension> #2 StateMachine._listenForEvents (package:amplify_core/src/state_machine/state_machine.dart:237:9) <asynchronous suspension> ``` The trace ends at `listenForEvents` in the state machine, which is where the event was popped off the internal queue. But what happened before? With chaining, we get the following which provides the whole picture: ``` ERROR | MyStateMachine | Emitted error: Exception test/state_machine/my_state_machine.dart 82:7 MyStateMachine.doWork test/state_machine/my_state_machine.dart 100:9 MyStateMachine.resolve package:amplify_core/src/state_machine/state_machine.dart 238:9 StateMachine._listenForEvents ===== asynchronous gap =========================== package:amplify_core/src/state_machine/event.dart 41:47 new EventCompleter package:amplify_core/src/state_machine/state_machine.dart 127:23 StateMachineManager.accept test/state_machine/state_machine_test.dart 47:30 main.<fn>.<fn> package:test_api/src/backend/declarer.dart 215:19 Declarer.test.<fn>.<fn> package:test_api/src/backend/declarer.dart 213:7 Declarer.test.<fn> package:test_api/src/backend/invoker.dart 258:9 Invoker._waitForOutstandingCallbacks.<fn> ``` Now we can see that the exception originated due to an event which was created by calling `StateMachineManager.accept` within a test. Much better!
aws-amplify · Feb 22, 2023 · f8150cd · f8150cd
1 parent 1769c90
commit f8150cd
Show file tree

Hide file tree

Showing 7 changed files with 147 additions and 29 deletions.
diff --git a/aft.yaml b/aft.yaml
@@ -16,6 +16,7 @@ dependencies:
   intl: ">=0.17.0 <1.0.0"
   mime: ^1.0.0
   pigeon: ^7.1.5
+  stack_trace: ^1.11.0
   uuid: ">=3.0.6 <=3.0.7"
   xml: ">=6.1.0 <=6.2.2"
 

diff --git a/packages/amplify_core/lib/src/state_machine/event.dart b/packages/amplify_core/lib/src/state_machine/event.dart
@@ -37,11 +37,21 @@ abstract class StateMachineEvent<EventType, StateType>
 class EventCompleter<Event extends StateMachineEvent,
     State extends StateMachineState> {
   /// {@macro amplify_core.event_completer}
-  EventCompleter(this.event);
+  EventCompleter(this.event, [StackTrace? stackTrace])
+      : stackTrace = stackTrace ?? StackTrace.current;
 
   /// The event to dispatch.
   final Event event;
 
+  /// The stack trace from when [event] was created.
+  ///
+  /// When exceptions are raised from within the state machines, the origin of
+  /// the exception should be traceable back to the API called which kicked off
+  /// this event. Since there may be multiple async gaps between the API call
+  /// and a state machine failure, it is necessary to capture the stack trace
+  /// here and chain it with later stack traces.
+  final StackTrace stackTrace;
+
   final Completer<void> _acceptedCompleter = Completer();
   final Completer<State> _completer = Completer();
 
@@ -77,10 +87,3 @@ class EventCompleter<Event extends StateMachineEvent,
     }
   }
 }
-
-/// Mixin functionality for error/failure events of a state machine.
-mixin ErrorEvent<EventType, StateType>
-    on StateMachineEvent<EventType, StateType> {
-  /// The exception which triggered this event.
-  Exception get exception;
-}
diff --git a/packages/amplify_core/lib/src/state_machine/state.dart b/packages/amplify_core/lib/src/state_machine/state.dart
@@ -24,4 +24,7 @@ mixin SuccessState<StateType> on StateMachineState<StateType> {}
 mixin ErrorState<StateType> on StateMachineState<StateType> {
   /// The exception which triggered this state.
   Exception get exception;
+
+  /// The stack trace for [exception].
+  StackTrace get stackTrace;
 }
diff --git a/packages/amplify_core/lib/src/state_machine/state_machine.dart b/packages/amplify_core/lib/src/state_machine/state_machine.dart
@@ -5,6 +5,7 @@ import 'dart:async';
 
 import 'package:amplify_core/amplify_core.dart';
 import 'package:meta/meta.dart';
+import 'package:stack_trace/stack_trace.dart';
 
 /// Interface for dispatching an event to a state machine.
 @optionalTypeArgs
@@ -138,7 +139,7 @@ abstract class StateMachineManager<
     final completer = accept(event);
     final state = await completer.completed;
     if (state is ErrorState) {
-      throw state.exception;
+      Error.throwWithStackTrace(state.exception, state.stackTrace);
     }
     return state as SuccessState;
   }
@@ -167,7 +168,7 @@ abstract class StateMachineManager<
     final completer = dispatch(event);
     final state = await completer.completed;
     if (state is ErrorState) {
-      throw state.exception;
+      Error.throwWithStackTrace(state.exception, state.stackTrace);
     }
     return state as SuccessState;
   }
@@ -236,7 +237,7 @@ abstract class StateMachine<
         // fire before listeners are registered.
         await Future<void>.delayed(Duration.zero, () => resolve(event));
       } on Object catch (error, st) {
-        _emitError(error, st);
+        emitError(error, st);
       } finally {
         completer.complete(_currentState);
       }
@@ -264,16 +265,25 @@ abstract class StateMachine<
     _currentState = state;
   }
 
-  void _emitError(Object error, StackTrace st) {
-    logger.error('Emitted error', error, st);
+  /// Emits an [error] and corresponding [stackTrace].
+  @visibleForOverriding
+  @mustCallSuper
+  void emitError(Object error, StackTrace stackTrace) {
+    // Chain the stack trace of [_currentEvent]'s creation and the state machine
+    // error to create a full picture of the error's lifecycle.
+    final eventTrace = Trace.from(_currentCompleter.stackTrace);
+    final stateMachineTrace = Trace.from(stackTrace);
+    stackTrace = Chain([stateMachineTrace, eventTrace]);
 
-    final resolution = resolveError(error, st);
+    logger.error('Emitted error', error, stackTrace);
+
+    final resolution = resolveError(error, stackTrace);
 
     // Add the error to the state stream if it cannot be resolved to a new
     // state internally.
     if (resolution == null) {
-      _currentCompleter.completeError(error, st);
-      _stateController.addError(error, st);
+      _currentCompleter.completeError(error, stackTrace);
+      _stateController.addError(error, stackTrace);
       return;
     }
 
@@ -286,11 +296,11 @@ abstract class StateMachine<
     final precondError = event.checkPrecondition(currentState);
     if (precondError != null) {
       logger.debug(
-        'Precondition not met for event ($event):\n'
+        'Precondition not met for event ($_currentEvent):\n'
         '${precondError.precondition}',
       );
       if (precondError.shouldEmit) {
-        _emitError(precondError, StackTrace.current);
+        emitError(precondError, StackTrace.current);
       }
       return false;
     }
@@ -334,7 +344,7 @@ abstract class StateMachine<
   ///
   /// If the error cannot be resolved, return `null` and the error will be
   /// rethrown.
-  State? resolveError(Object error, [StackTrace? st]);
+  State? resolveError(Object error, StackTrace st);
 
   /// Logger for the state machine.
   @override

diff --git a/packages/amplify_core/pubspec.yaml b/packages/amplify_core/pubspec.yaml
@@ -18,6 +18,7 @@ dependencies:
   logging: ^1.0.0
   meta: ^1.7.0
   retry: ^3.1.0
+  stack_trace: ^1.11.0
   uuid: ">=3.0.6 <=3.0.7"
 
 dev_dependencies:

diff --git a/packages/amplify_core/test/state_machine/my_state_machine.dart b/packages/amplify_core/test/state_machine/my_state_machine.dart
@@ -56,6 +56,16 @@ class MyState extends StateMachineState<MyType> {
   String get runtimeTypeName => 'MyState';
 }
 
+class MyErrorState extends MyState with ErrorState {
+  const MyErrorState(this.exception, this.stackTrace) : super(MyType.error);
+
+  @override
+  final Exception exception;
+
+  @override
+  final StackTrace stackTrace;
+}
+
 class MyStateMachine extends StateMachine<MyEvent, MyState, StateMachineEvent,
     StateMachineState, MyStateMachineManager> {
   MyStateMachine(MyStateMachineManager manager) : super(manager, type);
@@ -96,8 +106,11 @@ class MyStateMachine extends StateMachine<MyEvent, MyState, StateMachineEvent,
   }
 
   @override
-  MyState? resolveError(Object error, [StackTrace? st]) {
-    return const MyState(MyType.error);
+  MyState? resolveError(Object error, StackTrace st) {
+    if (error is Exception) {
+      return MyErrorState(error, st);
+    }
+    return null;
   }
 
   @override
@@ -140,6 +153,17 @@ class WorkerState extends StateMachineState<WorkType> {
   String get runtimeTypeName => 'WorkerState';
 }
 
+class WorkerErrorState extends WorkerState with ErrorState {
+  const WorkerErrorState(this.exception, this.stackTrace)
+      : super(WorkType.error);
+
+  @override
+  final Exception exception;
+
+  @override
+  final StackTrace stackTrace;
+}
+
 class WorkerMachine extends StateMachine<WorkerEvent, WorkerState,
     StateMachineEvent, StateMachineState, MyStateMachineManager> {
   WorkerMachine(MyStateMachineManager manager) : super(manager, type);
@@ -171,8 +195,11 @@ class WorkerMachine extends StateMachine<WorkerEvent, WorkerState,
   }
 
   @override
-  WorkerState? resolveError(Object error, [StackTrace? st]) {
-    return const WorkerState(WorkType.error);
+  WorkerState? resolveError(Object error, StackTrace st) {
+    if (error is Exception) {
+      return WorkerErrorState(error, st);
+    }
+    return null;
   }
 
   @override

diff --git a/packages/amplify_core/test/state_machine/state_machine_test.dart b/packages/amplify_core/test/state_machine/state_machine_test.dart
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import 'package:amplify_core/amplify_core.dart';
+import 'package:stack_trace/stack_trace.dart';
 import 'package:test/test.dart';
 
 import 'my_state_machine.dart';
@@ -36,12 +37,84 @@ void main() {
       );
     });
 
-    test('handles errors', () {
-      stateMachine.accept(const MyEvent(MyType.tryWork));
-      expect(
-        stateMachine.stream,
-        emitsThrough(const MyState(MyType.error)),
-      );
+    group('handles errors', () {
+      /// Creates a matcher originating at StateMachineManager.[method].
+      ///
+      /// The state machine should chain stack traces such that both the
+      /// creation of the event and the exception are visible in the same trace.
+      Matcher matchesChain(String method) => isA<Chain>().having(
+            (chain) => chain.traces.map((trace) => trace.toString()),
+            'traces',
+            containsAllInOrder([
+              contains('MyStateMachine.doWork'),
+              contains('StateMachineManager.$method'),
+            ]),
+          );
+
+      test('emitted from stream', () {
+        expect(
+          stateMachine.stream,
+          emitsThrough(
+            isA<MyErrorState>().having(
+              (s) => s.stackTrace,
+              'stackTrace',
+              matchesChain('accept'),
+            ),
+          ),
+        );
+        stateMachine.accept(const MyEvent(MyType.tryWork));
+      });
+
+      test('accept', () async {
+        final completion =
+            await stateMachine.accept(const MyEvent(MyType.tryWork)).completed;
+        expect(
+          completion,
+          isA<MyErrorState>().having(
+            (s) => s.stackTrace,
+            'stackTrace',
+            matchesChain('accept'),
+          ),
+        );
+      });
+
+      test('acceptAndComplete', () async {
+        try {
+          await stateMachine.acceptAndComplete(const MyEvent(MyType.tryWork));
+          fail(
+            'acceptAndComplete should rethrow the exception from the '
+            'state machine',
+          );
+        } on Exception catch (_, st) {
+          expect(st, matchesChain('acceptAndComplete'));
+        }
+      });
+
+      test('dispatch', () async {
+        final completion = await stateMachine
+            .dispatch(const MyEvent(MyType.tryWork))
+            .completed;
+        expect(
+          completion,
+          isA<MyErrorState>().having(
+            (s) => s.stackTrace,
+            'stackTrace',
+            matchesChain('dispatch'),
+          ),
+        );
+      });
+
+      test('dispatchAndComplete', () async {
+        try {
+          await stateMachine.dispatchAndComplete(const MyEvent(MyType.tryWork));
+          fail(
+            'dispatchAndComplete should rethrow the exception from the '
+            'state machine',
+          );
+        } on Exception catch (_, st) {
+          expect(st, matchesChain('dispatchAndComplete'));
+        }
+      });
     });
 
     group('subscribeTo', () {