Skip to content

Commit

Permalink
fix: stacktrace in computed (#988)
Browse files Browse the repository at this point in the history
* fix: preserve stacktrace in Computed and Reaction

* tests: Add test cases for testing preserving stacktrace in Computed and Observer

* chore: bump version to 2.3.1 and update changelog with bug fix for preserving stacktrace in Computed and Reaction functions

---------

Co-authored-by: Pavan Podila <[email protected]>
  • Loading branch information
altynbek132 and pavanpodila authored Mar 23, 2024
1 parent fdfb466 commit c4c217b
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 17 deletions.
73 changes: 64 additions & 9 deletions flutter_mobx/test/flutter_mobx_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,34 @@ void main() {
expect(exception, isInstanceOf<MobXCaughtException>());
});

testWidgets(
'Observer should print full stacktrace of error coming from computed',
(tester) async {
late StackTrace stackTrace;
final errorWrapper = await _testThrowingObserverWithStackTrace(
tester,
firstError: true,
actionThrows: (tester) async {
await tester.pumpWidget(
Observer(
builder: (context) {
Computed(() {
try {
throw Exception();
} on Exception catch (e, st) {
stackTrace = st;
rethrow;
}
}).value;
},
),
);
},
);
expect(errorWrapper.stackTrace, stackTrace);
},
);

testWidgets('Observer unmount should dispose Reaction', (tester) async {
final mock = MockReaction();
when(() => mock.hasObservables).thenReturn(true);
Expand Down Expand Up @@ -345,18 +373,45 @@ Future<MobXCaughtException> _testThrowingObserver(
WidgetTester tester,
Object errorToThrow,
) async {
late Object exception;
return (await _testThrowingObserverWithStackTrace(
tester,
actionThrows: (tester) async {
final count = Observable(0);
await tester.pumpWidget(FlutterErrorThrowingObserver(
errorToThrow: errorToThrow,
builder: (context) => Text(count.value.toString()),
));
count.value++;
},
))
.exception as MobXCaughtException;
}

class _ErrorWrapper {
final Object exception;
final StackTrace? stackTrace;

_ErrorWrapper({required this.exception, required this.stackTrace});
}

Future<_ErrorWrapper> _testThrowingObserverWithStackTrace(
WidgetTester tester, {
required Future<void> Function(WidgetTester tester) actionThrows,
bool firstError = false,
}) async {
_ErrorWrapper? errorWrapper;
final prevOnError = FlutterError.onError;
FlutterError.onError = (details) => exception = details.exception;
FlutterError.onError = (details) {
if (firstError && errorWrapper != null) {
return;
}
errorWrapper =
_ErrorWrapper(exception: details.exception, stackTrace: details.stack);
};

try {
final count = Observable(0);
await tester.pumpWidget(FlutterErrorThrowingObserver(
errorToThrow: errorToThrow,
builder: (context) => Text(count.value.toString()),
));
count.value++;
return exception as MobXCaughtException;
await actionThrows(tester);
return errorWrapper!;
} finally {
FlutterError.onError = prevOnError;
}
Expand Down
4 changes: 4 additions & 0 deletions mobx/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.3.1

- Fix preserving stacktrace in Computed and Reaction when exception thrown inside argument function

## 2.3.0+1

- `pubspec.yaml` updated to include homepage and topics
Expand Down
2 changes: 1 addition & 1 deletion mobx/lib/src/core/computed.dart
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Computed<T> extends Atom implements Derivation, ObservableValue<T> {
}

if (_context._hasCaughtException(this)) {
throw _errorValue!;
Error.throwWithStackTrace(_errorValue!, _errorValue!._stackTrace);
}

return _value as T;
Expand Down
13 changes: 8 additions & 5 deletions mobx/lib/src/core/reaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class ReactionImpl with DebugCreationStack implements Reaction {
}

if (_context._hasCaughtException(this)) {
_reportException(_errorValue!);
_reportException(_errorValue!, _errorValue!.stackTrace);
}

if (notify) {
Expand Down Expand Up @@ -120,7 +120,7 @@ class ReactionImpl with DebugCreationStack implements Reaction {
} on Object catch (e, s) {
// Note: "on Object" accounts for both Error and Exception
_errorValue = MobXCaughtException(e, stackTrace: s);
_reportException(_errorValue!);
_reportException(_errorValue!, s);
}
}

Expand Down Expand Up @@ -165,15 +165,18 @@ class ReactionImpl with DebugCreationStack implements Reaction {
// Not applicable right now
}

void _reportException(Object exception) {
void _reportException(Object exception, StackTrace? stackTrace) {
if (_onError != null) {
_onError!(exception, this);
return;
}

if (_context.config.disableErrorBoundaries == true) {
// ignore: only_throw_errors
throw exception;
if (stackTrace != null) {
Error.throwWithStackTrace(exception, stackTrace);
} else {
throw exception;
}
}

if (_context.isSpyEnabled) {
Expand Down
2 changes: 1 addition & 1 deletion mobx/lib/version.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated via set_version.dart. !!!DO NOT MODIFY BY HAND!!!

/// The current version as per `pubspec.yaml`.
const version = '2.3.0+1';
const version = '2.3.1';
2 changes: 1 addition & 1 deletion mobx/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: mobx
version: 2.3.0+1
version: 2.3.1
description: "MobX is a library for reactively managing the state of your applications. Use the power of observables, actions, and reactions to supercharge your Dart and Flutter apps."

repository: https://github.com/mobxjs/mobx.dart
Expand Down
17 changes: 17 additions & 0 deletions mobx/test/exceptions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,21 @@ void main() {
expect(ex.stackTrace, isNotNull);
}
});

test('should preserve stacktrace', () async {
late StackTrace stackTrace;
try {
Computed(() {
try {
throw Exception();
} on Exception catch (e, st) {
stackTrace = st;
rethrow;
}
}).value;
} on MobXCaughtException catch (e, st) {
expect(st, stackTrace);
expect(st, e.stackTrace);
}
});
}

0 comments on commit c4c217b

Please sign in to comment.