Skip to content

Commit

Permalink
move platfomr out of paltform checker
Browse files Browse the repository at this point in the history
  • Loading branch information
denrase committed Feb 24, 2025
1 parent 58e55ff commit 68e3fb8
Show file tree
Hide file tree
Showing 44 changed files with 1,588 additions and 1,835 deletions.
8 changes: 4 additions & 4 deletions dart/lib/src/event_processor/enricher/io_platform_memory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ class PlatformMemory {
final SentryOptions options;

int? getTotalPhysicalMemory() {
if (options.platformChecker.platform.isLinux) {
if (options.platform.isLinux) {
return _getLinuxMemInfoValue('MemTotal');
} else if (options.platformChecker.platform.isWindows) {
} else if (options.platform.isWindows) {
return _getWindowsWmicValue('ComputerSystem', 'TotalPhysicalMemory');
} else {
return null;
}
}

int? getFreePhysicalMemory() {
if (options.platformChecker.platform.isLinux) {
if (options.platform.isLinux) {
return _getLinuxMemInfoValue('MemFree');
} else if (options.platformChecker.platform.isWindows) {
} else if (options.platform.isWindows) {
return _getWindowsWmicValue('OS', 'FreePhysicalMemory');
} else {
return null;
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/load_dart_debug_images_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class LoadImageIntegrationEventProcessor implements EventProcessor {
// It doesn't need to exist and is not used for symbolication.
late final String codeFile;

final platform = _options.platformChecker.platform;
final platform = _options.platform;

if (platform.isAndroid || platform.isWindows) {
type = 'elf';
Expand Down
19 changes: 14 additions & 5 deletions dart/lib/src/platform/mock_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,20 @@ class MockPlatform extends Platform {
@override
late final String? operatingSystemVersion;

MockPlatform({
OperatingSystem? operatingSystem,
String? operatingSystemVersion,
bool? isWeb,
}) {
@override
late final bool supportsNativeIntegration;

MockPlatform(
{OperatingSystem? operatingSystem,
String? operatingSystemVersion,
bool? isWeb,
bool? supportsNativeIntegration}) {
this.isWeb = isWeb ?? super.isWeb;
this.operatingSystem = operatingSystem ?? super.operatingSystem;
this.operatingSystemVersion =
operatingSystemVersion ?? super.operatingSystemVersion;
this.supportsNativeIntegration =
supportsNativeIntegration ?? super.supportsNativeIntegration;
}

factory MockPlatform.android({bool isWeb = false}) {
Expand All @@ -40,4 +45,8 @@ class MockPlatform extends Platform {
factory MockPlatform.windows({bool isWeb = false}) {
return MockPlatform(operatingSystem: OperatingSystem.windows, isWeb: isWeb);
}

factory MockPlatform.fuchsia({bool isWeb = false}) {
return MockPlatform(operatingSystem: OperatingSystem.fuchsia, isWeb: isWeb);
}
}
4 changes: 2 additions & 2 deletions dart/lib/src/platform/platform.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import '_io_platform.dart' if (dart.library.js_interop) '_web_platform.dart'
as impl;

const currentPlatform = Platform();

class Platform extends impl.PlatformBase {
const Platform();

Expand All @@ -17,6 +15,8 @@ class Platform extends impl.PlatformBase {
bool get isIOS => operatingSystem == OperatingSystem.ios;

bool get isFuchsia => operatingSystem == OperatingSystem.fuchsia;

bool get supportsNativeIntegration => !isFuchsia;
}

enum OperatingSystem {
Expand Down
14 changes: 0 additions & 14 deletions dart/lib/src/platform_checker.dart
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import 'dart:async';
import 'platform/platform.dart';

/// Helper to check in which environment the library is running.
/// The environment checks (release/debug/profile) are mutually exclusive.
// TODO rename this to `RuntimeChecker` or something similar to better represent what it does.
// TODO move `platform` directly to options - that is what we actually access 99 % of the times in tests and lib.
class PlatformChecker {
PlatformChecker({
this.platform = currentPlatform,
bool? isRootZone,
}) : isRootZone = isRootZone ?? Zone.current == Zone.root;

Expand Down Expand Up @@ -35,16 +33,4 @@ class PlatformChecker {
? 'debug'
: 'profile';
}

// TODO remove this check - it should be handled by the native integration... also, it's actually always true...
/// Indicates whether a native integration is available.
bool get hasNativeIntegration =>
platform.isWeb ||
platform.isAndroid ||
platform.isIOS ||
platform.isMacOS ||
platform.isWindows ||
platform.isLinux;

final Platform platform;
}
2 changes: 1 addition & 1 deletion dart/lib/src/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class Sentry {
_setEnvironmentVariables(options);

// Throws when running on the browser
if (!options.platformChecker.platform.isWeb) {
if (!options.platform.isWeb) {
// catch any errors that may occur within the entry function, main()
// in the ‘root zone’ where all Dart programs start
options.addIntegrationByIndex(0, IsolateErrorIntegration());
Expand Down
11 changes: 5 additions & 6 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ class SentryClient {
);
// TODO: Use spotlight integration directly through JS SDK, then we can remove isWeb check
final enableFlutterSpotlight = (options.spotlight.enabled &&
(options.platformChecker.platform.isWeb ||
options.platformChecker.platform.isLinux ||
options.platformChecker.platform.isWindows));
(options.platform.isWeb ||
options.platform.isLinux ||
options.platform.isWindows));
// Spotlight in the Flutter layer is only enabled for Web, Linux and Windows
// Other platforms use spotlight through their native SDKs
if (enableFlutterSpotlight) {
Expand Down Expand Up @@ -214,8 +214,7 @@ class SentryClient {
environment: event.environment ?? _options.environment,
release: event.release ?? _options.release,
sdk: event.sdk ?? _options.sdk,
platform: event.platform ??
sdkPlatform(_options.platformChecker.platform.isWeb),
platform: event.platform ?? sdkPlatform(_options.platform.isWeb),
);

if (event is SentryTransaction) {
Expand Down Expand Up @@ -250,7 +249,7 @@ class SentryClient {

SentryThread? sentryThread;

if (!_options.platformChecker.platform.isWeb &&
if (!_options.platform.isWeb &&
isolateName != null &&
_options.attachThreads) {
sentryException = sentryException.copyWith(threadId: isolateId);
Expand Down
11 changes: 8 additions & 3 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'client_reports/noop_client_report_recorder.dart';
import 'diagnostic_logger.dart';
import 'environment/environment_variables.dart';
import 'noop_client.dart';
import 'platform/platform.dart';
import 'sentry_exception_factory.dart';
import 'sentry_stack_trace_factory.dart';
import 'transport/noop_transport.dart';
Expand Down Expand Up @@ -275,6 +276,8 @@ class SentryOptions {
/// This is useful in tests. Should be an implementation of [PlatformChecker].
PlatformChecker platformChecker = PlatformChecker();

Platform platform = Platform();

/// If [environmentVariables] is provided, it is used get the environment
/// variables. This is useful in tests.
EnvironmentVariables environmentVariables = EnvironmentVariables.instance();
Expand Down Expand Up @@ -494,13 +497,15 @@ class SentryOptions {
/// iOS only supports http proxies, while macOS also supports socks.
SentryProxy? proxy;

SentryOptions({String? dsn, PlatformChecker? checker}) {
SentryOptions({String? dsn, Platform? platform, PlatformChecker? checker}) {
this.dsn = dsn;
if (platform != null) {
this.platform = platform;
}
if (checker != null) {
platformChecker = checker;
}
sdk = SdkVersion(
name: sdkName(platformChecker.platform.isWeb), version: sdkVersion);
sdk = SdkVersion(name: sdkName(this.platform.isWeb), version: sdkVersion);
sdk.addPackage('pub:sentry', sdkVersion);
}

Expand Down
3 changes: 1 addition & 2 deletions dart/lib/src/sentry_stack_trace_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ class SentryStackTraceFactory {
SentryLevel.debug, "Failed to parse stack frame: $member");
}

final platform =
_options.platformChecker.platform.isWeb ? 'javascript' : 'dart';
final platform = _options.platform.isWeb ? 'javascript' : 'dart';
final fileName =
frame.uri.pathSegments.isNotEmpty ? frame.uri.pathSegments.last : null;
final abs = '$eventOrigin${_absolutePathForCrashReport(frame)}';
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/transport/http_transport_request_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class HttpTransportRequestHandler {
HttpTransportRequestHandler(this._options, this._requestUri)
: _dsn = _options.parsedDsn,
_headers = _buildHeaders(
_options.platformChecker.platform.isWeb,
_options.platform.isWeb,
_options.sentryClientName,
) {
_credentialBuilder = _CredentialBuilder(
Expand Down
7 changes: 4 additions & 3 deletions dart/test/event_processor/enricher/io_enricher_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import 'dart:io';

import 'package:sentry/sentry.dart';
import 'package:sentry/src/event_processor/enricher/io_enricher_event_processor.dart';
import 'package:sentry/src/platform/mock_platform.dart';
import 'package:test/test.dart';

import '../../mocks.dart';
import '../../mocks/mock_platform_checker.dart';
import '../../test_utils.dart';

void main() {
Expand Down Expand Up @@ -255,8 +255,9 @@ class Fixture {
bool hasNativeIntegration = false,
bool includePii = false,
}) {
final options = defaultTestOptions(
MockPlatformChecker(hasNativeIntegration: hasNativeIntegration))
final options = defaultTestOptions()
..platform =
hasNativeIntegration ? MockPlatform.iOS() : MockPlatform.fuchsia()
..sendDefaultPii = includePii;

return IoEnricherEventProcessor(options);
Expand Down
6 changes: 3 additions & 3 deletions dart/test/event_processor/enricher/web_enricher_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ library;

import 'package:sentry/sentry.dart';
import 'package:sentry/src/event_processor/enricher/web_enricher_event_processor.dart';
import 'package:sentry/src/platform/mock_platform.dart';
import 'package:test/test.dart';

import '../../mocks.dart';
import '../../mocks/mock_platform_checker.dart';
import '../../test_utils.dart';

// can be tested on command line with
Expand Down Expand Up @@ -200,8 +200,8 @@ void main() {

class Fixture {
WebEnricherEventProcessor getSut() {
final options =
defaultTestOptions(MockPlatformChecker(hasNativeIntegration: false));
final options = defaultTestOptions()
..platform = MockPlatform.fuchsia(); // Does not have native integration
return enricherEventProcessor(options) as WebEnricherEventProcessor;
}
}
8 changes: 2 additions & 6 deletions dart/test/load_dart_debug_images_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:sentry/src/platform/mock_platform.dart';
import 'package:sentry/src/sentry_stack_trace_factory.dart';
import 'package:test/test.dart';

import 'mocks/mock_platform_checker.dart';
import 'test_utils.dart';

void main() {
Expand All @@ -26,8 +25,7 @@ void main() {

setUp(() {
fixture = Fixture();
fixture.options.platformChecker =
MockPlatformChecker(platform: platform);
fixture.options.platform = platform;
});

test('adds itself to sdk.integrations', () {
Expand Down Expand Up @@ -193,9 +191,7 @@ isolate_dso_base: 10000000
}

test('debug image is null on unsupported platforms', () async {
final fixture = Fixture()
..options.platformChecker =
MockPlatformChecker(platform: MockPlatform.linux());
final fixture = Fixture()..options.platform = MockPlatform.linux();
final event = fixture.newEvent(stackTrace: fixture.parse('''
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
build_id: 'b680cb890f9e3c12a24b172d050dec73'
Expand Down
13 changes: 1 addition & 12 deletions dart/test/mocks/mock_platform_checker.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import 'package:sentry/src/platform/platform.dart';
import 'package:sentry/src/platform_checker.dart';

import 'no_such_method_provider.dart';
Expand All @@ -8,19 +7,12 @@ class MockPlatformChecker extends PlatformChecker with NoSuchMethodProvider {
this.isDebug = false,
this.isProfile = false,
this.isRelease = false,
this.hasNativeIntegration = false,
Platform? platform,
}) : _platform = platform;

final Platform? _platform;
});

final bool isDebug;
final bool isProfile;
final bool isRelease;

@override
bool hasNativeIntegration = false;

@override
bool isDebugMode() => isDebug;

Expand All @@ -29,7 +21,4 @@ class MockPlatformChecker extends PlatformChecker with NoSuchMethodProvider {

@override
bool isReleaseMode() => isRelease;

@override
Platform get platform => _platform ?? super.platform;
}
Loading

0 comments on commit 68e3fb8

Please sign in to comment.