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

Incompatiblity with recordStackTraceAtLevel from the Logging package #1580

Closed
maBarabas opened this issue Aug 3, 2023 · 2 comments · Fixed by #1690
Closed

Incompatiblity with recordStackTraceAtLevel from the Logging package #1580

maBarabas opened this issue Aug 3, 2023 · 2 comments · Fixed by #1690

Comments

@maBarabas
Copy link
Contributor

maBarabas commented Aug 3, 2023

Platform

Flutter Mobile

Obfuscation

Enabled

Debug Info

Enabled

Doctor

[!] Flutter (Channel unknown, 3.10.6, on macOS 13.4 22F66 darwin-x64, locale en-GB)
    ! Flutter version 3.10.6 on channel unknown at /Users/barabas/src/flutter
      Currently on an unknown channel. Run `flutter channel` to switch to an official channel.
      If that doesn't fix the issue, reinstall Flutter by following instructions at https://flutter.dev/docs/get-started/install.
    ! Unknown upstream repository.
      Reinstall Flutter by following instructions at https://flutter.dev/docs/get-started/install.
    • Framework revision f468f3366c (3 weeks ago), 2023-07-12 15:19:05 -0700
    • Engine revision cdbeda788a
    • Dart version 3.0.6
    • DevTools version 2.23.1
    • 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 34.0.0)
    • Android SDK at /Users/barabas/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/barabas/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14E300c
    • CocoaPods version 1.11.3

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

[✓] Android Studio (version 2022.3)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b829.9-10027231)

[✓] Connected device (3 available)
    • Pixel 5 (mobile) • 0C221FDD4003LH • android-arm64  • Android 13 (API 33)
    • macOS (desktop)  • macos          • darwin-x64     • macOS 13.4 22F66 darwin-x64
    • Chrome (web)     • chrome         • web-javascript • Google Chrome 113.0.5672.126
    ! Error: iPhone XR has recently restarted. Xcode will continue when iPhone XR is unlocked. (code -14)

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

! Doctor found issues in 1 category.

Version

7.9.0

Steps to Reproduce

  1. Set up a project using the logging package integration
  2. Set recordStackTraceAtLevel = Level.SEVERE. The makes Logging automatically set the error to a string, and assign the current stacktrace to the log.
  3. Fire a severe log: Logger('test').severe('hello world')

The snippet from Logger.log that auto generates the String is:


    if (stackTrace == null && logLevel >= recordStackTraceAtLevel) {
      stackTrace = StackTrace.current;
      error ??= 'autogenerated stack trace for $logLevel $msg';
    }

Expected Result

Error is captured, and sentry does not print internal errors.

Sentry logging integration should be compatible with all types accepted by the Logging package. The signature or log is:

void log(
    Level logLevel,
    Object message,
    [Object? error,
    StackTrace stackTrace,
    Zone? zone]
)

Therefore, Sentry should not have internal errors regardless of the type of the error. At the very least, it needs to support String for this use case. I guess this can be worked around by constructing an Exception from the String, but this must be done inside Sentry.

Actual Result

Sentry logs an internal error:

[sentry] [info] Throwable type: String is not supported for associating errors to a transaction.
         Invalid argument (object): Cannot be a string, number, boolean, record, null, Pointer, Struct or Union: "autogenerated stack tra...
         #0      checkValidWeakTarget (dart:_internal-patch/internal_patch.dart:220:5)
         #1      Expando.[] (dart:core-patch/expando_patch.dart:31:5)
         #2      _WeakMap.get (package:sentry/src/hub.dart:619:22)
         #3      Hub._assignTraceContext (package:sentry/src/hub.dart:559:37)
         #4      Hub.captureEvent (package:sentry/src/hub.dart:94:19)
         <asynchronous suspension>
         #5      LoggingIntegration._onLog (package:sentry_logging/src/logging_integration.dart:58:7)
         <asynchronous suspension>

It seems like the error is captured regardless. Maybe it will be missing from the transation that was in progress.

Are you willing to submit a PR?

None

@krystofwoldrich
Copy link
Member

krystofwoldrich commented Aug 4, 2023

Hi,
thank you for the message and the details. We can't reproduce the behavior.

Here is an example stack trace reported to Sentry.

Code snippet:

recordStackTraceAtLevel = Level.SEVERE;
final log = Logger('Logging');
log.severe('My Logging test severe');

Can you share a minimal reproducible example?

@getsantry getsantry bot removed the status in GitHub Issues with 👀 Aug 4, 2023
@krystofwoldrich krystofwoldrich moved this from Waiting for: Product Owner to Waiting for: Community in GitHub Issues with 👀 Aug 4, 2023
@krystofwoldrich krystofwoldrich moved this from Needs Discussion to Needs More Information in Mobile & Cross Platform SDK Aug 4, 2023
@marandaneto
Copy link
Contributor

Hi @maBarabas
Yes, the errors and transactions won't be linked if the error type isn't supported.

_options.logger(
SentryLevel.info,
'Throwable type: ${throwable.runtimeType} is not supported for associating errors to a transaction.',
exception: exception,
stackTrace: stackTrace,
);

This is just an info logging so nothing to be worried about.

I guess this can be worked around by constructing an Exception from the String, but this must be done inside Sentry.

This could be done but the problem is that if the same error is thrown twice, since a new object is constructed every time, the expando won't identify as the same object.
It's a limitation of the Expando and WeakReference classes in Dart.

Maybe if the wrapper exception overrides bool operator ==(Object other) and int get hashCode;, I have to check this out.

@marandaneto marandaneto moved this from Needs More Information to Needs Investigation in Mobile & Cross Platform SDK Aug 7, 2023
@buenaflor buenaflor moved this from Needs Investigation to In Progress in Mobile & Cross Platform SDK Oct 23, 2023
@buenaflor buenaflor moved this from In Progress to Needs Review in Mobile & Cross Platform SDK Oct 23, 2023
@github-project-automation github-project-automation bot moved this from Needs Review to Done in Mobile & Cross Platform SDK Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants