Skip to content

Commit

Permalink
chore(core)!: Chain stack traces for state machines
Browse files Browse the repository at this point in the history 
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!
  • Loading branch information
@dnys1
Dillon Nys authored and dnys1 committed Feb 23, 2023
1 parent 48ce72b commit 1fe6e41
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 27 deletions.
1 change: 1 addition & 0 deletions aft.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dependencies:
intl: ">=0.17.0 <1.0.0"
mime: ^1.0.0
pigeon: ^7.1.5
stack_trace: ^1.10.0
uuid: ">=3.0.6 <=3.0.7"
xml: ">=6.1.0 <=6.2.2"

Expand Down
19 changes: 11 additions & 8 deletions packages/amplify_core/lib/src/state_machine/event.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;
}
3 changes: 3 additions & 0 deletions packages/amplify_core/lib/src/state_machine/state.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
26 changes: 17 additions & 9 deletions packages/amplify_core/lib/src/state_machine/state_machine.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -264,16 +265,23 @@ abstract class StateMachine<
_currentState = state;
}

void _emitError(Object error, StackTrace st) {
logger.error('Emitted error', error, st);
/// Emits an [error] and corresponding [stackTrace].
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;
}

Expand All @@ -286,7 +294,7 @@ 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) {
Expand Down Expand Up @@ -334,7 +342,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
Expand Down
1 change: 1 addition & 0 deletions packages/amplify_core/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ dependencies:
logging: ^1.0.0
meta: ^1.7.0
retry: ^3.1.0
stack_trace: ^1.10.0
uuid: ">=3.0.6 <=3.0.7"

dev_dependencies:
Expand Down
35 changes: 31 additions & 4 deletions packages/amplify_core/test/state_machine/my_state_machine.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
91 changes: 85 additions & 6 deletions packages/amplify_core/test/state_machine/state_machine_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -36,12 +37,90 @@ 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) {
// On Web, stack traces are handled differently. While they are still
// chained, method names and types are represented different so there
// isn't much use trying to pin down specifics here.
if (zIsWeb) return isA<StackTrace>();
return 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', () {
Expand Down

0 comments on commit 1fe6e41

Please sign in to comment.