Skip to content

Commit

Permalink
refactor: use options.clock wherever possible (#1110)
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind committed Nov 10, 2022
1 parent 2f2a9a9 commit 25e9b59
Show file tree
Hide file tree
Showing 15 changed files with 88 additions and 27 deletions.
1 change: 1 addition & 0 deletions dart/lib/src/default_integrations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class RunZonedGuardedIntegration extends Integration {
final event = SentryEvent(
throwable: throwableMechanism,
level: SentryLevel.fatal,
timestamp: hub.options.clock(),
);

await hub.captureEvent(event, stackTrace: stackTrace);
Expand Down
1 change: 1 addition & 0 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ class FailedRequestClient extends BaseClient {
final event = SentryEvent(
throwable: throwableMechanism,
request: sentryRequest,
timestamp: _hub.options.clock(),
);

if (response != null) {
Expand Down
1 change: 1 addition & 0 deletions dart/lib/src/isolate_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Future<void> handleIsolateError(
final event = SentryEvent(
throwable: throwableMechanism,
level: SentryLevel.fatal,
timestamp: hub.options.clock(),
);

await hub.captureEvent(event, stackTrace: stackTrace);
Expand Down
12 changes: 6 additions & 6 deletions dart/lib/src/protocol/sentry_span.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class SentrySpan extends ISentrySpan {
this.samplingDecision,
Function({DateTime? endTimestamp})? finishedCallback,
}) {
_startTimestamp = startTimestamp?.toUtc() ?? getUtcDateTime();
_startTimestamp = startTimestamp?.toUtc() ?? _hub.options.clock();
_finishedCallback = finishedCallback;
}

Expand All @@ -42,20 +42,20 @@ class SentrySpan extends ISentrySpan {
return;
}

final utcDateTime = getUtcDateTime();

if (status != null) {
_status = status;
}

if (endTimestamp?.isBefore(_startTimestamp) ?? false) {
if (endTimestamp == null) {
_endTimestamp = _hub.options.clock();
} else if (endTimestamp.isBefore(_startTimestamp)) {
_hub.options.logger(
SentryLevel.warning,
'End timestamp ($endTimestamp) cannot be before start timestamp ($_startTimestamp)',
);
_endTimestamp = utcDateTime;
_endTimestamp = _hub.options.clock();
} else {
_endTimestamp = endTimestamp?.toUtc() ?? utcDateTime;
_endTimestamp = endTimestamp.toUtc();
}

// associate error
Expand Down
28 changes: 26 additions & 2 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class SentryOptions {

/// If [clock] is provided, it is used to get time instead of the system
/// clock. This is useful in tests. Should be an implementation of [ClockProvider].
/// The ClockProvider is expected to return UTC time.
@internal
ClockProvider clock = getUtcDateTime;

int _maxBreadcrumbs = 100;
Expand Down Expand Up @@ -87,6 +89,7 @@ class SentryOptions {
_maxSpans = maxSpans;
}

// ignore: deprecated_member_use_from_same_package
SentryLogger _logger = noOpLogger;

/// Logger interface to log useful debugging information if debug is enabled
Expand Down Expand Up @@ -117,10 +120,12 @@ class SentryOptions {

set debug(bool newValue) {
_debug = newValue;
// ignore: deprecated_member_use_from_same_package
if (_debug == true && logger == noOpLogger) {
_logger = dartLogger;
_logger = _debugLogger;
}
if (_debug == false && logger == dartLogger) {
if (_debug == false && logger == _debugLogger) {
// ignore: deprecated_member_use_from_same_package
_logger = noOpLogger;
}
}
Expand Down Expand Up @@ -359,6 +364,23 @@ class SentryOptions {
@internal
late SentryClientAttachmentProcessor clientAttachmentProcessor =
SentryClientAttachmentProcessor();

void _debugLogger(
SentryLevel level,
String message, {
String? logger,
Object? exception,
StackTrace? stackTrace,
}) {
log(
'[${level.name}] $message',
level: level.toDartLogLevel(),
name: logger ?? 'sentry',
time: clock(),
error: exception,
stackTrace: stackTrace,
);
}
}

/// This function is called with an SDK specific event object and can return a modified event
Expand Down Expand Up @@ -391,6 +413,7 @@ typedef TracesSamplerCallback = double? Function(
SentrySamplingContext samplingContext);

/// A NoOp logger that does nothing
@Deprecated('This will be made private or removed in the future')
void noOpLogger(
SentryLevel level,
String message, {
Expand All @@ -400,6 +423,7 @@ void noOpLogger(
}) {}

/// A Logger that prints out the level and message
@Deprecated('This will be made private or removed in the future')
void dartLogger(
SentryLevel level,
String message, {
Expand Down
3 changes: 1 addition & 2 deletions dart/lib/src/sentry_tracer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import 'dart:async';

import 'package:intl/intl.dart';
import 'package:meta/meta.dart';
import 'utils.dart';

import '../sentry.dart';
import 'sentry_tracer_finish_status.dart';
Expand Down Expand Up @@ -72,7 +71,7 @@ class SentryTracer extends ISentrySpan {

@override
Future<void> finish({SpanStatus? status, DateTime? endTimestamp}) async {
final commonEndTimestamp = endTimestamp ?? getUtcDateTime();
final commonEndTimestamp = endTimestamp ?? _hub.options.clock();
_autoFinishAfterTimer?.cancel();
_finishStatus = SentryTracerFinishStatus.finishing(status);
if (!_rootSpan.finished &&
Expand Down
6 changes: 5 additions & 1 deletion dart/test/sentry_options_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ void main() {

test('SentryLogger sets a diagnostic logger', () {
final options = SentryOptions(dsn: fakeDsn);
// ignore: deprecated_member_use_from_same_package
expect(options.logger, noOpLogger);
// ignore: deprecated_member_use_from_same_package
options.logger = dartLogger;

expect(false, options.logger == noOpLogger);
// ignore: deprecated_member_use_from_same_package
expect(options.logger, isNot(noOpLogger));
});

test('tracesSampler is null by default', () {
Expand Down
9 changes: 6 additions & 3 deletions dart/test/sentry_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -342,20 +342,23 @@ void main() {
}, options: sentryOptions);
});

test('options.logger is not dartLogger after debug = false', () async {
test('options.logger is set by setting the debug flag', () async {
final sentryOptions =
SentryOptions(dsn: fakeDsn, checker: FakePlatformChecker.debugMode());

await Sentry.init((options) {
options.dsn = fakeDsn;
options.debug = true;
expect(options.logger, dartLogger);
// ignore: deprecated_member_use_from_same_package
expect(options.logger, isNot(noOpLogger));

options.debug = false;
// ignore: deprecated_member_use_from_same_package
expect(options.logger, noOpLogger);
}, options: sentryOptions);

expect(sentryOptions.logger == dartLogger, false);
// ignore: deprecated_member_use_from_same_package
expect(sentryOptions.logger, isNot(dartLogger));
});

group("Sentry init optionsConfiguration", () {
Expand Down
2 changes: 2 additions & 0 deletions flutter/lib/src/integrations/flutter_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class FlutterErrorIntegration extends Integration<SentryFlutterOptions> {
contexts: flutterErrorDetails.isNotEmpty
? (Contexts()..['flutter_error_details'] = flutterErrorDetails)
: null,
// ignore: invalid_use_of_internal_member
timestamp: options.clock(),
);

await hub.captureEvent(event, stackTrace: errorDetails.stack);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
'Scheduler binding is null. Can\'t auto detect app start time.');
} else {
schedulerBinding.addPostFrameCallback((timeStamp) {
_native.appStartEnd = DateTime.now().toUtc();
// ignore: invalid_use_of_internal_member
_native.appStartEnd = options.clock();
});
}
}
Expand Down
2 changes: 2 additions & 0 deletions flutter/lib/src/integrations/on_error_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
var event = SentryEvent(
throwable: throwableMechanism,
level: SentryLevel.fatal,
// ignore: invalid_use_of_internal_member
timestamp: options.clock(),
);

// unawaited future
Expand Down
6 changes: 6 additions & 0 deletions flutter/lib/src/navigation/sentry_navigator_observer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
navigationType: type,
from: _routeNameExtractor?.call(from) ?? from,
to: _routeNameExtractor?.call(to) ?? to,
// ignore: invalid_use_of_internal_member
timestamp: _hub.options.clock(),
data: _additionalInfoProvider?.call(from, to),
));
}
Expand Down Expand Up @@ -232,6 +234,7 @@ class RouteObserverBreadcrumb extends Breadcrumb {
RouteSettings? from,
RouteSettings? to,
SentryLevel? level,
DateTime? timestamp,
Map<String, dynamic>? data,
}) {
final dynamic fromArgs = _formatArgs(from?.arguments);
Expand All @@ -243,6 +246,7 @@ class RouteObserverBreadcrumb extends Breadcrumb {
toArgs: toArgs,
navigationType: navigationType,
level: level,
timestamp: timestamp,
data: data,
);
}
Expand All @@ -254,11 +258,13 @@ class RouteObserverBreadcrumb extends Breadcrumb {
String? to,
dynamic toArgs,
SentryLevel? level,
DateTime? timestamp,
Map<String, dynamic>? data,
}) : super(
category: _navigationKey,
type: _navigationKey,
level: level,
timestamp: timestamp,
data: <String, dynamic>{
'state': navigationType,
if (from != null) 'from': from,
Expand Down
10 changes: 10 additions & 0 deletions flutter/lib/src/widgets_binding_observer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
data: <String, String>{
'state': _lifecycleToString(state),
},
// ignore: invalid_use_of_internal_member
timestamp: _options.clock(),
));
}

Expand All @@ -95,6 +97,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
category: 'device.screen',
type: 'navigation',
data: data,
// ignore: invalid_use_of_internal_member
timestamp: _options.clock(),
));
}

Expand All @@ -117,6 +121,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
data: <String, String>{
'action': 'BRIGHTNESS_CHANGED_TO_${brightnessDescription.toUpperCase()}'
},
// ignore: invalid_use_of_internal_member
timestamp: _options.clock(),
));
}

Expand All @@ -136,6 +142,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
data: <String, String>{
'action': 'TEXT_SCALE_CHANGED_TO_$newTextScaleFactor'
},
// ignore: invalid_use_of_internal_member
timestamp: _options.clock(),
));
}

Expand All @@ -162,6 +170,8 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver {
},
// This is kinda bad. Therefore this gets added as a warning.
level: SentryLevel.warning,
// ignore: invalid_use_of_internal_member
timestamp: _options.clock(),
));
}

Expand Down
13 changes: 8 additions & 5 deletions flutter/test/integrations/native_app_start_integration_test.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
@TestOn('vm')

import 'package:flutter_test/flutter_test.dart';
import 'package:mockito/mockito.dart';
import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_flutter/src/integrations/native_app_start_integration.dart';
import 'package:sentry_flutter/src/sentry_native.dart';
Expand All @@ -25,7 +26,7 @@ void main() {
fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10);
fixture.wrapper.nativeAppStart = NativeAppStart(0, true);

fixture.getNativeAppStartIntegration().call(MockHub(), fixture.options);
fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer);
Expand All @@ -44,7 +45,7 @@ void main() {
fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10);
fixture.wrapper.nativeAppStart = NativeAppStart(0, true);

fixture.getNativeAppStartIntegration().call(MockHub(), fixture.options);
fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer);
Expand All @@ -63,7 +64,7 @@ void main() {
fixture.wrapper.nativeAppStart = NativeAppStart(0, true);
final measurement = SentryMeasurement.warmAppStart(Duration(seconds: 1));

fixture.getNativeAppStartIntegration().call(MockHub(), fixture.options);
fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer).copyWith();
Expand All @@ -83,7 +84,7 @@ void main() {
fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(60001);
fixture.wrapper.nativeAppStart = NativeAppStart(0, true);

fixture.getNativeAppStartIntegration().call(MockHub(), fixture.options);
fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer);
Expand All @@ -97,13 +98,15 @@ void main() {
}

class Fixture {
final hub = MockHub();
final options = SentryFlutterOptions(dsn: fakeDsn);
final wrapper = MockNativeChannel();
late final native = SentryNative();

Fixture() {
native.setNativeChannel(wrapper);
native.reset();
when(hub.options).thenReturn(options);
}

NativeAppStartIntegration getNativeAppStartIntegration() {
Expand All @@ -124,6 +127,6 @@ class Fixture {
'op',
samplingDecision: SentryTracesSamplingDecision(sampled!),
);
return SentryTracer(context, MockHub());
return SentryTracer(context, hub);
}
}
Loading

0 comments on commit 25e9b59

Please sign in to comment.