From 1ab55fca819184aaa7066eca84d84f4fbc651348 Mon Sep 17 00:00:00 2001 From: Peter Leibiger Date: Thu, 16 Mar 2023 13:51:37 +0100 Subject: [PATCH] Add a way to extract stacktraces from custom exception types (#1335) --- CHANGELOG.md | 4 ++ dart/lib/sentry.dart | 1 + .../src/exception_stacktrace_extractor.dart | 35 +++++++++++++ dart/lib/src/sentry_exception_factory.dart | 4 ++ dart/lib/src/sentry_options.dart | 18 +++++-- dart/test/sentry_client_test.dart | 39 ++++++++++++++ dart/test/sentry_exception_factory_test.dart | 51 +++++++++++++++++++ dio/lib/src/dio_stacktrace_extractor.dart | 10 ++++ dio/lib/src/sentry_dio_extension.dart | 8 ++- dio/test/dio_stacktrace_extractor_test.dart | 49 ++++++++++++++++++ dio/test/sentry_dio_extension_test.dart | 12 +++++ 11 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 dart/lib/src/exception_stacktrace_extractor.dart create mode 100644 dio/lib/src/dio_stacktrace_extractor.dart create mode 100644 dio/test/dio_stacktrace_extractor_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 24ec666e33..0ccdc6e4c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Exception StackTrace Extractor ([#1335](https://github.com/getsentry/sentry-dart/pull/1335)) + ### Dependencies - Bump Cocoa SDK from v8.0.0 to v8.3.1 ([#1331](https://github.com/getsentry/sentry-dart/pull/1331)) diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart index 70e22a487d..005941073f 100644 --- a/dart/lib/sentry.dart +++ b/dart/lib/sentry.dart @@ -35,6 +35,7 @@ export 'src/type_check_hint.dart'; // exception extraction export 'src/exception_cause_extractor.dart'; export 'src/exception_cause.dart'; +export 'src/exception_stacktrace_extractor.dart'; // Isolates export 'src/sentry_isolate_extension.dart'; export 'src/sentry_isolate.dart'; diff --git a/dart/lib/src/exception_stacktrace_extractor.dart b/dart/lib/src/exception_stacktrace_extractor.dart new file mode 100644 index 0000000000..bf7b785bb6 --- /dev/null +++ b/dart/lib/src/exception_stacktrace_extractor.dart @@ -0,0 +1,35 @@ +import 'protocol.dart'; +import 'sentry_options.dart'; + +/// Sentry handles [Error.stackTrace] by default. For other cases +/// extend this abstract class and return a custom [StackTrace] of your +/// exceptions. +/// +/// Implementing an extractor and providing it through +/// [SentryOptions.addExceptionStackTraceExtractor] will enable the framework to +/// extract the inner stacktrace and add it to [SentryException] when no other +/// stacktrace was provided while capturing the event. +/// +/// For an example on how to use the API refer to dio/DioStackTraceExtractor or the +/// code below: +/// +/// ```dart +/// class ExceptionWithInner { +/// ExceptionWithInner(this.innerException, this.innerStackTrace); +/// Object innerException; +/// dynamic innerStackTrace; +/// } +/// +/// class ExceptionWithInnerStackTraceExtractor extends ExceptionStackTraceExtractor { +/// @override +/// dynamic cause(ExceptionWithInner error) { +/// return error.innerStackTrace; +/// } +/// } +/// +/// options.addExceptionStackTraceExtractor(ExceptionWithInnerStackTraceExtractor()); +/// ``` +abstract class ExceptionStackTraceExtractor { + dynamic stackTrace(T error); + Type get exceptionType => T; +} diff --git a/dart/lib/src/sentry_exception_factory.dart b/dart/lib/src/sentry_exception_factory.dart index f307c464f7..a3494bcd52 100644 --- a/dart/lib/src/sentry_exception_factory.dart +++ b/dart/lib/src/sentry_exception_factory.dart @@ -30,6 +30,10 @@ class SentryExceptionFactory { if (throwable is Error) { stackTrace ??= throwable.stackTrace; } + stackTrace ??= _options + .exceptionStackTraceExtractor(throwable.runtimeType) + ?.stackTrace(throwable); + // throwable.stackTrace is null if its an exception that was never thrown // hence we check again if stackTrace is null and if not, read the current stack trace // but only if attachStacktrace is enabled diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart index e396a28670..5d9b5e8b62 100644 --- a/dart/lib/src/sentry_options.dart +++ b/dart/lib/src/sentry_options.dart @@ -331,16 +331,28 @@ class SentryOptions { /// The default is 3 seconds. Duration? idleTimeout = Duration(seconds: 3); - final _extractorsByType = {}; + final _causeExtractorsByType = {}; + + final _stackTraceExtractorsByType = {}; /// Returns a previously added [ExceptionCauseExtractor] by type ExceptionCauseExtractor? exceptionCauseExtractor(Type type) { - return _extractorsByType[type]; + return _causeExtractorsByType[type]; } /// Adds [ExceptionCauseExtractor] in order to extract inner exceptions void addExceptionCauseExtractor(ExceptionCauseExtractor extractor) { - _extractorsByType[extractor.exceptionType] = extractor; + _causeExtractorsByType[extractor.exceptionType] = extractor; + } + + /// Returns a previously added [ExceptionStackTraceExtractor] by type + ExceptionStackTraceExtractor? exceptionStackTraceExtractor(Type type) { + return _stackTraceExtractorsByType[type]; + } + + /// Adds [ExceptionStackTraceExtractor] in order to extract inner exceptions + void addExceptionStackTraceExtractor(ExceptionStackTraceExtractor extractor) { + _stackTraceExtractorsByType[extractor.exceptionType] = extractor; } /// Changed SDK behaviour when set to true: diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index 776726419b..15b0cc421a 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -291,6 +291,32 @@ void main() { expect(capturedEvent.exceptions?[1].stackTrace!.frames.first.colNo, 9); }); + test('should capture custom stacktrace', () async { + fixture.options.addExceptionStackTraceExtractor( + ExceptionWithStackTraceExtractor(), + ); + + final stackTrace = StackTrace.fromString(''' +#0 baz (file:///pathto/test.dart:50:3) + +#1 bar (file:///pathto/test.dart:46:9) + '''); + + exception = ExceptionWithStackTrace(stackTrace); + + final client = fixture.getSut(attachStacktrace: true); + await client.captureException(exception, stackTrace: null); + + final capturedEnvelope = (fixture.transport).envelopes.first; + final capturedEvent = await eventFromEnvelope(capturedEnvelope); + + expect(capturedEvent.exceptions?[0].stackTrace, isNotNull); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.fileName, + 'test.dart'); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.lineNo, 46); + expect(capturedEvent.exceptions?[0].stackTrace!.frames.first.colNo, 9); + }); + test('should not capture cause stacktrace when attachStacktrace is false', () async { fixture.options.addExceptionCauseExtractor( @@ -1638,3 +1664,16 @@ class ExceptionWithCauseExtractor return ExceptionCause(error.cause, error.stackTrace); } } + +class ExceptionWithStackTrace { + ExceptionWithStackTrace(this.stackTrace); + final StackTrace stackTrace; +} + +class ExceptionWithStackTraceExtractor + extends ExceptionStackTraceExtractor { + @override + StackTrace? stackTrace(ExceptionWithStackTrace error) { + return error.stackTrace; + } +} diff --git a/dart/test/sentry_exception_factory_test.dart b/dart/test/sentry_exception_factory_test.dart index 70d1d30e5f..d5c13c0ba9 100644 --- a/dart/test/sentry_exception_factory_test.dart +++ b/dart/test/sentry_exception_factory_test.dart @@ -73,6 +73,43 @@ void main() { expect(sentryException.stackTrace!.frames.first.fileName, 'test.dart'); }); + test('should extract stackTrace from custom exception', () { + fixture.options + .addExceptionStackTraceExtractor(CustomExceptionStackTraceExtractor()); + + SentryException sentryException; + try { + throw CustomException(StackTrace.fromString(''' +#0 baz (file:///pathto/test.dart:50:3) + +#1 bar (file:///pathto/test.dart:46:9) + ''')); + } catch (err, _) { + sentryException = fixture.getSut().getSentryException( + err, + ); + } + + expect(sentryException.type, 'CustomException'); + expect(sentryException.stackTrace!.frames.first.lineNo, 46); + expect(sentryException.stackTrace!.frames.first.colNo, 9); + expect(sentryException.stackTrace!.frames.first.fileName, 'test.dart'); + }); + + test('should not fail when stackTrace property does not exist', () { + SentryException sentryException; + try { + throw Object(); + } catch (err, _) { + sentryException = fixture.getSut().getSentryException( + err, + ); + } + + expect(sentryException.type, 'Object'); + expect(sentryException.stackTrace, isNotNull); + }); + test('getSentryException with not thrown Error and frames', () { final sentryException = fixture.getSut().getSentryException( CustomError(), @@ -136,6 +173,20 @@ void main() { class CustomError extends Error {} +class CustomException implements Exception { + final StackTrace stackTrace; + + CustomException(this.stackTrace); +} + +class CustomExceptionStackTraceExtractor + extends ExceptionStackTraceExtractor { + @override + StackTrace? stackTrace(CustomException error) { + return error.stackTrace; + } +} + class Fixture { final options = SentryOptions(dsn: fakeDsn); diff --git a/dio/lib/src/dio_stacktrace_extractor.dart b/dio/lib/src/dio_stacktrace_extractor.dart new file mode 100644 index 0000000000..d41221d802 --- /dev/null +++ b/dio/lib/src/dio_stacktrace_extractor.dart @@ -0,0 +1,10 @@ +import 'package:dio/dio.dart'; +import 'package:sentry/sentry.dart'; + +/// Extracts the inner stacktrace from [DioError] +class DioStackTraceExtractor extends ExceptionStackTraceExtractor { + @override + StackTrace? stackTrace(DioError error) { + return error.stackTrace; + } +} diff --git a/dio/lib/src/sentry_dio_extension.dart b/dio/lib/src/sentry_dio_extension.dart index 271447bb12..5d46e43eee 100644 --- a/dio/lib/src/sentry_dio_extension.dart +++ b/dio/lib/src/sentry_dio_extension.dart @@ -2,6 +2,7 @@ import 'package:dio/dio.dart'; import 'package:sentry/sentry.dart'; import 'dio_error_extractor.dart'; import 'dio_event_processor.dart'; +import 'dio_stacktrace_extractor.dart'; import 'failed_request_interceptor.dart'; import 'sentry_transformer.dart'; import 'sentry_dio_client_adapter.dart'; @@ -50,11 +51,16 @@ extension SentryDioExtension on Dio { // ignore: invalid_use_of_internal_member final options = hub.options; - // Add to get inner exception & stacktrace + // Add to get inner exception if (options.exceptionCauseExtractor(DioError) == null) { options.addExceptionCauseExtractor(DioErrorExtractor()); } + // Add to get inner stacktrace + if (options.exceptionStackTraceExtractor(DioError) == null) { + options.addExceptionStackTraceExtractor(DioStackTraceExtractor()); + } + // Add DioEventProcessor when it's not already present if (options.eventProcessors.whereType().isEmpty) { options.sdk.addIntegration('sentry_dio'); diff --git a/dio/test/dio_stacktrace_extractor_test.dart b/dio/test/dio_stacktrace_extractor_test.dart new file mode 100644 index 0000000000..d83ff1381a --- /dev/null +++ b/dio/test/dio_stacktrace_extractor_test.dart @@ -0,0 +1,49 @@ +import 'package:dio/dio.dart'; +import 'package:sentry_dio/src/dio_stacktrace_extractor.dart'; +import 'package:test/test.dart'; + +void main() { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + group(DioStackTraceExtractor, () { + test('extracts stacktrace', () { + final sut = fixture.getSut(); + final exception = Exception('foo bar'); + final stacktrace = StackTrace.current; + + final dioError = DioError( + error: exception, + requestOptions: RequestOptions(path: '/foo/bar'), + stackTrace: stacktrace, + ); + + final result = sut.stackTrace(dioError); + + expect(result, stacktrace); + }); + + test('extracts nothing with missing stacktrace', () { + final sut = fixture.getSut(); + final exception = Exception('foo bar'); + + final dioError = DioError( + error: exception, + requestOptions: RequestOptions(path: '/foo/bar'), + ); + + final result = sut.stackTrace(dioError); + + expect(result, isNull); + }); + }); +} + +class Fixture { + DioStackTraceExtractor getSut() { + return DioStackTraceExtractor(); + } +} diff --git a/dio/test/sentry_dio_extension_test.dart b/dio/test/sentry_dio_extension_test.dart index c87e0b018a..8900d94ac6 100644 --- a/dio/test/sentry_dio_extension_test.dart +++ b/dio/test/sentry_dio_extension_test.dart @@ -1,6 +1,7 @@ import 'package:dio/dio.dart'; import 'package:sentry_dio/sentry_dio.dart'; import 'package:sentry_dio/src/dio_error_extractor.dart'; +import 'package:sentry_dio/src/dio_stacktrace_extractor.dart'; import 'package:sentry_dio/src/sentry_dio_client_adapter.dart'; import 'package:sentry_dio/src/sentry_dio_extension.dart'; import 'package:sentry_dio/src/sentry_transformer.dart'; @@ -69,6 +70,17 @@ void main() { ); }); + test('addSentry adds $DioStackTraceExtractor', () { + final dio = fixture.getSut(); + + dio.addSentry(hub: fixture.hub); + + expect( + fixture.hub.options.exceptionStackTraceExtractor(DioError), + isNotNull, + ); + }); + test('addSentry adds integration to sdk', () { final dio = fixture.getSut();