Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add options.attachStacktrace #160

Merged
merged 25 commits into from
Nov 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
902dd1d
add options.attachStacktrace
rxlabz Nov 10, 2020
e1f347d
fix case
rxlabz Nov 10, 2020
07048d5
add a _exceptionFactory.stacktraceFactory getter
rxlabz Nov 10, 2020
a61f760
- auto-attach stacktrace to exception only if options.attachStackTrace
rxlabz Nov 10, 2020
0a5addb
fix test
rxlabz Nov 10, 2020
f7f2458
add client._stackTraceFactory
rxlabz Nov 10, 2020
a526771
fix import
rxlabz Nov 10, 2020
3c6cd02
attach stacktrace only if frames is not empty
rxlabz Nov 10, 2020
65c4d2b
attachStackTrace doc
rxlabz Nov 10, 2020
6d349fc
attachStackTrace only if event.stacktrace is null
rxlabz Nov 10, 2020
23738c2
init factories in client.ctor
rxlabz Nov 10, 2020
cbb7be8
feedback
rxlabz Nov 10, 2020
ee39da9
required SentryExceptionFactory.stacktraceFactory
rxlabz Nov 10, 2020
d901e26
type SentryEvent.stackTrace as SentryStackTrace
rxlabz Nov 11, 2020
490d3b7
serialize exception or stacktrace
rxlabz Nov 11, 2020
75d69a6
feedback
rxlabz Nov 11, 2020
b0c4a8e
feedbacks
rxlabz Nov 11, 2020
a027fe8
attach stacktrace only if stacktrace is null
rxlabz Nov 11, 2020
b92f661
event.stacktrace only if event.throwable AND event.exception are null
rxlabz Nov 11, 2020
7f4d18e
remove null conditional instanciation in exceptionFactory
rxlabz Nov 11, 2020
b3f546d
attach the passed stacktrace if attachStackTrace is true
rxlabz Nov 11, 2020
67c9ae8
fix & refactor client._prepareEvent
rxlabz Nov 11, 2020
c65800e
add stacktrace and exception.stacktrace tests
rxlabz Nov 11, 2020
ac1ee38
feedbacks
rxlabz Nov 11, 2020
3077703
fix frame.uri.pathSegments.isNotEmpty
rxlabz Nov 12, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- feat: add missing protocol classes
- fix: logger method and refactoring little things
- fix: sentry protocol is v7
- feat: add an attachStackTrace options

## 4.0.0-alpha.1

Expand Down
2 changes: 1 addition & 1 deletion dart/example_web/web/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ void captureException() async {
}

Future<void> captureCompleteExampleEvent() async {
print('\nReporting a complete event example: ${sdkName}');
final sentryId = await Sentry.captureEvent(event);

print('\nReporting a complete event example: ${sdkName}');
print('Response SentryId: ${sentryId}');

if (sentryId != SentryId.empty()) {
Expand Down
7 changes: 6 additions & 1 deletion dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ class Hub {
SentryId get lastEventId => _lastEventId;

/// Captures the event.
Future<SentryId> captureEvent(SentryEvent event, {dynamic hint}) async {
Future<SentryId> captureEvent(
SentryEvent event, {
dynamic stackTrace,
dynamic hint,
}) async {
var sentryId = SentryId.empty();

if (!_isEnabled) {
Expand All @@ -73,6 +77,7 @@ class Hub {
try {
sentryId = await item.client.captureEvent(
event,
stackTrace: stackTrace,
Copy link
Member

Choose a reason for hiding this comment

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

What about we make the stackTrace part of the hint?
The idea of hint was to have a single .. hint to deal with these extra things without having to expand the public API

Copy link
Contributor

@marandaneto marandaneto Nov 11, 2020

Choose a reason for hiding this comment

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

the hint still could be used for other stuff as we do on Android, but stackTrace on dart is captured separately so it makes sense IMO to have this expanded API.
on Java, a stack trace is always part of the exception, which is not this case.
The other way would be adding a new field (that is also called stackTrace or sentryStackTrace) to the event, which is confusing.

scope: item.scope,
hint: hint,
);
Expand Down
6 changes: 5 additions & 1 deletion dart/lib/src/hub_adapter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ class HubAdapter implements Hub {
void bindClient(SentryClient client) => Sentry.bindClient(client);

@override
Future<SentryId> captureEvent(SentryEvent event, {dynamic hint}) =>
Future<SentryId> captureEvent(
SentryEvent event, {
dynamic stackTrace,
dynamic hint,
}) =>
Sentry.captureEvent(event, hint: hint);

@override
Expand Down
6 changes: 5 additions & 1 deletion dart/lib/src/noop_hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ class NoOpHub implements Hub {
void bindClient(SentryClient client) {}

@override
Future<SentryId> captureEvent(SentryEvent event, {dynamic hint}) =>
Future<SentryId> captureEvent(
SentryEvent event, {
dynamic stackTrace,
dynamic hint,
}) =>
Future.value(SentryId.empty());

@override
Expand Down
1 change: 1 addition & 0 deletions dart/lib/src/noop_sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class NoOpSentryClient implements SentryClient {
@override
Future<SentryId> captureEvent(
SentryEvent event, {
dynamic stackTrace,
Scope scope,
dynamic hint,
}) =>
Expand Down
18 changes: 14 additions & 4 deletions dart/lib/src/protocol/sentry_event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,9 @@ class SentryEvent {
/// If this behavior is undesirable, consider using a custom formatted [message] instead.
final dynamic throwable;

/// The stack trace corresponding to the thrown [exception].
///
/// Can be `null`, a [String], or a [StackTrace].
final dynamic stackTrace;
/// an optional attached StackTrace
/// used when event has no throwable or exception, see [SentryOptions.attachStackTrace]
final SentryStackTrace stackTrace;

/// an exception or error that occurred in a program
/// TODO more doc
Expand Down Expand Up @@ -262,6 +261,17 @@ class SentryEvent {
json['exception'] = {
'values': [exception.toJson()].toList(growable: false)
};
} else if (stackTrace != null) {
json['threads'] = {
'values': [
{
'id': 0,
'stacktrace': stackTrace.toJson(),
'crashed': true,
'name': 'Current Isolate',
}
]
};
}

if (level != null) {
Expand Down
8 changes: 4 additions & 4 deletions dart/lib/src/protocol/sentry_exception.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class SentryException {
final String module;

/// An optional stack trace object
final SentryStackTrace stacktrace;
final SentryStackTrace stackTrace;

/// An optional object describing the [Mechanism] that created this exception
final Mechanism mechanism;
Expand All @@ -26,7 +26,7 @@ class SentryException {
@required this.type,
@required this.value,
this.module,
this.stacktrace,
this.stackTrace,
this.mechanism,
this.threadId,
});
Expand All @@ -46,8 +46,8 @@ class SentryException {
json['module'] = module;
}

if (stacktrace != null) {
json['stacktrace'] = stacktrace.toJson();
if (stackTrace != null) {
json['stacktrace'] = stackTrace.toJson();
}

if (mechanism != null) {
Expand Down
3 changes: 2 additions & 1 deletion dart/lib/src/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ class Sentry {
/// Reports an [event] to Sentry.io.
static Future<SentryId> captureEvent(
SentryEvent event, {
dynamic stackTrace,
dynamic hint,
}) async =>
currentHub.captureEvent(event, hint: hint);
currentHub.captureEvent(event, stackTrace: stackTrace, hint: hint);

/// Reports the [throwable] and optionally its [stackTrace] to Sentry.io.
static Future<SentryId> captureException(
Expand Down
63 changes: 44 additions & 19 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,23 @@ import 'protocol.dart';
import 'scope.dart';
import 'sentry_exception_factory.dart';
import 'sentry_options.dart';
import 'sentry_stack_trace_factory.dart';
import 'transport/http_transport.dart';
import 'transport/noop_transport.dart';
import 'version.dart';

/// Logs crash reports and events to the Sentry.io service.
class SentryClient {
final SentryOptions _options;

final Random _random;

static final _sentryId = Future.value(SentryId.empty());

SentryExceptionFactory _exceptionFactory;

SentryStackTraceFactory _stackTraceFactory;

/// Instantiates a client using [SentryOptions]
factory SentryClient(SentryOptions options) {
if (options == null) {
Expand All @@ -20,27 +31,25 @@ class SentryClient {
if (options.transport is NoOpTransport) {
options.transport = HttpTransport(options);
}

return SentryClient._(options);
}

final SentryOptions _options;

final Random _random;

final SentryExceptionFactory _exceptionFactory;

static final _sentryId = Future.value(SentryId.empty());

/// Instantiates a client using [SentryOptions]
SentryClient._(this._options, {SentryExceptionFactory exceptionFactory})
: _exceptionFactory =
exceptionFactory ?? SentryExceptionFactory(options: _options),
_random = _options.sampleRate == null ? null : Random();
SentryClient._(this._options)
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
: _random = _options.sampleRate == null ? null : Random() {
_stackTraceFactory = SentryStackTraceFactory(_options);
_exceptionFactory = SentryExceptionFactory(
options: _options,
stacktraceFactory: _stackTraceFactory,
);
}

/// Reports an [event] to Sentry.io.
Future<SentryId> captureEvent(
SentryEvent event, {
Scope scope,
dynamic stackTrace,
dynamic hint,
}) async {
event = _processEvent(event, eventProcessors: _options.eventProcessors);
Expand All @@ -61,7 +70,7 @@ class SentryClient {
return _sentryId;
}

event = _prepareEvent(event);
event = _prepareEvent(event, stackTrace: stackTrace);

if (_options.beforeSend != null) {
try {
Expand All @@ -81,7 +90,7 @@ class SentryClient {
return _options.transport.send(event);
}

SentryEvent _prepareEvent(SentryEvent event) {
SentryEvent _prepareEvent(SentryEvent event, {dynamic stackTrace}) {
event = event.copyWith(
serverName: event.serverName ?? _options.serverName,
dist: event.dist ?? _options.dist,
Expand All @@ -92,11 +101,22 @@ class SentryClient {
platform: event.platform ?? sdkPlatform,
);

if (event.throwable != null && event.exception == null) {
if (event.exception != null) return event;

if (event.throwable != null) {
final sentryException = _exceptionFactory
.getSentryException(event.throwable, stackTrace: event.stackTrace);
.getSentryException(event.throwable, stackTrace: stackTrace);

return event.copyWith(exception: sentryException);
}

event = event.copyWith(exception: sentryException);
if (stackTrace != null || _options.attachStackTrace) {
stackTrace ??= StackTrace.current;
final frames = _stackTraceFactory.getStackFrames(stackTrace);

if (frames != null && frames.isNotEmpty) {
event = event.copyWith(stackTrace: SentryStackTrace(frames: frames));
}
}

return event;
Expand All @@ -111,10 +131,15 @@ class SentryClient {
}) {
final event = SentryEvent(
throwable: throwable,
stackTrace: stackTrace,
timestamp: _options.clock(),
);
return captureEvent(event, scope: scope, hint: hint);

return captureEvent(
event,
stackTrace: stackTrace,
scope: scope,
hint: hint,
);
}

/// Reports the [template]
Expand Down
38 changes: 24 additions & 14 deletions dart/lib/src/sentry_exception_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,49 @@ import 'sentry_stack_trace_factory.dart';

/// class to convert Dart Error and exception to SentryException
class SentryExceptionFactory {
SentryStackTraceFactory _stacktraceFactory;
final SentryOptions _options;

final SentryStackTraceFactory _stacktraceFactory;

SentryExceptionFactory({
SentryStackTraceFactory stacktraceFactory,
@required SentryStackTraceFactory stacktraceFactory,
@required SentryOptions options,
}) {
if (options == null) {
}) : _options = options,
_stacktraceFactory = stacktraceFactory {
if (_options == null) {
throw ArgumentError('SentryOptions is required.');
}

_stacktraceFactory = stacktraceFactory ?? SentryStackTraceFactory(options);
if (_stacktraceFactory == null) {
throw ArgumentError('SentryStackTraceFactory is required.');
}
}

SentryException getSentryException(
dynamic exception, {
dynamic stackTrace,
Mechanism mechanism,
}) {
if (exception is Error) {
stackTrace ??= exception.stackTrace;
} else {
stackTrace ??= StackTrace.current;
SentryStackTrace sentryStackTrace;
if (stackTrace != null) {
sentryStackTrace = SentryStackTrace(
frames: _stacktraceFactory.getStackFrames(stackTrace),
);
} else if (exception is Error && exception.stackTrace != null) {
sentryStackTrace = SentryStackTrace(
frames: _stacktraceFactory.getStackFrames(exception.stackTrace),
);
} else if (_options.attachStackTrace) {
sentryStackTrace = SentryStackTrace(
frames: _stacktraceFactory.getStackFrames(StackTrace.current),
);
}

final sentryStackTrace = SentryStackTrace(
frames: _stacktraceFactory.getStackFrames(stackTrace),
);

final sentryException = SentryException(
type: '${exception.runtimeType}',
value: '$exception',
mechanism: mechanism,
stacktrace: sentryStackTrace,
stackTrace: sentryStackTrace,
);

return sentryException;
Expand Down
17 changes: 17 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,23 @@ class SentryOptions {
_sdk = sdk ?? _sdk;
}

bool _attachStackTrace = true;

/// When enabled, stack traces are automatically attached to all messages logged.
/// Stack traces are always attached to exceptions;
/// however, when this option is set, stack traces are also sent with messages.
/// This option, for instance, means that stack traces appear next to all log messages.
///
/// This option is true` by default.
///
/// Grouping in Sentry is different for events with stack traces and without.
/// As a result, you will get new groups as you enable or disable this flag for certain events.
bool get attachStackTrace => _attachStackTrace;
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

set attachStackTrace(bool attachStacktrace) {
_attachStackTrace = attachStacktrace ?? _attachStackTrace;
}

// TODO: Scope observers, enableScopeSync

// TODO: sendDefaultPii
Expand Down
14 changes: 9 additions & 5 deletions dart/lib/src/sentry_stack_trace_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ class SentryStackTraceFactory {

final frames = <SentryStackFrame>[];
for (var t = 0; t < chain.traces.length; t += 1) {
final encodedFrames =
chain.traces[t].frames.map((f) => encodeStackTraceFrame(f));
final encodedFrames = chain.traces[t].frames
// we don't want to add our own frames
.where((frame) => frame.package != 'sentry')
.map((f) => encodeStackTraceFrame(f));

frames.addAll(encodedFrames);

Expand Down Expand Up @@ -84,7 +86,9 @@ class SentryStackTraceFactory {
/// "dart:" and "package:" imports are always relative and are OK to send in
/// full.
String _absolutePathForCrashReport(Frame frame) {
if (frame.uri.scheme != 'dart' && frame.uri.scheme != 'package') {
if (frame.uri.scheme != 'dart' &&
frame.uri.scheme != 'package' &&
frame.uri.pathSegments.isNotEmpty) {
return frame.uri.pathSegments.last;
}

Expand All @@ -101,14 +105,14 @@ class SentryStackTraceFactory {

if (_inAppIncludes != null) {
for (final include in _inAppIncludes) {
if (frame.package != null && frame.package.startsWith(include)) {
if (frame.package != null && frame.package == include) {
rxlabz marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
}
}
if (_inAppExcludes != null) {
for (final exclude in _inAppExcludes) {
if (frame.package != null && frame.package.startsWith(exclude)) {
if (frame.package != null && frame.package == exclude) {
return false;
}
}
Expand Down
Loading