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

fix: context defualts on native platforms other than iOS/Android #2439

Merged
merged 6 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- Linux native error & obfuscation support ([#2431](https://github.com/getsentry/sentry-dart/pull/2431))
- Improve Device context on plain Dart and Flutter desktop apps ([#2441](https://github.com/getsentry/sentry-dart/pull/2441))

### Fixes

- OS & device contexts missing on Windows ([#2439](https://github.com/getsentry/sentry-dart/pull/2439))

## 8.11.0-beta.1

### Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,12 @@ class IoEnricherEventProcessor implements EnricherEventProcessor {
// Amend app with current memory usage, as this is not available on native.
final app = _getApp(event.contexts.app);

// If there's a native integration available, it probably has better
// information available than Flutter.

final device = _options.platformChecker.hasNativeIntegration
? null
: _getDevice(event.contexts.device);

final os = _options.platformChecker.hasNativeIntegration
? null
: _getOperatingSystem(event.contexts.operatingSystem);

final culture = _options.platformChecker.hasNativeIntegration
? null
: _getSentryCulture(event.contexts.culture);

final contexts = event.contexts.copyWith(
device: device,
operatingSystem: os,
device: _getDevice(event.contexts.device),
operatingSystem: _getOperatingSystem(event.contexts.operatingSystem),
runtimes: _getRuntimes(event.contexts.runtimes),
app: app,
culture: culture,
culture: _getSentryCulture(event.contexts.culture),
);

contexts['dart_context'] = _getDartContext();
Expand Down
29 changes: 12 additions & 17 deletions dart/test/event_processor/enricher/io_enricher_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,18 @@ void main() {
expect(event.contexts.runtimes.length, 2);
});

test('does not add device, os and culture if native integration is available',
() {
final enricher = fixture.getSut(hasNativeIntegration: true);
final event = enricher.apply(SentryEvent(), Hint());

expect(event?.contexts.device, isNull);
expect(event?.contexts.operatingSystem, isNull);
expect(event?.contexts.culture, isNull);
});

test('adds device, os and culture if no native integration is available', () {
final enricher = fixture.getSut(hasNativeIntegration: false);
final event = enricher.apply(SentryEvent(), Hint());

expect(event?.contexts.device, isNotNull);
expect(event?.contexts.operatingSystem, isNotNull);
expect(event?.contexts.culture, isNotNull);
group('adds device, os and culture', () {
for (final hasNativeIntegration in [true, false]) {
test('native=$hasNativeIntegration', () {
final enricher =
fixture.getSut(hasNativeIntegration: hasNativeIntegration);
final event = enricher.apply(SentryEvent(), Hint());

expect(event?.contexts.device, isNotNull);
expect(event?.contexts.operatingSystem, isNotNull);
expect(event?.contexts.culture, isNotNull);
});
}
});

test('device has name', () {
Expand Down
18 changes: 18 additions & 0 deletions flutter/lib/src/integrations/load_contexts_integration.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import 'dart:async';

import 'package:sentry/sentry.dart';
import 'package:collection/collection.dart';
// ignore: implementation_imports
import 'package:sentry/src/event_processor/enricher/enricher_event_processor.dart';
import '../native/sentry_native_binding.dart';
import '../sentry_flutter_options.dart';

Expand All @@ -24,6 +27,21 @@ class LoadContextsIntegration extends Integration<SentryFlutterOptions> {
options.addEventProcessor(
_LoadContextsIntegrationEventProcessor(_native, options),
);

// We need to move [IOEnricherEventProcessor] after [_LoadContextsIntegrationEventProcessor]
// so that we know which contexts were set by the user and which were set by the other processor.
// The priorities are:
// - user-set context values
// - context values set from native (this)
// - values set from IOEnricherEventProcessor
final enricherEventProcessor = options.eventProcessors.firstWhereOrNull(
(element) => element is EnricherEventProcessor,
);
if (enricherEventProcessor != null) {
options.removeEventProcessor(enricherEventProcessor);
options.addEventProcessor(enricherEventProcessor);
}

options.sdk.addIntegration('loadContextsIntegration');
}
}
Expand Down
2 changes: 1 addition & 1 deletion flutter/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ dependencies:
meta: ^1.3.0
ffi: ^2.0.0
file: '>=6.1.4'
collection: ^1.16.0

dev_dependencies:
build_runner: ^2.4.2
Expand All @@ -36,7 +37,6 @@ dev_dependencies:
mockito: ^5.1.0
yaml: ^3.1.0 # needed for version match (code and pubspec)
flutter_lints: '>=4.0.0'
collection: ^1.16.0
remove_from_coverage: ^2.0.0
flutter_localizations:
sdk: flutter
Expand Down
94 changes: 39 additions & 55 deletions flutter/test/sentry_flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:package_info_plus/package_info_plus.dart';
import 'package:sentry/src/platform/platform.dart';
import 'package:sentry/src/dart_exception_type_identifier.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_flutter/src/file_system_transport.dart';
import 'package:sentry_flutter/src/flutter_exception_type_identifier.dart';
import 'package:sentry_flutter/src/integrations/connectivity/connectivity_integration.dart';
import 'package:sentry_flutter/src/integrations/integrations.dart';
Expand Down Expand Up @@ -70,34 +71,31 @@ void main() {
});

test('Android', () async {
List<Integration> integrations = [];
Transport transport = MockTransport();
late final SentryFlutterOptions options;
late final Transport transport;

final sentryFlutterOptions = defaultTestOptions(
getPlatformChecker(platform: MockPlatform.android()))
..methodChannel = native.channel;

await SentryFlutter.init(
(options) async {
options.dsn = fakeDsn;
options.profilesSampleRate = 1.0;
integrations = options.integrations;
transport = options.transport;
(o) async {
o.dsn = fakeDsn;
o.profilesSampleRate = 1.0;
options = o;
transport = o.transport;
},
appRunner: appRunner,
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: true,
);
expect(transport, isA<FileSystemTransport>());

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: true);

testConfiguration(
integrations: integrations,
integrations: options.integrations,
shouldHaveIntegrations: [
...androidIntegrations,
...nativeIntegrations,
Expand All @@ -110,49 +108,51 @@ void main() {
],
);

integrations
options.integrations
.indexWhere((element) => element is WidgetsFlutterBindingIntegration);

testBefore(
integrations: integrations,
integrations: options.integrations,
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(
options.eventProcessors.indexOfTypeString('IoEnricherEventProcessor'),
greaterThan(options.eventProcessors
.indexOfTypeString('_LoadContextsIntegrationEventProcessor')));

expect(SentryFlutter.native, isNotNull);
expect(Sentry.currentHub.profilerFactory, isNull);

await Sentry.close();
}, testOn: 'vm');

test('iOS', () async {
List<Integration> integrations = [];
Transport transport = MockTransport();
late final SentryFlutterOptions options;
late final Transport transport;

final sentryFlutterOptions =
defaultTestOptions(getPlatformChecker(platform: MockPlatform.iOs()))
..methodChannel = native.channel;

await SentryFlutter.init(
(options) async {
options.dsn = fakeDsn;
options.profilesSampleRate = 1.0;
integrations = options.integrations;
transport = options.transport;
(o) async {
o.dsn = fakeDsn;
o.profilesSampleRate = 1.0;
options = o;
transport = o.transport;
},
appRunner: appRunner,
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: true,
);
expect(transport, isA<FileSystemTransport>());

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: true);

testConfiguration(
integrations: integrations,
integrations: options.integrations,
shouldHaveIntegrations: [
...iOsAndMacOsIntegrations,
...nativeIntegrations,
Expand All @@ -166,14 +166,19 @@ void main() {
);

testBefore(
integrations: integrations,
integrations: options.integrations,
beforeIntegration: WidgetsFlutterBindingIntegration,
afterIntegration: OnErrorIntegration);

expect(SentryFlutter.native, isNotNull);
expect(Sentry.currentHub.profilerFactory,
isInstanceOf<SentryNativeProfilerFactory>());

expect(
options.eventProcessors.indexOfTypeString('IoEnricherEventProcessor'),
greaterThan(options.eventProcessors
.indexOfTypeString('_LoadContextsIntegrationEventProcessor')));

await Sentry.close();
}, testOn: 'vm');

Expand All @@ -195,10 +200,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: true,
);
expect(transport, isA<FileSystemTransport>());

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: true);
Expand Down Expand Up @@ -244,10 +246,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: true);
Expand Down Expand Up @@ -295,10 +294,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: true);
Expand Down Expand Up @@ -345,10 +341,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testScopeObserver(
options: sentryFlutterOptions, expectedHasNativeScopeObserver: false);
Expand Down Expand Up @@ -396,10 +389,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testConfiguration(
integrations: integrations,
Expand Down Expand Up @@ -441,10 +431,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testConfiguration(
integrations: integrations,
Expand Down Expand Up @@ -487,10 +474,7 @@ void main() {
options: sentryFlutterOptions,
);

testTransport(
transport: transport,
hasFileSystemTransport: false,
);
expect(transport, isNot(isA<FileSystemTransport>()));

testConfiguration(
integrations: integrations,
Expand Down
34 changes: 17 additions & 17 deletions flutter/test/sentry_flutter_util.dart
Original file line number Diff line number Diff line change
@@ -1,19 +1,7 @@
import 'package:flutter_test/flutter_test.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_flutter/src/file_system_transport.dart';
import 'package:sentry_flutter/src/native/native_scope_observer.dart';

void testTransport({
required Transport transport,
required bool hasFileSystemTransport,
}) {
expect(
transport is FileSystemTransport,
hasFileSystemTransport,
reason: '$FileSystemTransport was wrongly set',
);
}

void testScopeObserver(
{required SentryFlutterOptions options,
required bool expectedHasNativeScopeObserver}) {
Expand Down Expand Up @@ -54,9 +42,21 @@ void testBefore({
required Type beforeIntegration,
required Type afterIntegration,
}) {
final beforeIndex = integrations
.indexWhere((element) => element.runtimeType == beforeIntegration);
final afterIndex = integrations
.indexWhere((element) => element.runtimeType == afterIntegration);
expect(beforeIndex < afterIndex, true);
expect(integrations.indexOfType(beforeIntegration),
lessThan(integrations.indexOfType(afterIntegration)));
}

extension ListExtension<T> on List<T> {
int indexOfType(Type type) {
final index = indexWhere((element) => element.runtimeType == type);
expect(index, greaterThanOrEqualTo(0), reason: '$type not found in $this');
return index;
}

int indexOfTypeString(String type) {
final index =
indexWhere((element) => element.runtimeType.toString() == type);
expect(index, greaterThanOrEqualTo(0), reason: '$type not found in $this');
return index;
}
}
Loading