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

Make hint non-nullable in BeforeSendCallback, BeforeBreadcrumbCall and EventProcessor #1574

Merged
merged 13 commits into from
Sep 4, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
- Refrain from overwriting the span status for unfinished spans ([#1577](https://github.com/getsentry/sentry-dart/pull/1577))
- Older self-hosted sentry instances will drop transactions containing unfinished spans.
- This change was introduced in [relay/#1690](https://github.com/getsentry/relay/pull/1690) and released with [22.12.0](https://github.com/getsentry/relay/releases/tag/22.12.0)
- Make `hint` non-nullable in `BeforeSendCallback`, `BeforeBreadcrumbCall` and `EventProcessor` ([#1574](https://github.com/getsentry/sentry-dart/pull/1574))
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 🚫 The changelog entry seems to be part of an already released section ## 8.0.0.
    Consider moving the entry to the ## Unreleased section, please.

- This will affect your callbacks, making this a breaking change.

## Unreleased

Expand Down
2 changes: 1 addition & 1 deletion dart/example/bin/example.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Future<void> decode() async {

class TagEventProcessor implements EventProcessor {
@override
SentryEvent? apply(SentryEvent event, {hint}) {
SentryEvent? apply(SentryEvent event, hint) {
return event..tags?.addAll({'page-locale': 'en-us'});
}
}
2 changes: 1 addition & 1 deletion dart/example_web/web/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ Future<void> parseData() async {

class TagEventProcessor implements EventProcessor {
@override
SentryEvent? apply(SentryEvent event, {hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
return event..tags?.addAll({'page-locale': 'en-us'});
}
}
6 changes: 3 additions & 3 deletions dart/lib/src/event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'protocol.dart';
/// null in case the event will be dropped and not sent.
abstract class EventProcessor {
FutureOr<SentryEvent?> apply(
SentryEvent event, {
Hint? hint,
});
SentryEvent event,
Hint hint,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DeduplicationEventProcessor implements EventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
if (event is SentryTransaction) {
return event;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class IoEnricherEventProcessor implements EnricherEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
// If there's a native integration available, it probably has better
// information available than Flutter.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class WebEnricherEventProcessor implements EnricherEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
// Web has no native integration, so no need to check for it

final contexts = event.contexts.copyWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class IoExceptionEventProcessor implements ExceptionEventProcessor {
final SentryOptions _options;

@override
SentryEvent? apply(SentryEvent event, {Hint? hint}) {
SentryEvent? apply(SentryEvent event, Hint hint) {
final throwable = event.throwable;
if (throwable is HttpException) {
return _applyHttpException(throwable, event);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ ExceptionEventProcessor exceptionEventProcessor(SentryOptions _) =>

class WebExcptionEventProcessor implements ExceptionEventProcessor {
@override
SentryEvent apply(SentryEvent event, {Hint? hint}) => event;
SentryEvent apply(SentryEvent event, Hint hint) => event;
}
8 changes: 4 additions & 4 deletions dart/lib/src/hint.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import 'sentry_attachment/sentry_attachment.dart';
/// Example:
///
/// ```dart
/// options.beforeSend = (event, {hint}) {
/// final syntheticException = hint?.get(TypeCheckHint.syntheticException);
/// options.beforeSend = (event, hint) {
/// final syntheticException = hint.get(TypeCheckHint.syntheticException);
/// if (syntheticException is FlutterErrorDetails) {
/// // Do something with hint data
/// }
Expand All @@ -28,14 +28,14 @@ import 'sentry_attachment/sentry_attachment.dart';
/// ```dart
/// import 'dart:convert';
///
/// options.beforeSend = (event, {hint}) {
/// options.beforeSend = (event, hint) {
/// final text = 'This event should not be sent happen in prod. Investigate.';
/// final textAttachment = SentryAttachment.fromIntList(
/// utf8.encode(text),
/// 'event_info.txt',
/// contentType: 'text/plain',
/// );
/// hint?.attachments.add(textAttachment);
/// hint.attachments.add(textAttachment);
/// return event;
/// };
/// ```
Expand Down
16 changes: 8 additions & 8 deletions dart/lib/src/scope.dart
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class Scope {

Scope(this._options);

Breadcrumb? _addBreadCrumbSync(Breadcrumb breadcrumb, {Hint? hint}) {
Breadcrumb? _addBreadCrumbSync(Breadcrumb breadcrumb, Hint hint) {
// bail out if maxBreadcrumbs is zero
if (_options.maxBreadcrumbs == 0) {
return null;
Expand All @@ -155,7 +155,7 @@ class Scope {
try {
processedBreadcrumb = _options.beforeBreadcrumb!(
processedBreadcrumb,
hint: hint,
hint,
);
if (processedBreadcrumb == null) {
_options.logger(
Expand Down Expand Up @@ -189,7 +189,7 @@ class Scope {

/// Adds a breadcrumb to the breadcrumbs queue
Future<void> addBreadcrumb(Breadcrumb breadcrumb, {Hint? hint}) async {
final addedBreadcrumb = _addBreadCrumbSync(breadcrumb, hint: hint);
final addedBreadcrumb = _addBreadCrumbSync(breadcrumb, hint ?? Hint());
if (addedBreadcrumb != null) {
await _callScopeObservers((scopeObserver) async =>
await scopeObserver.addBreadcrumb(addedBreadcrumb));
Expand Down Expand Up @@ -275,9 +275,9 @@ class Scope {
}

Future<SentryEvent?> applyToEvent(
SentryEvent event, {
Hint? hint,
}) async {
SentryEvent event,
Hint hint,
) async {
event = event.copyWith(
transaction: event.transaction ?? transaction,
user: _mergeUsers(user, event.user),
Expand Down Expand Up @@ -320,7 +320,7 @@ class Scope {
SentryEvent? processedEvent = event;
for (final processor in _eventProcessors) {
try {
final e = processor.apply(processedEvent!, hint: hint);
final e = processor.apply(processedEvent!, hint);
if (e is Future<SentryEvent?>) {
processedEvent = await e;
} else {
Expand Down Expand Up @@ -429,7 +429,7 @@ class Scope {
}

for (final breadcrumb in List.from(_breadcrumbs)) {
clone._addBreadCrumbSync(breadcrumb);
clone._addBreadCrumbSync(breadcrumb, Hint());
}

for (final eventProcessor in List.from(_eventProcessors)) {
Expand Down
29 changes: 16 additions & 13 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class SentryClient {
hint ??= Hint();

if (scope != null) {
preparedEvent = await scope.applyToEvent(preparedEvent, hint: hint);
preparedEvent = await scope.applyToEvent(preparedEvent, hint);
} else {
_options.logger(
SentryLevel.debug, 'No scope to apply on event was provided');
Expand All @@ -89,8 +89,8 @@ class SentryClient {

preparedEvent = await _runEventProcessors(
preparedEvent,
hint,
eventProcessors: _options.eventProcessors,
hint: hint,
);

// dropped by event processors
Expand All @@ -100,7 +100,7 @@ class SentryClient {

preparedEvent = await _runBeforeSend(
preparedEvent,
hint: hint,
hint,
);

// dropped by beforeSend
Expand Down Expand Up @@ -288,9 +288,11 @@ class SentryClient {
SentryTransaction? preparedTransaction =
_prepareEvent(transaction) as SentryTransaction;

final hint = Hint();

if (scope != null) {
preparedTransaction =
await scope.applyToEvent(preparedTransaction) as SentryTransaction?;
preparedTransaction = await scope.applyToEvent(preparedTransaction, hint)
as SentryTransaction?;
} else {
_options.logger(
SentryLevel.debug, 'No scope to apply on transaction was provided');
Expand All @@ -303,6 +305,7 @@ class SentryClient {

preparedTransaction = await _runEventProcessors(
preparedTransaction,
hint,
eventProcessors: _options.eventProcessors,
) as SentryTransaction?;

Expand All @@ -312,7 +315,7 @@ class SentryClient {
}

preparedTransaction =
await _runBeforeSend(preparedTransaction) as SentryTransaction?;
await _runBeforeSend(preparedTransaction, hint) as SentryTransaction?;

// dropped by beforeSendTransaction
if (preparedTransaction == null) {
Expand Down Expand Up @@ -354,9 +357,9 @@ class SentryClient {
void close() => _options.httpClient.close();

Future<SentryEvent?> _runBeforeSend(
SentryEvent event, {
Hint? hint,
}) async {
SentryEvent event,
Hint hint,
) async {
SentryEvent? eventOrTransaction = event;

final beforeSend = _options.beforeSend;
Expand All @@ -373,7 +376,7 @@ class SentryClient {
eventOrTransaction = e;
}
} else if (beforeSend != null) {
final e = beforeSend(event, hint: hint);
final e = beforeSend(event, hint);
if (e is Future<SentryEvent?>) {
eventOrTransaction = await e;
} else {
Expand Down Expand Up @@ -404,14 +407,14 @@ class SentryClient {
}

Future<SentryEvent?> _runEventProcessors(
SentryEvent event, {
Hint? hint,
SentryEvent event,
Hint hint, {
required List<EventProcessor> eventProcessors,
}) async {
SentryEvent? processedEvent = event;
for (final processor in eventProcessors) {
try {
final e = processor.apply(processedEvent!, hint: hint);
final e = processor.apply(processedEvent!, hint);
if (e is Future<SentryEvent?>) {
processedEvent = await e;
} else {
Expand Down
12 changes: 6 additions & 6 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,9 @@ class SentryOptions {
/// This function is called with an SDK specific event object and can return a modified event
/// object or nothing to skip reporting the event
typedef BeforeSendCallback = FutureOr<SentryEvent?> Function(
SentryEvent event, {
Hint? hint,
});
SentryEvent event,
Hint hint,
);

/// This function is called with an SDK specific transaction object and can return a modified transaction
/// object or nothing to skip reporting the transaction
Expand All @@ -457,9 +457,9 @@ typedef BeforeSendTransactionCallback = FutureOr<SentryTransaction?> Function(
/// This function is called with an SDK specific breadcrumb object before the breadcrumb is added
/// to the scope. When nothing is returned from the function, the breadcrumb is dropped
typedef BeforeBreadcrumbCallback = Breadcrumb? Function(
Breadcrumb? breadcrumb, {
Hint? hint,
});
Breadcrumb? breadcrumb,
Hint hint,
);

/// Used to provide timestamp for logging.
typedef ClockProvider = DateTime Function();
Expand Down
24 changes: 12 additions & 12 deletions dart/test/event_processor/deduplication_event_processor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,33 @@ void main() {
final sut = fixture.getSut(true);
var ogEvent = _createEvent('foo');

expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent), isNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNull);
});

test('does not deduplicate if disabled', () {
final sut = fixture.getSut(false);
var ogEvent = _createEvent('foo');

expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
expect(sut.apply(ogEvent, Hint()), isNotNull);
});

test('does not deduplicate if different events', () {
final sut = fixture.getSut(true);
var fooEvent = _createEvent('foo');
var barEvent = _createEvent('bar');

expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(barEvent), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
expect(sut.apply(barEvent, Hint()), isNotNull);
});

test('does not deduplicate transaction', () {
final sut = fixture.getSut(true);
final transaction = _createTransaction(fixture.hub);

expect(sut.apply(transaction), isNotNull);
expect(sut.apply(transaction), isNotNull);
expect(sut.apply(transaction, Hint()), isNotNull);
expect(sut.apply(transaction, Hint()), isNotNull);
});

test('exceptions to keep for deduplication', () {
Expand All @@ -51,10 +51,10 @@ void main() {
var barEvent = _createEvent('bar');
var fooBarEvent = _createEvent('foo bar');

expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(barEvent), isNotNull);
expect(sut.apply(fooBarEvent), isNotNull);
expect(sut.apply(fooEvent), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
expect(sut.apply(barEvent, Hint()), isNotNull);
expect(sut.apply(fooBarEvent, Hint()), isNotNull);
expect(sut.apply(fooEvent, Hint()), isNotNull);
});

test('integration test', () async {
Expand Down
Loading
Loading