Skip to content

Commit

Permalink
Support captureFeedback (#2230)
Browse files Browse the repository at this point in the history
* add captureFeedback methods

* add should capture feedback as event test

* add sentry_feedback_test

* update contexts test

* test before send feedback

* test hint and event processors

* basic scope test

* test trace context and attachment behaviour

* test sample rate for feedback and fix mock transport calls comparison

* add hub tests

* add sentry tests

* add changelog entry

* cleanup + comments

* test envelope item for feedback

* remove duplacte typedef

* fix test expectation

* Deprecate captureUserFeedback

* update depraction info in cl

* format

* add missing option in test

* run format

* organize imports

* add missing method

* fix test epectation

* ignore deprecations internally

* add to integration test, fix analyze errors

* fix cl

* disable fixture.options.automatedTestMode

* update test

* fix cl

* Add `SentryFeedbackWidget` (#2240)

* fix cl

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

---------

Co-authored-by: Giancarlo Buenaflor <[email protected]>
  • Loading branch information
denrase and buenaflor authored Oct 10, 2024
1 parent 547db82 commit 0880a97
Show file tree
Hide file tree
Showing 35 changed files with 1,497 additions and 545 deletions.
29 changes: 20 additions & 9 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@
- Emit `transaction.data` inside `contexts.trace.data` ([#2284](https://github.com/getsentry/sentry-dart/pull/2284))
- Blocking app starts if "appLaunchedInForeground" is false. (Android only) ([#2291](https://github.com/getsentry/sentry-dart/pull/2291))
- Windows native error & obfuscation support ([#2286](https://github.com/getsentry/sentry-dart/pull/2286))
- Support `captureFeedback` ([#2230](https://github.com/getsentry/sentry-dart/pull/2230))
- Deprecated `Sentry.captureUserFeedback`, use `captureFeedback` instead.
- Deprecated `Hub.captureUserFeedback`, use `captureFeedback` instead.
- Deprecated `SentryClient.captureUserFeedback`, use `captureFeedback` instead.
- Deprecated `SentryUserFeedback`, use `SentryFeedback` instead.
- Add `SentryFeedbackWidget` ([#2240](https://github.com/getsentry/sentry-dart/pull/2240))
```dart
Navigator.push(
context,
MaterialPageRoute(
builder: (context) => SentryFeedbackWidget(associatedEventId: id),
fullscreenDialog: true,
),
);
```

### Enhancements

Expand Down Expand Up @@ -59,7 +74,6 @@
```

- Support allowUrls and denyUrls for Flutter Web ([#2227](https://github.com/getsentry/sentry-dart/pull/2227))

```dart
await SentryFlutter.init(
(options) {
Expand All @@ -70,7 +84,6 @@
appRunner: () => runApp(MyApp()),
);
```

- Collect touch breadcrumbs for all buttons, not just those with `key` specified. ([#2242](https://github.com/getsentry/sentry-dart/pull/2242))
- Add `enableDartSymbolication` option to Sentry.init() for **Flutter iOS, macOS and Android** ([#2256](https://github.com/getsentry/sentry-dart/pull/2256))
- This flag enables symbolication of Dart stack traces when native debug images are not available.
Expand All @@ -93,14 +106,12 @@

- Add `SentryFlutter.nativeCrash()` using MethodChannels for Android and iOS ([#2239](https://github.com/getsentry/sentry-dart/pull/2239))
- This can be used to test if native crash reporting works

- Add `ignoreRoutes` parameter to `SentryNavigatorObserver`. ([#2218](https://github.com/getsentry/sentry-dart/pull/2218))
- This will ignore the Routes and prevent the Route from being pushed to the Sentry server.
- Ignored routes will also create no TTID and TTFD spans.

```dart
SentryNavigatorObserver(ignoreRoutes: ["/ignoreThisRoute"]),
```
- This will ignore the Routes and prevent the Route from being pushed to the Sentry server.
- Ignored routes will also create no TTID and TTFD spans.
```dart
SentryNavigatorObserver(ignoreRoutes: ["/ignoreThisRoute"]),
```

### Improvements

Expand Down
2 changes: 2 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,5 @@ export 'src/utils.dart';
export 'src/spotlight.dart';
// proxy
export 'src/protocol/sentry_proxy.dart';
// feedback
export 'src/protocol/sentry_feedback.dart';
43 changes: 43 additions & 0 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ class Hub {
return sentryId;
}

@Deprecated(
'Will be removed in a future version. Use [captureFeedback] instead')
Future<void> captureUserFeedback(SentryUserFeedback userFeedback) async {
if (!_isEnabled) {
_options.logger(
Expand Down Expand Up @@ -288,6 +290,47 @@ class Hub {
}
}

/// Captures the feedback.
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) async {
var sentryId = SentryId.empty();

if (!_isEnabled) {
_options.logger(
SentryLevel.warning,
"Instance is disabled and this 'captureFeedback' call is a no-op.",
);
} else {
final item = _peek();
late Scope scope;
final s = _cloneAndRunWithScope(item.scope, withScope);
if (s is Future<Scope>) {
scope = await s;
} else {
scope = s;
}

try {
sentryId = await item.client.captureFeedback(
feedback,
hint: hint,
scope: scope,
);
} catch (exception, stacktrace) {
_options.logger(
SentryLevel.error,
'Error while capturing feedback',
exception: exception,
stackTrace: stacktrace,
);
}
}
return sentryId;
}

FutureOr<Scope> _cloneAndRunWithScope(
Scope scope, ScopeCallback? withScope) async {
if (withScope != null) {
Expand Down
19 changes: 17 additions & 2 deletions dart/lib/src/hub_adapter.dart
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import 'dart:async';

import 'package:meta/meta.dart';
import 'hint.dart';

import 'hint.dart';
import 'hub.dart';
import 'metrics/metric.dart';
import 'metrics/metrics_aggregator.dart';
import 'metrics/metrics_api.dart';
import 'profiling.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry.dart';
import 'sentry_client.dart';
import 'sentry_user_feedback.dart';
import 'sentry_options.dart';
import 'sentry_user_feedback.dart';
import 'tracing.dart';

/// Hub adapter to make Integrations testable
Expand Down Expand Up @@ -118,7 +119,9 @@ class HubAdapter implements Hub {
ISentrySpan? getSpan() => Sentry.currentHub.getSpan();

@override
// ignore: deprecated_member_use_from_same_package
Future<void> captureUserFeedback(SentryUserFeedback userFeedback) =>
// ignore: deprecated_member_use_from_same_package
Sentry.captureUserFeedback(userFeedback);

@override
Expand Down Expand Up @@ -197,4 +200,16 @@ class HubAdapter implements Hub {
@override
MetricsAggregator? get metricsAggregator =>
Sentry.currentHub.metricsAggregator;

@override
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) =>
Sentry.currentHub.captureFeedback(
feedback,
hint: hint,
withScope: withScope,
);
}
10 changes: 10 additions & 0 deletions dart/lib/src/noop_hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'metrics/metrics_aggregator.dart';
import 'metrics/metrics_api.dart';
import 'profiling.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry_client.dart';
import 'sentry_options.dart';
Expand Down Expand Up @@ -96,8 +97,17 @@ class NoOpHub implements Hub {
SentryId.empty();

@override
// ignore: deprecated_member_use_from_same_package
Future<void> captureUserFeedback(SentryUserFeedback userFeedback) async {}

@override
Future<SentryId> captureFeedback(
SentryFeedback feedback, {
Hint? hint,
ScopeCallback? withScope,
}) async =>
SentryId.empty();

@override
ISentrySpan startTransaction(
String name,
Expand Down
7 changes: 7 additions & 0 deletions dart/lib/src/noop_sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'hint.dart';
import 'metrics/metric.dart';
import 'metrics/metrics_aggregator.dart';
import 'protocol.dart';
import 'protocol/sentry_feedback.dart';
import 'scope.dart';
import 'sentry_client.dart';
import 'sentry_envelope.dart';
Expand Down Expand Up @@ -55,6 +56,7 @@ class NoOpSentryClient implements SentryClient {
SentryId.empty();

@override
// ignore: deprecated_member_use_from_same_package
Future<void> captureUserFeedback(SentryUserFeedback userFeedback) async {}

@override
Expand All @@ -77,4 +79,9 @@ class NoOpSentryClient implements SentryClient {
@override
@internal
MetricsAggregator? get metricsAggregator => null;

@override
Future<SentryId> captureFeedback(SentryFeedback feedback,
{Scope? scope, Hint? hint}) async =>
SentryId.empty();
}
22 changes: 22 additions & 0 deletions dart/lib/src/protocol/contexts.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:collection';

import '../protocol.dart';
import 'sentry_feedback.dart';

/// The context interfaces provide additional context data.
///
Expand All @@ -19,6 +20,7 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture? culture,
SentryTraceContext? trace,
SentryResponse? response,
SentryFeedback? feedback,
}) : super({
SentryDevice.type: device,
SentryOperatingSystem.type: operatingSystem,
Expand All @@ -29,6 +31,7 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture.type: culture,
SentryTraceContext.type: trace,
SentryResponse.type: response,
SentryFeedback.type: feedback,
});

/// Deserializes [Contexts] from JSON [Map].
Expand Down Expand Up @@ -62,6 +65,9 @@ class Contexts extends MapView<String, dynamic> {
response: data[SentryResponse.type] != null
? SentryResponse.fromJson(Map.from(data[SentryResponse.type]))
: null,
feedback: data[SentryFeedback.type] != null
? SentryFeedback.fromJson(Map.from(data[SentryFeedback.type]))
: null,
);

data.keys
Expand Down Expand Up @@ -136,6 +142,11 @@ class Contexts extends MapView<String, dynamic> {

set response(SentryResponse? value) => this[SentryResponse.type] = value;

/// Feedback context for a feedback event.
SentryFeedback? get feedback => this[SentryFeedback.type];

set feedback(SentryFeedback? value) => this[SentryFeedback.type] = value;

/// Produces a [Map] that can be serialized to JSON.
Map<String, dynamic> toJson() {
final json = <String, dynamic>{};
Expand Down Expand Up @@ -198,6 +209,13 @@ class Contexts extends MapView<String, dynamic> {
}
break;

case SentryFeedback.type:
final feedbackMap = feedback?.toJson();
if (feedbackMap?.isNotEmpty ?? false) {
json[SentryFeedback.type] = feedbackMap;
}
break;

case SentryRuntime.listType:
if (runtimes.length == 1) {
final runtime = runtimes[0];
Expand Down Expand Up @@ -249,6 +267,7 @@ class Contexts extends MapView<String, dynamic> {
trace: trace?.clone(),
response: response?.clone(),
runtimes: runtimes.map((runtime) => runtime.clone()).toList(),
feedback: feedback?.clone(),
)..addEntries(
entries.where((element) => !_defaultFields.contains(element.key)),
);
Expand All @@ -266,6 +285,7 @@ class Contexts extends MapView<String, dynamic> {
SentryGpu? gpu,
SentryTraceContext? trace,
SentryResponse? response,
SentryFeedback? feedback,
}) =>
Contexts(
device: device ?? this.device,
Expand All @@ -277,6 +297,7 @@ class Contexts extends MapView<String, dynamic> {
culture: culture ?? this.culture,
trace: trace ?? this.trace,
response: response ?? this.response,
feedback: feedback ?? this.feedback,
)..addEntries(
entries.where((element) => !_defaultFields.contains(element.key)),
);
Expand All @@ -292,5 +313,6 @@ class Contexts extends MapView<String, dynamic> {
SentryCulture.type,
SentryTraceContext.type,
SentryResponse.type,
SentryFeedback.type,
];
}
81 changes: 81 additions & 0 deletions dart/lib/src/protocol/sentry_feedback.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import 'package:meta/meta.dart';

import 'access_aware_map.dart';
import 'sentry_id.dart';

@immutable
class SentryFeedback {
static const type = 'feedback';

SentryFeedback({
required this.message,
this.contactEmail,
this.name,
this.replayId,
this.url,
this.associatedEventId,
this.unknown,
});

final String message;
final String? contactEmail;
final String? name;
final String? replayId;
final String? url;
final SentryId? associatedEventId;

@internal
final Map<String, dynamic>? unknown;

/// Deserializes a [SentryFeedback] from JSON [Map].
factory SentryFeedback.fromJson(Map<String, dynamic> data) {
final json = AccessAwareMap(data);

String? associatedEventId = json['associated_event_id'];

return SentryFeedback(
message: json['message'],
contactEmail: json['contact_email'],
name: json['name'],
replayId: json['replay_id'],
url: json['url'],
associatedEventId:
associatedEventId != null ? SentryId.fromId(associatedEventId) : null,
unknown: json.notAccessed(),
);
}

Map<String, dynamic> toJson() {
return {
...?unknown,
'message': message,
if (contactEmail != null) 'contact_email': contactEmail,
if (name != null) 'name': name,
if (replayId != null) 'replay_id': replayId,
if (url != null) 'url': url,
if (associatedEventId != null)
'associated_event_id': associatedEventId.toString(),
};
}

SentryFeedback copyWith({
String? message,
String? contactEmail,
String? name,
String? replayId,
String? url,
SentryId? associatedEventId,
Map<String, dynamic>? unknown,
}) =>
SentryFeedback(
message: message ?? this.message,
contactEmail: contactEmail ?? this.contactEmail,
name: name ?? this.name,
replayId: replayId ?? this.replayId,
url: url ?? this.url,
associatedEventId: associatedEventId ?? this.associatedEventId,
unknown: unknown ?? this.unknown,
);

SentryFeedback clone() => copyWith();
}
Loading

0 comments on commit 0880a97

Please sign in to comment.