Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- Feat: Use `Hint` for screenshots ([#1165](https://github.com/getsentry/sentry-dart/pull/1165))
- Feat: Support custom units for custom measurements ([#1181](https://github.com/getsentry/sentry-dart/pull/1181))

### Enhancements

- Exceptions not handled by user code should be `unhandled: true` #1186 ([#1186](https://github.com/getsentry/sentry-dart/pull/1186))

### Fixes

- Fix: Remove `SentryOptions` related parameters from classes which also take `Hub` as a parameter (#816)
Expand Down
5 changes: 3 additions & 2 deletions dart/lib/src/isolate_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ Future<void> handleIsolateError(
stackTrace: stackTrace == null ? null : StackTrace.fromString(stackTrace),
);

// Isolate errors don't crash the App.
final mechanism = Mechanism(type: 'isolateError', handled: true);
// Isolate errors don't crash the app, but are set to handled false, as
// users did not handle them manually
final mechanism = Mechanism(type: 'isolateError', handled: false);
final throwableMechanism = ThrowableMechanism(mechanism, throwable);
final event = SentryEvent(
throwable: throwableMechanism,
Expand Down
5 changes: 3 additions & 2 deletions dart/lib/src/run_zoned_guarded_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ class RunZonedGuardedIntegration extends Integration {
stackTrace: stackTrace,
);

// runZonedGuarded doesn't crash the App.
final mechanism = Mechanism(type: 'runZonedGuarded', handled: true);
// runZonedGuarded doesn't crash the App, but since it is not handled by
// users manually, it is marked as not handled
final mechanism = Mechanism(type: 'runZonedGuarded', handled: false);
final throwableMechanism = ThrowableMechanism(mechanism, exception);

final event = SentryEvent(
Expand Down
24 changes: 13 additions & 11 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,17 +399,19 @@ class SentryClient {
}

SentryEvent _eventWithRemovedBreadcrumbsIfHandled(SentryEvent event) {
final mechanisms =
(event.exceptions ?? []).map((e) => e.mechanism).whereType<Mechanism>();
final hasNoMechanism = mechanisms.isEmpty;
final hasOnlyHandledMechanism =
mechanisms.every((e) => (e.handled ?? true));

if (hasNoMechanism || hasOnlyHandledMechanism) {
return event.copyWith(breadcrumbs: []);
} else {
return event;
}
// final mechanisms = (event.exceptions ?? [])
// .map((e) => e.mechanism)
// .whereType<Mechanism>();
// final hasNoMechanism = mechanisms.isEmpty;

// final hasOnlyHandledMechanism = mechanisms
// .every((e) => (e.handled ?? true));

// if (hasNoMechanism || hasOnlyHandledMechanism) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marandaneto Is this final or are you still in implementation process?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just testing and found out that this has to be fixed, hence the comments.

return event.copyWith(breadcrumbs: []);
// } else {
// return event;
// }
}

Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ void main() {
);
expect(event.exceptions?.first.mechanism?.type, 'OSError');
expect(event.exceptions?.first.mechanism?.meta['errno']['number'], 54);
expect(event.exceptions?.first.mechanism?.handled, isNull);
});

test('adds OSError SentryException for $FileSystemException', () async {
Expand All @@ -92,6 +93,7 @@ void main() {
);
expect(event.exceptions?.first.mechanism?.type, 'OSError');
expect(event.exceptions?.first.mechanism?.meta['errno']['number'], 42);
expect(event.exceptions?.first.mechanism?.handled, isNull);
});
});
}
Expand Down
2 changes: 2 additions & 0 deletions dart/test/http_client/failed_request_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ void main() {
expect(exception?.stackTrace, isNotNull);
expect(exception?.stackTrace!.snapshot, isNull);
expect(mechanism?.type, 'SentryHttpClient');
expect(mechanism?.handled, isNull);

final request = eventCall.request;
expect(request, isNotNull);
Expand Down Expand Up @@ -107,6 +108,7 @@ void main() {
mechanism?.description,
'HTTP Client Error with status code: 404',
);
expect(mechanism?.handled, isNull);

expect(exception?.type, 'SentryHttpClientError');
expect(
Expand Down
1 change: 1 addition & 0 deletions dio/test/failed_request_interceptor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ void main() {
final throwable =
fixture.hub.captureExceptionCalls.first.throwable as ThrowableMechanism;
expect(throwable.mechanism.type, 'SentryDioClientAdapter');
expect(throwable.mechanism.handled, isNull);
expect(throwable.throwable, error);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class FlutterErrorIntegration extends Integration<SentryFlutterOptions> {
// FlutterError doesn't crash the App.
final mechanism = Mechanism(
type: 'FlutterError',
handled: true,
handled: false,
);
final throwableMechanism = ThrowableMechanism(mechanism, exception);

Expand Down
5 changes: 5 additions & 0 deletions flutter/lib/src/integrations/load_contexts_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ class _LoadContextsIntegrationEventProcessor extends EventProcessor {
final breadcrumbsList = infos['breadcrumbs'] as List?;
if (breadcrumbsList != null && breadcrumbsList.isNotEmpty) {
final breadcrumbs = event.breadcrumbs ?? [];
if ((_options.platformChecker.platform.isIOS ||
_options.platformChecker.platform.isMacOS) &&
_options.enableScopeSync) {
breadcrumbs.clear();
}
final newBreadcrumbs =
List<Map<dynamic, dynamic>>.from(breadcrumbsList);

Expand Down
7 changes: 4 additions & 3 deletions flutter/lib/src/integrations/on_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,16 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
_defaultOnError = wrapper.onError;

_integrationOnError = (Object exception, StackTrace stackTrace) {
final handled = _defaultOnError?.call(exception, stackTrace) ?? true;
final handledToReturn =
_defaultOnError?.call(exception, stackTrace) ?? false;

// As per docs, the app might crash on some platforms
// after this is called.
// https://master-api.flutter.dev/flutter/dart-ui/PlatformDispatcher/onError.html
// https://master-api.flutter.dev/flutter/dart-ui/ErrorCallback.html
final mechanism = Mechanism(
type: 'PlatformDispatcher.onError',
handled: handled,
handled: false,
);
final throwableMechanism = ThrowableMechanism(mechanism, exception);

Expand All @@ -63,7 +64,7 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
// unawaited future
hub.captureEvent(event, stackTrace: stackTrace);

return handled;
return handledToReturn;
};

wrapper.onError = _integrationOnError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ void main() {

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.type, 'FlutterError');
expect(throwableMechanism.mechanism.handled, true);
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
expect(throwableMechanism.throwable, exception);

expect(event.contexts['flutter_error_details']['library'], 'sentry');
Expand Down Expand Up @@ -91,7 +92,8 @@ void main() {

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.type, 'FlutterError');
expect(throwableMechanism.mechanism.handled, true);
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);

expect(event.contexts['flutter_error_details']['library'], 'sentry');
expect(event.contexts['flutter_error_details']['context'],
Expand All @@ -115,7 +117,8 @@ void main() {

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.type, 'FlutterError');
expect(throwableMechanism.mechanism.handled, true);
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
expect(throwableMechanism.mechanism.data['hint'], isNull);

expect(event.contexts['flutter_error_details'], isNull);
Expand Down
39 changes: 32 additions & 7 deletions flutter/test/integrations/on_error_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ void main() {
required Object exception,
required StackTrace stackTrace,
ErrorCallback? handler,
bool useFallbackHandler = true,
}) {
fixture.platformDispatcherWrapper.onError = handler ??
(_, __) {
return fixture.onErrorReturnValue;
};
if (handler != null) {
fixture.platformDispatcherWrapper.onError = handler;
} else if (useFallbackHandler) {
fixture.platformDispatcherWrapper.onError = (_, __) {
return fixture.onErrorReturnValue;
};
}

when(fixture.hub.captureEvent(captureAny,
stackTrace: captureAnyNamed('stackTrace')))
Expand Down Expand Up @@ -53,11 +57,30 @@ void main() {

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.type, 'PlatformDispatcher.onError');
expect(throwableMechanism.mechanism.handled, true);
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
expect(throwableMechanism.throwable, exception);
});

test('onError: handled is true if onError returns true', () async {
test('onError: no handler marks event as not handled', () async {
final exception = StateError('error');

_reportError(
exception: exception,
stackTrace: StackTrace.current,
useFallbackHandler: false);

final event = verify(
await fixture.hub
.captureEvent(captureAny, stackTrace: captureAnyNamed('stackTrace')),
).captured.first as SentryEvent;

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
});

test('onError: handled is false if onError returns true', () async {
fixture.onErrorReturnValue = true;
final exception = StateError('error');
_reportError(exception: exception, stackTrace: StackTrace.current);
Expand All @@ -68,7 +91,8 @@ void main() {
).captured.first as SentryEvent;

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.handled, true);
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
});

test('onError: handled is false if onError returns false', () async {
Expand All @@ -82,6 +106,7 @@ void main() {
).captured.first as SentryEvent;

final throwableMechanism = event.throwableMechanism as ThrowableMechanism;
expect(throwableMechanism.mechanism.handled, isNotNull);
expect(throwableMechanism.mechanism.handled, false);
});

Expand Down