From 564d8181b8352a7844af450c42f72f6be5cf9d00 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 19 Oct 2023 16:17:59 +0200 Subject: [PATCH 1/9] Support string, int, double and bool as throwable for expando --- dart/lib/src/hub.dart | 37 +++++++++++ dart/test/hub_test.dart | 6 +- dart/test/wrapper_exception_test.dart | 92 +++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 2 deletions(-) create mode 100644 dart/test/wrapper_exception_test.dart diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index 2693996d2a..e865ec0416 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -589,6 +589,8 @@ class _WeakMap { final SentryOptions _options; + final exceptionsWrapperUtil = ExceptionWrapperUtil(); + _WeakMap(this._options); void add( @@ -599,6 +601,8 @@ class _WeakMap { if (throwable == null) { return; } + throwable = exceptionsWrapperUtil.wrapIfUnsupportedType(throwable); + _expando[throwable]; try { if (_expando[throwable] == null) { _expando[throwable] = MapEntry(span, transaction); @@ -617,6 +621,7 @@ class _WeakMap { if (throwable == null) { return null; } + throwable = exceptionsWrapperUtil.wrapIfUnsupportedType(throwable); try { return _expando[throwable] as MapEntry?; } catch (exception, stackTrace) { @@ -626,7 +631,39 @@ class _WeakMap { exception: exception, stackTrace: stackTrace, ); + print('hahaa'); } return null; } } + +class ExceptionWrapperUtil { + final _unsupportedTypes = {String, int, double, bool}; + final _unsupportedThrowables = {}; + + /// Wraps the throwable as [_ExceptionWrapper] if it is of an unsupported type. + dynamic wrapIfUnsupportedType(dynamic throwable) { + if (_unsupportedTypes.contains(throwable.runtimeType)) { + throwable = _ExceptionWrapper(Exception(throwable)); + _unsupportedThrowables.add(throwable); + } + return _unsupportedThrowables.lookup(throwable) ?? throwable; + } +} + +class _ExceptionWrapper { + _ExceptionWrapper(this.exception); + + final Exception exception; + + @override + bool operator ==(Object other) { + if (other is _ExceptionWrapper) { + return other.exception.toString() == exception.toString(); + } + return false; + } + + @override + int get hashCode => exception.toString().hashCode; +} diff --git a/dart/test/hub_test.dart b/dart/test/hub_test.dart index 8cc4e99502..38c1497d78 100644 --- a/dart/test/hub_test.dart +++ b/dart/test/hub_test.dart @@ -1,3 +1,5 @@ +import 'dart:ffi'; + import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; @@ -148,8 +150,8 @@ void main() { final capturedEvent = fixture.client.captureEventCalls.first; - expect(capturedEvent.event.transaction, isNull); - expect(capturedEvent.event.contexts.trace, isNull); + expect(capturedEvent.event.transaction, 'test'); + expect(capturedEvent.event.contexts.trace, isNotNull); }); }); diff --git a/dart/test/wrapper_exception_test.dart b/dart/test/wrapper_exception_test.dart new file mode 100644 index 0000000000..79bcca3165 --- /dev/null +++ b/dart/test/wrapper_exception_test.dart @@ -0,0 +1,92 @@ +import 'package:sentry/src/hub.dart'; +import 'package:test/expect.dart'; +import 'package:test/scaffolding.dart'; + +void main() { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + group('unsupported throwable types', () { + test('wrapped string throwable does not throw when expanding', () async { + final exceptionWrapperUtil = fixture.sut; + final unsupportedThrowable = 'test throwable'; + final wrappedThrowable = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + + expect(() { + fixture.expando[wrappedThrowable]; + }, returnsNormally); + }); + + test('wrapped int throwable does not throw when expanding', () async { + final exceptionWrapperUtil = fixture.sut; + final unsupportedThrowable = 1; + final wrappedThrowable = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + + expect(() { + fixture.expando[wrappedThrowable]; + }, returnsNormally); + }); + + test('wrapped double throwable does not throw when expanding', () async { + final exceptionWrapperUtil = fixture.sut; + final unsupportedThrowable = 1.0; + final wrappedThrowable = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + + expect(() { + fixture.expando[wrappedThrowable]; + }, returnsNormally); + }); + + test('wrapped bool throwable does not throw when expanding', () async { + final exceptionWrapperUtil = fixture.sut; + final unsupportedThrowable = true; + final wrappedThrowable = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + + expect(() { + fixture.expando[wrappedThrowable]; + }, returnsNormally); + }); + + test( + 'creating multiple instances of string wrapped exceptions accesses the same expando value', + () async { + final unsupportedThrowable = 'test throwable'; + final exceptionWrapperUtil = fixture.sut; + + final first = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + fixture.expando[first] = 1; + + final second = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + expect(fixture.expando[second], 1); + fixture.expando[second] = 2.0; + + final third = + exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + expect(fixture.expando[third], 2); + }); + }); + + group('supported throwable type', () { + test('does not wrap exception if it is a supported type', () async { + final supportedThrowable = Exception('test throwable'); + final result = fixture.sut.wrapIfUnsupportedType(supportedThrowable); + + expect(result, supportedThrowable); + }); + }); +} + +class Fixture { + final expando = Expando(); + + ExceptionWrapperUtil get sut => ExceptionWrapperUtil(); +} From 7b28d84939c2453ac50f6d7dcf6ea7d656433171 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 19 Oct 2023 16:21:52 +0200 Subject: [PATCH 2/9] Rename the test file and move it to utils folder{ --- .../wrapper_exception_util_test.dart} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename dart/test/{wrapper_exception_test.dart => utils/wrapper_exception_util_test.dart} (100%) diff --git a/dart/test/wrapper_exception_test.dart b/dart/test/utils/wrapper_exception_util_test.dart similarity index 100% rename from dart/test/wrapper_exception_test.dart rename to dart/test/utils/wrapper_exception_util_test.dart From 2f8819c7eb175c05592540434d355b1ab17dc06e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 09:43:11 +0200 Subject: [PATCH 3/9] Remvoe ffi import --- dart/test/hub_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/dart/test/hub_test.dart b/dart/test/hub_test.dart index 38c1497d78..c14239e474 100644 --- a/dart/test/hub_test.dart +++ b/dart/test/hub_test.dart @@ -1,5 +1,3 @@ -import 'dart:ffi'; - import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; From b052f89495d194b9b6f9e45701d7e886a253cc25 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 09:50:06 +0200 Subject: [PATCH 4/9] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11707b1d57..73846c6010 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Unsupported types with Expando ([#1690](https://github.com/getsentry/sentry-dart/pull/1690)) + ### Features - Breadcrumbs for file I/O operations ([#1649](https://github.com/getsentry/sentry-dart/pull/1649)) From 11035e25a9d609082dac5794e09b0cb030d985eb Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 09:57:52 +0200 Subject: [PATCH 5/9] Try other url for dio pubspec doc url --- dio/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dio/pubspec.yaml b/dio/pubspec.yaml index 56e5102e60..ce7a826218 100644 --- a/dio/pubspec.yaml +++ b/dio/pubspec.yaml @@ -4,7 +4,7 @@ version: 7.10.1 homepage: https://docs.sentry.io/platforms/dart/ repository: https://github.com/getsentry/sentry-dart issue_tracker: https://github.com/getsentry/sentry-dart/issues -documentation: https://docs.sentry.io/platforms/dart/configuration/integrations/dio/ +documentation: https://docs.sentry.io/platforms/dart/integrations/dio/ environment: sdk: '>=2.17.0 <4.0.0' From ce89d78c9d13a1f37ccfb9fddfb1815226908c97 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 10:01:07 +0200 Subject: [PATCH 6/9] Change doc url of logging pubspec --- logging/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logging/pubspec.yaml b/logging/pubspec.yaml index a9394bc92c..5deb649563 100644 --- a/logging/pubspec.yaml +++ b/logging/pubspec.yaml @@ -4,7 +4,7 @@ version: 7.10.1 homepage: https://docs.sentry.io/platforms/dart/ repository: https://github.com/getsentry/sentry-dart issue_tracker: https://github.com/getsentry/sentry-dart/issues -documentation: https://docs.sentry.io/platforms/dart/configuration/integrations/logging/ +documentation: https://docs.sentry.io/platforms/dart/integrations/logging/ environment: sdk: '>=2.17.0 <4.0.0' From 707928274da1f9dca8d29df65e8ca2f164cead43 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 10:10:50 +0200 Subject: [PATCH 7/9] Remove unnecessary code --- dart/lib/src/hub.dart | 2 -- dart/test/utils/wrapper_exception_util_test.dart | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index e865ec0416..701e4998d6 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -602,7 +602,6 @@ class _WeakMap { return; } throwable = exceptionsWrapperUtil.wrapIfUnsupportedType(throwable); - _expando[throwable]; try { if (_expando[throwable] == null) { _expando[throwable] = MapEntry(span, transaction); @@ -631,7 +630,6 @@ class _WeakMap { exception: exception, stackTrace: stackTrace, ); - print('hahaa'); } return null; } diff --git a/dart/test/utils/wrapper_exception_util_test.dart b/dart/test/utils/wrapper_exception_util_test.dart index 79bcca3165..579d265d07 100644 --- a/dart/test/utils/wrapper_exception_util_test.dart +++ b/dart/test/utils/wrapper_exception_util_test.dart @@ -71,7 +71,7 @@ void main() { final third = exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); - expect(fixture.expando[third], 2); + expect(fixture.expando[third], 2.0); }); }); From 12ec0723d51f201d16f63ded0afa299ee0bbb966 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 11:58:10 +0200 Subject: [PATCH 8/9] Update naming --- dart/lib/src/hub.dart | 15 ++++++++------- ...test.dart => unsupported_throwables_test.dart} | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) rename dart/test/{utils/wrapper_exception_util_test.dart => unsupported_throwables_test.dart} (97%) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index 701e4998d6..ed586d50df 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -589,7 +589,7 @@ class _WeakMap { final SentryOptions _options; - final exceptionsWrapperUtil = ExceptionWrapperUtil(); + final exceptionsWrapperUtil = UnsupportedThrowablesHandler(); _WeakMap(this._options); @@ -635,28 +635,29 @@ class _WeakMap { } } -class ExceptionWrapperUtil { +/// A handler for unsupported throwables used for Expando. +@visibleForTesting +class UnsupportedThrowablesHandler { final _unsupportedTypes = {String, int, double, bool}; final _unsupportedThrowables = {}; - /// Wraps the throwable as [_ExceptionWrapper] if it is of an unsupported type. dynamic wrapIfUnsupportedType(dynamic throwable) { if (_unsupportedTypes.contains(throwable.runtimeType)) { - throwable = _ExceptionWrapper(Exception(throwable)); + throwable = _UnsupportedExceptionWrapper(Exception(throwable)); _unsupportedThrowables.add(throwable); } return _unsupportedThrowables.lookup(throwable) ?? throwable; } } -class _ExceptionWrapper { - _ExceptionWrapper(this.exception); +class _UnsupportedExceptionWrapper { + _UnsupportedExceptionWrapper(this.exception); final Exception exception; @override bool operator ==(Object other) { - if (other is _ExceptionWrapper) { + if (other is _UnsupportedExceptionWrapper) { return other.exception.toString() == exception.toString(); } return false; diff --git a/dart/test/utils/wrapper_exception_util_test.dart b/dart/test/unsupported_throwables_test.dart similarity index 97% rename from dart/test/utils/wrapper_exception_util_test.dart rename to dart/test/unsupported_throwables_test.dart index 579d265d07..ae13059fd5 100644 --- a/dart/test/utils/wrapper_exception_util_test.dart +++ b/dart/test/unsupported_throwables_test.dart @@ -88,5 +88,5 @@ void main() { class Fixture { final expando = Expando(); - ExceptionWrapperUtil get sut => ExceptionWrapperUtil(); + UnsupportedThrowablesHandler get sut => UnsupportedThrowablesHandler(); } From 8370ad932cbbb2f77121895a3fd563c8eca6815c Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 20 Oct 2023 11:59:59 +0200 Subject: [PATCH 9/9] Rename --- dart/lib/src/hub.dart | 6 +++--- dart/test/unsupported_throwables_test.dart | 24 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index ed586d50df..8beacb7096 100644 --- a/dart/lib/src/hub.dart +++ b/dart/lib/src/hub.dart @@ -589,7 +589,7 @@ class _WeakMap { final SentryOptions _options; - final exceptionsWrapperUtil = UnsupportedThrowablesHandler(); + final throwableHandler = UnsupportedThrowablesHandler(); _WeakMap(this._options); @@ -601,7 +601,7 @@ class _WeakMap { if (throwable == null) { return; } - throwable = exceptionsWrapperUtil.wrapIfUnsupportedType(throwable); + throwable = throwableHandler.wrapIfUnsupportedType(throwable); try { if (_expando[throwable] == null) { _expando[throwable] = MapEntry(span, transaction); @@ -620,7 +620,7 @@ class _WeakMap { if (throwable == null) { return null; } - throwable = exceptionsWrapperUtil.wrapIfUnsupportedType(throwable); + throwable = throwableHandler.wrapIfUnsupportedType(throwable); try { return _expando[throwable] as MapEntry?; } catch (exception, stackTrace) { diff --git a/dart/test/unsupported_throwables_test.dart b/dart/test/unsupported_throwables_test.dart index ae13059fd5..9cac063a34 100644 --- a/dart/test/unsupported_throwables_test.dart +++ b/dart/test/unsupported_throwables_test.dart @@ -11,10 +11,10 @@ void main() { group('unsupported throwable types', () { test('wrapped string throwable does not throw when expanding', () async { - final exceptionWrapperUtil = fixture.sut; + final throwableHandler = fixture.sut; final unsupportedThrowable = 'test throwable'; final wrappedThrowable = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(() { fixture.expando[wrappedThrowable]; @@ -22,10 +22,10 @@ void main() { }); test('wrapped int throwable does not throw when expanding', () async { - final exceptionWrapperUtil = fixture.sut; + final throwableHandler = fixture.sut; final unsupportedThrowable = 1; final wrappedThrowable = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(() { fixture.expando[wrappedThrowable]; @@ -33,10 +33,10 @@ void main() { }); test('wrapped double throwable does not throw when expanding', () async { - final exceptionWrapperUtil = fixture.sut; + final throwableHandler = fixture.sut; final unsupportedThrowable = 1.0; final wrappedThrowable = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(() { fixture.expando[wrappedThrowable]; @@ -44,10 +44,10 @@ void main() { }); test('wrapped bool throwable does not throw when expanding', () async { - final exceptionWrapperUtil = fixture.sut; + final throwableHandler = fixture.sut; final unsupportedThrowable = true; final wrappedThrowable = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(() { fixture.expando[wrappedThrowable]; @@ -58,19 +58,19 @@ void main() { 'creating multiple instances of string wrapped exceptions accesses the same expando value', () async { final unsupportedThrowable = 'test throwable'; - final exceptionWrapperUtil = fixture.sut; + final throwableHandler = fixture.sut; final first = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); fixture.expando[first] = 1; final second = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(fixture.expando[second], 1); fixture.expando[second] = 2.0; final third = - exceptionWrapperUtil.wrapIfUnsupportedType(unsupportedThrowable); + throwableHandler.wrapIfUnsupportedType(unsupportedThrowable); expect(fixture.expando[third], 2.0); }); });