[go_router] Execute log method when `hierarchicalLoggingEnabled` is true regardless of `debugLogDiagnostics` value #139667

ycherniavskyi · 2023-12-06T16:34:02Z

Is there an existing issue for this?

I have searched the existing issues
I have read the guide to filing a bug

Steps to reproduce

Create a project with some routes using go_router version >=11.1.2.
set hierarchicalLoggingEnabled to true in the main function of the project
listen to onRecord stream of Logger('GoRouter')

Before flutter/packages#4875 everything works as described in expected resiles.

Expected results

Get event from onRecord stream of Logger('GoRouter') if hierarchicalLoggingEnabled is true regardless of debugLogDiagnostics value.

Because, if I understand it right, then if hierarchicalLoggingEnabled is true it means that you must have to control the whole logging logic from one place - the logging package, without interaction with each package that outputs logs separately.

Actual results

If debugLogDiagnostics is false then no events from onRecord stream of Logger('GoRouter') are received at all.
But if debugLogDiagnostics is true then events from onRecord stream of Logger('GoRouter') are received but they doubled in the console with default developer.log executed by go_router.

Get event from onRecord stream of Logger('GoRouter') regardless of debugLogDiagnostics value.

Proposed fix

To achieve the expected results the following condition must be extended.

From

/// Logs the message if logging is enabled.
void log(String message, {Level level = Level.INFO}) {
  if (_enabled) {
    logger.log(level, message);
  }
}

To:

/// Logs the message if logging is enabled.
void log(String message, {Level level = Level.INFO}) {
  if (_enabled || hierarchicalLoggingEnabled) {
    logger.log(level, message);
  }
}

The text was updated successfully, but these errors were encountered:

darshankawar · 2023-12-07T07:24:28Z

Thanks for the report @ycherniavskyi
Can you take a look at this issue and see if it resembles your case or not ?

ycherniavskyi · 2023-12-09T21:17:46Z

@darshankawar, my issue differs. From what I understand, setting hierarchicalLoggingEnabled to true actually resolves the issue you referenced.

Specifically, my concern aligns with flutter/packages#4875, which is due to the introduction of this condition. This condition effectively disables all logger calls regardless of the logging mode, which is controlled by the hierarchicalLoggingEnabled flag.

darshankawar · 2023-12-11T05:25:25Z

Thanks for the feedback. Seeing the same result as reported.

stable, master flutter doctor -v

[!] Flutter (Channel stable, 3.16.3, on macOS 12.2.1 21D62 darwin-x64, locale
    en-GB)
    • Flutter version 3.16.3 on channel stable at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision b0366e0a3f (5 days ago), 2023-12-05 19:46:39 -0800
    • Engine revision 54a7145303
    • Dart version 3.2.3
    • DevTools version 2.28.4
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

[!] Flutter (Channel master, 3.18.0-7.0.pre.58, on macOS 12.2.1 21D62
    darwin-x64, locale en-GB)
    • Flutter version 3.18.0-7.0.pre.58 on channel master at
      /Users/dhs/documents/fluttersdk/flutter
    ! Warning: `flutter` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/flutter, which is not inside
      your current Flutter SDK checkout at
      /Users/dhs/documents/fluttersdk/flutter. Consider adding
      /Users/dhs/documents/fluttersdk/flutter/bin to the front of your path.
    ! Warning: `dart` on your path resolves to
      /Users/dhs/Documents/Fluttersdk/flutter/bin/dart, which is not inside your
      current Flutter SDK checkout at /Users/dhs/documents/fluttersdk/flutter.
      Consider adding /Users/dhs/documents/fluttersdk/flutter/bin to the front
      of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision c0a481e31d (7 hours ago), 2023-12-10 17:46:24 -0500
    • Engine revision bc0222b64c
    • Dart version 3.3.0 (build 3.3.0-214.0.dev)
    • DevTools version 2.30.0
    • If those were intentional, you can disregard the above warnings; however
      it is recommended to use "git" directly to perform update checks and
      upgrades.

[!] Android toolchain - develop for Android devices (Android SDK version 30.0.3)
    • Android SDK at /Users/dhs/Library/Android/sdk
    ✗ cmdline-tools component is missing
      Run `path/to/sdkmanager --install "cmdline-tools;latest"`
      See https://developer.android.com/studio/command-line for more details.
    ✗ Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/macos#android-setup for
      more details.

[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 13C100
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] IntelliJ IDEA Ultimate Edition (version 2021.3.2)
    • IntelliJ at /Applications/IntelliJ IDEA.app
    • Flutter plugin version 65.1.4
    • Dart plugin version 213.7228

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.29.0

[✓] Connected device (3 available)
    • Darshan's iphone (mobile) • 21150b119064aecc249dfcfe05e259197461ce23 • ios
      • iOS 15.3.1 19D52
    • macOS (desktop)           • macos                                    •
      darwin-x64     • macOS 12.2.1 21D62 darwin-x64
    • Chrome (web)              • chrome                                   •
      web-javascript • Google Chrome 109.0.5414.119

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.
      
[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

markbeij · 2023-12-13T12:17:36Z

Is there a workaround?

chunhtai · 2023-12-14T17:33:59Z

cc @ValentinVignal

ValentinVignal · 2023-12-15T10:50:54Z

To achieve the expected results the following condition must be extended.

From

/// Logs the message if logging is enabled.
void log(String message, {Level level = Level.INFO}) {
  if (_enabled) {
    logger.log(level, message);
  }
}

To:

/// Logs the message if logging is enabled.
void log(String message, {Level level = Level.INFO}) {
 if (_enabled || hierarchicalLoggingEnabled) {
    logger.log(level, message);
  }
}

Wouldn't it be weird to have logs even though enabled = false ?

Maybe it would be better to modify setLogging instead ?

https://github.com/flutter/packages/blob/ca16173e67e42076996f1f9957479d8ad847c80b/packages/go_router/lib/src/logging.dart#L28-L36

And change it to

void setLogging({bool enabled = false}) {
  _subscription?.cancel();
  _enabled = enabled;
-  if (!enabled) {
+  if (!enabled || hierarchicalLoggingEnabled) {
    return;
  }

  _subscription = logger.onRecord.listen((LogRecord e) {

At least, it wouldn't log anything when enabled is false

ycherniavskyi · 2023-12-17T15:14:42Z

@ValentinVignal, I completely agree with your suggestion – it's definitely clearer than what I had proposed.

Fixes flutter/flutter#139667 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

github-actions · 2024-05-14T06:00:38Z

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

…ter#6019) Fixes flutter/flutter#139667 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

darshankawar added the in triage Presently being triaged by the triage team label Dec 7, 2023

darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Dec 7, 2023

github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Dec 9, 2023

chunhtai added P2 Important issues not at the top of the work list triaged-go_router Triaged by Go Router team labels Dec 14, 2023

ValentinVignal mentioned this issue Jan 31, 2024

[go_router] Don't log if hierarchicalLoggingEnabled is true flutter/packages#6019

Merged

11 tasks

darshankawar mentioned this issue Feb 21, 2024

GoRouter have duplicated logs #143704

Closed

auto-submit bot closed this as completed in flutter/packages#6019 Apr 29, 2024

darshankawar added the r: fixed Issue is closed as already fixed in a newer version label Apr 30, 2024

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[go_router] Execute log method when `hierarchicalLoggingEnabled` is true regardless of `debugLogDiagnostics` value #139667

[go_router] Execute log method when `hierarchicalLoggingEnabled` is true regardless of `debugLogDiagnostics` value #139667

ycherniavskyi commented Dec 6, 2023 •

edited

darshankawar commented Dec 7, 2023

ycherniavskyi commented Dec 9, 2023

darshankawar commented Dec 11, 2023

markbeij commented Dec 13, 2023

chunhtai commented Dec 14, 2023

ValentinVignal commented Dec 15, 2023

ycherniavskyi commented Dec 17, 2023

github-actions bot commented May 14, 2024

[go_router] Execute log method when hierarchicalLoggingEnabled is true regardless of debugLogDiagnostics value #139667

[go_router] Execute log method when hierarchicalLoggingEnabled is true regardless of debugLogDiagnostics value #139667

Comments

ycherniavskyi commented Dec 6, 2023 • edited

Is there an existing issue for this?

Steps to reproduce

Expected results

Actual results

Proposed fix

darshankawar commented Dec 7, 2023

ycherniavskyi commented Dec 9, 2023

darshankawar commented Dec 11, 2023

markbeij commented Dec 13, 2023

chunhtai commented Dec 14, 2023

ValentinVignal commented Dec 15, 2023

ycherniavskyi commented Dec 17, 2023

github-actions bot commented May 14, 2024

[go_router] Execute log method when `hierarchicalLoggingEnabled` is true regardless of `debugLogDiagnostics` value #139667

[go_router] Execute log method when `hierarchicalLoggingEnabled` is true regardless of `debugLogDiagnostics` value #139667

ycherniavskyi commented Dec 6, 2023 •

edited