fix: options.diagnosticLevel not affecting logs (#2856)

* v9: Set log level to `warning` by default  (#2836)

* update

* update test

* update init native sdk test

* Update CHANGELOG

* Add additional test

* Update CHANGELOG

* Update CHANGELOG

* Update

* Update

* Fix test

* Fix test

* Fix analyze

* Remove prod scheme

* Update mocks

* Update mocks

Author: buenaflor

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- `options.diagnosticLevel` not affecting logs ([#2856](https://github.com/getsentry/sentry-dart/pull/2856))
+
 ## 8.14.1
 
 ### Fixes

dart/lib/src/diagnostic_logger.dart

@@ -2,8 +2,9 @@ import 'protocol.dart';
 import 'sentry_options.dart';
 
 class DiagnosticLogger {
-  final SentryLogger _logger;
   final SentryOptions _options;
+  final SentryLogger _logger;
+  SentryLogger get logger => _logger;
 
   DiagnosticLogger(this._logger, this._options);
 

dart/lib/src/sentry_options.dart

@@ -135,9 +135,13 @@ class SentryOptions {
   SentryLogger get logger => _logger;
 
   set logger(SentryLogger logger) {
-    _logger = DiagnosticLogger(logger, this).log;
+    diagnosticLogger = DiagnosticLogger(logger, this);
+    _logger = diagnosticLogger!.log;
   }
 
+  @visibleForTesting
+  DiagnosticLogger? diagnosticLogger;
+
   final List<EventProcessor> _eventProcessors = [];
 
   /// Are callbacks that run for every event. They can either return a new event which in most cases
@@ -159,13 +163,15 @@ class SentryOptions {
 
   set debug(bool newValue) {
     _debug = newValue;
-    // ignore: deprecated_member_use_from_same_package
-    if (_debug == true && logger == noOpLogger) {
-      _logger = _debugLogger;
+    if (_debug == true &&
+        // ignore: deprecated_member_use_from_same_package
+        (logger == noOpLogger || diagnosticLogger?.logger == noOpLogger)) {
+      logger = debugLogger;
     }
-    if (_debug == false && logger == _debugLogger) {
+    if (_debug == false &&
+        (logger == debugLogger || diagnosticLogger?.logger == debugLogger)) {
       // ignore: deprecated_member_use_from_same_package
-      _logger = noOpLogger;
+      logger = noOpLogger;
     }
   }
 
@@ -583,7 +589,8 @@ class SentryOptions {
   late SentryStackTraceFactory stackTraceFactory =
       SentryStackTraceFactory(this);
 
-  void _debugLogger(
+  @visibleForTesting
+  void debugLogger(
     SentryLevel level,
     String message, {
     String? logger,

dart/test/diagnostic_logger_test.dart

@@ -11,37 +11,39 @@ void main() {
     fixture = Fixture();
   });
 
-  test('$DiagnosticLogger do not log if debug is disabled', () {
-    fixture.options.debug = false;
+  group(DiagnosticLogger, () {
+    test('does not log if debug is disabled', () {
+      fixture.options.debug = false;
 
-    fixture.getSut().log(SentryLevel.error, 'foobar');
+      fixture.getSut().log(SentryLevel.error, 'foobar');
 
-    expect(fixture.loggedMessage, isNull);
-  });
+      expect(fixture.loggedMessage, isNull);
+    });
 
-  test('$DiagnosticLogger log if debug is enabled', () {
-    fixture.options.debug = true;
+    test('logs if debug is enabled', () {
+      fixture.options.debug = true;
 
-    fixture.getSut().log(SentryLevel.error, 'foobar');
+      fixture.getSut().log(SentryLevel.error, 'foobar');
 
-    expect(fixture.loggedMessage, 'foobar');
-  });
+      expect(fixture.loggedMessage, 'foobar');
+    });
 
-  test('$DiagnosticLogger do not log if level is too low', () {
-    fixture.options.debug = true;
-    fixture.options.diagnosticLevel = SentryLevel.error;
+    test('does not log if level is too low', () {
+      fixture.options.debug = true;
+      fixture.options.diagnosticLevel = SentryLevel.error;
 
-    fixture.getSut().log(SentryLevel.warning, 'foobar');
+      fixture.getSut().log(SentryLevel.warning, 'foobar');
 
-    expect(fixture.loggedMessage, isNull);
-  });
+      expect(fixture.loggedMessage, isNull);
+    });
 
-  test('$DiagnosticLogger always log fatal', () {
-    fixture.options.debug = false;
+    test('always logs fatal', () {
+      fixture.options.debug = false;
 
-    fixture.getSut().log(SentryLevel.fatal, 'foobar');
+      fixture.getSut().log(SentryLevel.fatal, 'foobar');
 
-    expect(fixture.loggedMessage, 'foobar');
+      expect(fixture.loggedMessage, 'foobar');
+    });
   });
 }
 

dart/test/sentry_options_test.dart

@@ -1,3 +1,5 @@
+// ignore_for_file: deprecated_member_use_from_same_package
+
 import 'package:http/http.dart';
 import 'package:sentry/sentry.dart';
 import 'package:sentry/src/noop_client.dart';
@@ -35,15 +37,32 @@ void main() {
 
   test('SentryLogger sets a diagnostic logger', () {
     final options = defaultTestOptions();
-    // ignore: deprecated_member_use_from_same_package
     expect(options.logger, noOpLogger);
-    // ignore: deprecated_member_use_from_same_package
     options.logger = dartLogger;
 
-    // ignore: deprecated_member_use_from_same_package
     expect(options.logger, isNot(noOpLogger));
   });
 
+  test('setting debug correctly sets logger', () {
+    final options = defaultTestOptions();
+    expect(options.logger, noOpLogger);
+    expect(options.diagnosticLogger, isNull);
+    options.debug = true;
+    expect(options.logger, isNot(options.debugLogger));
+    expect(options.diagnosticLogger!.logger, options.debugLogger);
+    expect(options.logger, options.diagnosticLogger!.log);
+
+    options.debug = false;
+    expect(options.logger, isNot(noOpLogger));
+    expect(options.diagnosticLogger!.logger, noOpLogger);
+    expect(options.logger, options.diagnosticLogger!.log);
+
+    options.debug = true;
+    expect(options.logger, isNot(options.debugLogger));
+    expect(options.diagnosticLogger!.logger, options.debugLogger);
+    expect(options.logger, options.diagnosticLogger!.log);
+  });
+
   test('tracesSampler is null by default', () {
     final options = defaultTestOptions();
 
@@ -112,15 +131,13 @@ void main() {
 
   test('when enableTracing is set to true tracing is considered enabled', () {
     final options = SentryOptions.empty();
-    // ignore: deprecated_member_use_from_same_package
     options.enableTracing = true;
 
     expect(options.isTracingEnabled(), true);
   });
 
   test('when enableTracing is set to false tracing is considered disabled', () {
     final options = SentryOptions.empty();
-    // ignore: deprecated_member_use_from_same_package
     options.enableTracing = false;
     options.tracesSampleRate = 1.0;
     options.tracesSampler = (_) {

dart/test/sentry_test.dart

@@ -1,3 +1,5 @@
+// ignore_for_file: deprecated_member_use_from_same_package
+
 import 'dart:async';
 
 import 'package:sentry/sentry.dart';
@@ -478,18 +480,18 @@ void main() {
       (options) {
         options.dsn = fakeDsn;
         options.debug = true;
-        // ignore: deprecated_member_use_from_same_package
-        expect(options.logger, isNot(noOpLogger));
+        expect(options.diagnosticLogger?.logger, isNot(noOpLogger));
 
         options.debug = false;
-        // ignore: deprecated_member_use_from_same_package
-        expect(options.logger, noOpLogger);
+        expect(options.diagnosticLogger?.logger, noOpLogger);
+
+        options.debug = true;
+        expect(options.diagnosticLogger?.logger, isNot(noOpLogger));
       },
       options: sentryOptions,
     );
 
-    // ignore: deprecated_member_use_from_same_package
-    expect(sentryOptions.logger, isNot(dartLogger));
+    expect(sentryOptions.diagnosticLogger?.logger, isNot(noOpLogger));
   });
 
   group('Sentry init optionsConfiguration', () {

flutter/lib/src/screenshot/recorder.dart

@@ -2,10 +2,10 @@ import 'dart:async';
 import 'dart:developer';
 import 'dart:ui';
 
+import 'package:flutter/cupertino.dart' as cupertino;
+import 'package:flutter/material.dart' as material;
 import 'package:flutter/rendering.dart';
 import 'package:flutter/widgets.dart' as widgets;
-import 'package:flutter/material.dart' as material;
-import 'package:flutter/cupertino.dart' as cupertino;
 import 'package:meta/meta.dart';
 
 import '../../sentry_flutter.dart';

flutter/test/mocks.mocks.dart

@@ -2020,12 +2020,16 @@ class MockWidgetsFlutterBinding extends _i1.Mock
   int scheduleFrameCallback(
     _i21.FrameCallback? callback, {
     bool? rescheduling = false,
+    bool? scheduleNewFrame = true,
   }) =>
       (super.noSuchMethod(
         Invocation.method(
           #scheduleFrameCallback,
           [callback],
-          {#rescheduling: rescheduling},
+          {
+            #rescheduling: rescheduling,
+            #scheduleNewFrame: scheduleNewFrame,
+          },
         ),
         returnValue: 0,
       ) as int);
@@ -2303,6 +2307,24 @@ class MockWidgetsFlutterBinding extends _i1.Mock
         returnValueForMissingStub: null,
       );
 
+  @override
+  void addSemanticsActionListener(
+    _i8.ValueSetter<_i6.SemanticsActionEvent>? listener,
+  ) =>
+      super.noSuchMethod(
+        Invocation.method(#addSemanticsActionListener, [listener]),
+        returnValueForMissingStub: null,
+      );
+
+  @override
+  void removeSemanticsActionListener(
+    _i8.ValueSetter<_i6.SemanticsActionEvent>? listener,
+  ) =>
+      super.noSuchMethod(
+        Invocation.method(#removeSemanticsActionListener, [listener]),
+        returnValueForMissingStub: null,
+      );
+
   @override
   _i12.SemanticsHandle ensureSemantics() => (super.noSuchMethod(
         Invocation.method(#ensureSemantics, []),