Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
88e39d2
sample on how to recursiveley extract errors
denrase Dec 19, 2022
ba00651
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 10, 2023
81394d1
introduce exception cause with exception and stacktrace
denrase Jan 10, 2023
620e917
move extractors to options
denrase Jan 10, 2023
1c7d835
extract exceptions and attach as sentry exceptions to sentry event
denrase Jan 10, 2023
2584bad
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 16, 2023
e8384ca
make internal field private
denrase Jan 16, 2023
b4fb217
make var final
denrase Jan 16, 2023
077aff4
make var final
denrase Jan 16, 2023
a1acbf3
add tests covering stacktrace attachments
denrase Jan 16, 2023
fcd1a38
check if correct stacktrace was captured
denrase Jan 16, 2023
34e9e8a
break circularity
denrase Jan 16, 2023
84f1104
add changelog entry
denrase Jan 16, 2023
0c15cd2
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 17, 2023
413ddd0
expose type in extractor
denrase Jan 17, 2023
4af4d07
export classes
denrase Jan 17, 2023
5159821
move recursivce extractor to exception factory
denrase Jan 17, 2023
983bdf3
format
denrase Jan 17, 2023
bde4df0
remove todo
denrase Jan 17, 2023
4bd8ad4
add throwable to sentry exception
denrase Jan 17, 2023
406e493
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 17, 2023
61900d4
apply new approach in dio event processor
denrase Jan 17, 2023
5d78242
update changelog
denrase Jan 17, 2023
b2abac7
dont remove stacktrace from DioError
denrase Jan 17, 2023
2e65a9e
preserve throwable mechanism
denrase Jan 17, 2023
4f37d19
search for first in list
denrase Jan 23, 2023
72cfb5b
search for dioexception in list
denrase Jan 23, 2023
db64952
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 23, 2023
dcbc09f
format changelog
denrase Jan 23, 2023
4938f3a
Merge branch 'v7.0.0' into feat/exception-extractor
denrase Jan 23, 2023
cce32de
format changelog
denrase Jan 23, 2023
22196e6
format
denrase Jan 23, 2023
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
Prev Previous commit
Next Next commit
apply new approach in dio event processor
  • Loading branch information
denrase committed Jan 17, 2023
commit 61900d406f8b825f203e83c5ec6cfc86c03458fb
16 changes: 16 additions & 0 deletions dio/lib/src/dio_error_extractor.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import 'package:dio/dio.dart';
import 'package:sentry/sentry.dart';

/// Extracts the inner exception and stacktrace from [DioError]
class DioErrorExtractor extends ExceptionCauseExtractor<DioError> {
@override
ExceptionCause? cause(DioError error) {
if (error.stackTrace == null) {
return null;
}
return ExceptionCause(
error.error ?? 'DioError inner stacktrace',
error.stackTrace,
);
}
}
52 changes: 11 additions & 41 deletions dio/lib/src/dio_event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,16 @@ import 'dart:async';

import 'package:dio/dio.dart';
import 'package:sentry/sentry.dart';
// ignore: implementation_imports
import 'package:sentry/src/sentry_exception_factory.dart';

/// This is an [EventProcessor], which improves crash reports of [DioError]s.
/// It adds information about [DioError.requestOptions] if present and also about
/// the inner exceptions.
class DioEventProcessor implements EventProcessor {
// Because of obfuscation, we need to dynamically get the name
static final _dioErrorType = (DioError).toString();

/// This is an [EventProcessor], which improves crash reports of [DioError]s.
DioEventProcessor(this._options);

final SentryOptions _options;

SentryExceptionFactory get _sentryExceptionFactory =>
// ignore: invalid_use_of_internal_member
_options.exceptionFactory;

@override
FutureOr<SentryEvent?> apply(SentryEvent event, {Hint? hint}) {
final dynamic dioError = event.throwable;
Expand All @@ -41,41 +32,20 @@ class DioEventProcessor implements EventProcessor {
contexts: contexts,
);

final innerDioStackTrace = dioError.stackTrace;
final innerDioErrorException = dioError.error as Object?;

// If the inner errors stacktrace is null,
// there's nothing to create chained exception
if (innerDioStackTrace == null) {
// there is no chained exception
if (dioError.stackTrace == null) {
return event;
}

try {
final innerException = _sentryExceptionFactory.getSentryException(
innerDioErrorException ?? 'DioError inner stacktrace',
stackTrace: innerDioStackTrace,
);

final exceptions = _removeDioErrorStackTraceFromValue(
List<SentryException>.from(event.exceptions ?? <SentryException>[]),
dioError,
);

return event.copyWith(
exceptions: [
innerException,
...exceptions,
],
);
} catch (e, stackTrace) {
_options.logger(
SentryLevel.debug,
'Could not convert DioError to SentryException',
exception: e,
stackTrace: stackTrace,
);
}
return event;
final exceptions = _removeDioErrorStackTraceFromValue(
List<SentryException>.from(event.exceptions ?? <SentryException>[]),
dioError,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though it wouldn't look as nice without this code, I think it would greatly simplify the code if we just remove it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no strong opinion here, opted to keep the previous functionality intact.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed it for now. Lets see if @marandaneto also has an opinion here.


return event.copyWith(
exceptions: exceptions.reversed.toList(), // Inner before DioError
);
}

/// Remove the StackTrace from [dioError] so the message on Sentry looks
Expand All @@ -85,7 +55,7 @@ class DioEventProcessor implements EventProcessor {
DioError dioError,
) {
final dioSentryExceptions =
exceptions.where((element) => element.type == _dioErrorType);
exceptions.where((element) => element.throwable is DioError);

if (dioSentryExceptions.isEmpty) {
return exceptions;
Expand Down
4 changes: 4 additions & 0 deletions dio/lib/src/sentry_dio_extension.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:dio/dio.dart';
import 'package:sentry/sentry.dart';
import 'dio_error_extractor.dart';
import 'dio_event_processor.dart';
import 'failed_request_interceptor.dart';
import 'sentry_transformer.dart';
Expand All @@ -19,6 +20,9 @@ extension SentryDioExtension on Dio {
// ignore: invalid_use_of_internal_member
final options = hub.options;

// Add to get inner exception & stacktrace
options.addExceptionCauseExtractor(DioErrorExtractor());
Comment on lines +23 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to do something similar to the lines below here, L27-28, only adding the dio extractor if already not there, just for defense programming, otherwise calling this extension multiple times would add multiple extractors, maybe causing side effects.


// Add DioEventProcessor when it's not already present
if (options.eventProcessors.whereType<DioEventProcessor>().isEmpty) {
options.sdk.addIntegration('sentry_dio');
Expand Down
61 changes: 61 additions & 0 deletions dio/test/dio_error_extractor_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import 'package:dio/dio.dart';
import 'package:sentry_dio/src/dio_error_extractor.dart';
import 'package:test/test.dart';

void main() {
late Fixture fixture;

setUp(() {
fixture = Fixture();
});

test('$DioErrorExtractor extracts error and 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 cause = sut.cause(dioError);

expect(cause?.exception, exception);
expect(cause?.stackTrace, stacktrace);
});

test('$DioErrorExtractor extracts stacktrace only', () {
final sut = fixture.getSut();
final stacktrace = StackTrace.current;

final dioError = DioError(
requestOptions: RequestOptions(path: '/foo/bar'),
)..stackTrace = stacktrace;

final cause = sut.cause(dioError);

expect(cause?.exception, 'DioError inner stacktrace');
expect(cause?.stackTrace, stacktrace);
});

test('$DioErrorExtractor 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 cause = sut.cause(dioError);

expect(cause, isNull);
});
}

class Fixture {
DioErrorExtractor getSut() {
return DioErrorExtractor();
}
}
12 changes: 11 additions & 1 deletion dio/test/dio_event_processor_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:dio/dio.dart';
import 'package:sentry/sentry.dart';
import 'package:sentry_dio/sentry_dio.dart';
import 'package:sentry_dio/src/dio_error_extractor.dart';
import 'package:test/test.dart';
import 'package:sentry/src/sentry_exception_factory.dart';

Expand Down Expand Up @@ -157,16 +158,25 @@ void main() {
});

test('$DioEventProcessor adds chained stacktraces', () {
fixture.options.addExceptionCauseExtractor(DioErrorExtractor());

final sut = fixture.getSut(sendDefaultPii: false);
final exception = Exception('foo bar');
final dioError = DioError(
error: exception,
requestOptions: requestOptions,
)..stackTrace = StackTrace.current;

final extracted =
fixture.exceptionFactory.extractor.flatten(dioError, null);
final exceptions = extracted.map((element) {
return fixture.exceptionFactory.getSentryException(element.exception,
stackTrace: element.stackTrace);
}).toList();

final event = SentryEvent(
throwable: dioError,
exceptions: [fixture.exceptionFactory.getSentryException(dioError)],
exceptions: exceptions,
);

final processedEvent = sut.apply(event) as SentryEvent;
Expand Down