Logging overhaul

[patmuk]

Changes

Please list issues fixed by this PR here, using format "Fixes #the-issue-number".

Checklist

• An issue to be fixed by this PR is listed above.
• New tests are added to ensure new features are working. Please refer to this page to see how to add a test.
• ./frb_internal precommit --mode slow (or fast) is run (it internal runs code generator, does auto formatting, etc).
• If this PR adds/changes features, documentations (in the ./website folder) are updated.
• CI is passing. Please refer to this page to see how to solve a failed CI.

Remark for PR creator

• ./frb_internal --help shows utilities for development.
• If fzyzcjy does not reply for a few days, maybe he just did not see it, so please ping him.

[patmuk]

@fzyzcjy I did not know that :) Much better to discuss here as well :)

I don't know how to write tests for checking the logging output.

Can I provide a dart_logging example, showing what can be done, and additionally talking care that it still compiles in the future? Or would that be too many examples?

[fzyzcjy]

I don't know how to write tests for checking the logging output.

I guess you can add a callback in dart logging package (just like you add callback to print), to assert you get something.

Can I provide a dart_logging example, showing what can be done, and additionally talking care that it still compiles in the future? Or would that be too many examples?

Firstly try to put tests in pure_dart (probably add a new file, mimicking https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/frb_example/pure_dart/rust/src/api/misc_no_twin_example_a.rs); if really cannot, feel free to discuss why and maybe create a new package!

[patmuk]

@fzyzcjy I will take care of the testing later ... for now I moved my modifications on dart_minimal to a new package dart_logging - which I will delete once the code is integrated into frb and the way you propose testing works :)

However, I need some advice on how to integrate it :)
I initially thought of adding my files to the runtimes, frb_dart and frb_rust.
But the (only) files needed (rust/src/api/log_2_dart.rs and lib/Logger.dart) are using code generation for

• a Steam_Sink
• en-/decryption of a custom type (Log Level)

Then again, the generated files are project independent and will always be the same.

So, I could either add my code & the needed parts of the generated code to the runtimes or I could modify the code generation so that my code gets added to the generated files.

I think adding it to the runtimes would be conceptually better and easier, as this is nothing the user is supposed to modify.
Do you agree?

[fzyzcjy]

for now I moved my modifications

Btw you can even just modify dart_minimal temporarily when developing it. I usually do this (modify dart_minimal, do work, after it is done move test to pure_dart)

[fzyzcjy]

That's a good question. Especially the problem is, we need to let frb handle generation of several functions (e.g. the function from rust to dart).

Thus one way may be, a macro like

macro_rules! hello_logging {
  () => {
    #[frb(dart_code="put your whole big dart code here")]
    pub fn put_your_rust_functions_here() {}
  }
}

And we may modify the default template to call this hello_logging!.

[patmuk]

Thanks, that sounds good!

I will first try to implement as much as I can without code generation in the runtimes, so I don't slow down the generation.
Whatever is left and needs to be generated I will try the macro way :)

[fzyzcjy]

Btw no worries about generation slowdown - such several small function should not take noticable time (o/w frb will be useless for larger projects)

[patmuk]

Btw no worries about generation slowdown - such several small function should not take noticable time (o/w frb will be useless for larger projects)

that is true! Premature optimization ...

[patmuk]

I ran into a problem with the thread handler.

I thought we don't need code generation - as everything needed is independent of the user's code.
So I moved, in my latest commit, the hand-written and generated code to frb_rust and frb_dart, thinking that these will be automatically included by frb. Changed all imports to the files in this packages.

However, When running codegen generate in dart_logging I am getting the following errors

compile errors

``` error[E0433]: failed to resolve: use of undeclared crate or module `flutter_rust_bridge` --> /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/for_generated/boilerplate.rs:216:21 | 216 | flutter_rust_bridge::for_generated::FLUTTER_RUST_BRIDGE_RUNTIME_VERSION, | ^^^^^^^^^^^^^^^^^^^ use of undeclared crate or module `flutter_rust_bridge` | ::: /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/logging/frb_generated.rs:47:1 | 47 | crate::frb_generated_default_handler!(); | --------------------------------------- in this macro invocation | = note: this error originates in the macro `crate::frb_generated_default_handler` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider importing this module --> /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/logging/frb_generated.rs:28:1 | 28 + use crate::for_generated; | error[E0433]: failed to resolve: use of undeclared crate or module `flutter_rust_bridge` --> /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/for_generated/boilerplate.rs:219:21 | 219 | flutter_rust_bridge::for_generated::FLUTTER_RUST_BRIDGE_RUNTIME_VERSION, | ^^^^^^^^^^^^^^^^^^^ use of undeclared crate or module `flutter_rust_bridge` | ::: /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/logging/frb_generated.rs:47:1 | 47 | crate::frb_generated_default_handler!(); | --------------------------------------- in this macro invocation | = note: this error originates in the macro `crate::frb_generated_default_handler` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider importing this module --> /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/logging/frb_generated.rs:28:1 | 28 + use crate::for_generated; | error[E0425]: cannot find function `set_boxed_logger` in crate `log` --> /Users/patmuk/code/own/oss/frb/flutter_rust_bridge_fork_pat/frb_rust/src/logging/log_2_dart.rs:12:10 | 12 | log::set_boxed_logger(Box::new(Log2Dart { | ^^^^^^^^^^^^^^^^ not found in `log` | note: found an item that was configured out --> /Users/patmuk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/log-0.4.22/src/lib.rs:1337:8 | 1337 | pub fn set_boxed_logger(logger: Box) -> Result<(), SetLoggerError> { | ^^^^^^^^^^^^^^^^ note: the item is gated here --> /Users/patmuk/.cargo/registry/src/index.crates.io-6f17d22bba15001f/log-0.4.22/src/lib.rs:1336:1 ```

As I understand it, the Macro crate::frb_generated_default_handler!(); is called from my code in frb_rust - but it should be called from the user project (so a user can change to an own handler implementation). However, I cannot just remove that, as FLUTTER_RUST_BRIDGE_HANDLER would not be set.

How to progress here? Should we hardcode the logger to use the default handler? Or is there a way to wire it to wait & use the user instantiated handler?

Or - is that best to be code generated, and thus implement the logging code into a macro, as you actually suggested?

I will try implementing the macro now.

[patmuk]

Small update: I fixed the error by indeed instantiating the default handler:

// Section: executor
// TODO fix this for web
// crate::frb_generated_default_handler!();
pub static FLUTTER_RUST_BRIDGE_HANDLER: LazyLock<
    // pub static FLUTTER_RUST_BRIDGE_HANDLER_LOGGER: LazyLock<
    DefaultHandler<for_generated::SimpleThreadPool>,
> = { LazyLock::new(|| DefaultHandler::new_simple(Default::default())) };

Will that be problematic if somebody implements his own thread handler?

For now, I am continuing with the "runtime integration" approach.

[fzyzcjy]

Well, I guess it is great to use the HANDLER object that the user chooses. Otherwise it is very confusing or even buggy to have two handlers co-exist.

[patmuk]

Well, I guess it is great to use the HANDLER object that the user chooses. Otherwise it is very confusing or even buggy to have two handlers co-exist.

Yes, I agree ... but how to solve that? ... by implementing the marco like you suggested? Or is there another way to take the user defined handler?

[patmuk]

Hey @fzyzcjy,
again a question :) I am implementing the macro, as you suggested.

I am confused about this:

the package `frb_example_dart_logging` depends on `flutter_rust_bridge`, with features: `log, user-utils` but `flutter_rust_bridge` does not have these features.

I implemented the macro definition (setup_logging) in frb_rust/src/log_2_dart.rs.
The macro is called in pub fn setup_default_user_utils() in frb_rust/src/user_utils.rs - I removed the previous logging code (for iOS and Android).

I left the #[cfg(feature = "log")] and noticed in frb_rust/src/lib.rsa#[cfg(feature = "user-utils")]`.

However, it looks like my macro is not called ... how is setup_default_user_utils triggered?
I read somewhere that this is done automatically, but in dart_minimal I see

   3   │ #[frb(init)]
   4   │ pub fn init_app() {
   5   │     flutter_rust_bridge::setup_default_user_utils();
   6   │ }

in dart_minimal/rust/src/api/minimal.rs
And

   5   │ Future<void> main() async {
   6   │   await RustLib.init();
   7   │   print('Call Rust and get: 100+200 = ${await minimalAdder(a: 100, b: 200)}');
   8   │ }

in dart_minimal/lib/main.dart

Is await RustLib.init(); calling init_app, because it is annotated with #[frb(init)], but the call to flutter_rust_bridge::setup_default_user_utils(); is actually not needed, as it is compiled into await RustLib.init();?

But setup_default_user_utils() is not called, because the feature user-utils is not set in cargo.toml, and additionally the feature log as well not?

But If I set that in dart_minimal/rust/Cargo.toml I get the error that these feature flags are not existing ... where am I off?

Thanks :)

[fzyzcjy]

Good job!

Yes, I agree ... but how to solve that? ... by implementing the marco like you suggested? Or is there another way to take the user defined handler?

Macro may be the simplest...

again a question :)

macro_rules! hello_logging {
  () => {
    #[frb(dart_code="put your whole big dart code here")]
    #[frb(init)]
    pub fn this_function_will_also_be_auto_called_on_init() {
      do_whatever_rust_logging_setup_here;
    }
  }
}

and in frb_example

// users (or us) just add this line
hello_logging!();

fn other_normal_things();

For flags, iirc the default flags should be working

[patmuk]

Hey @fzyzcjy,
I am gain a bit stuck ...
I now implement it "the macro way".

As I had a conflict adding the logger (log::set_boxed_logger clashed with the implementation in frb_codegen/src/library/utils/logs.rs) I removed that code. It will be replaced by this implementation anyways. The breaking change is now that logger configuration is to be set from dart - I will implement env parsing there later as well. We cannot really have a hybrid setup, where some things are interpreted by rust and others by dart. And I don't think somebody needs different log levels for the dart vs rust log (if so we can implement that later).

However, I have the following problems:

1. the code in the macro (dart and rust) seems to not be integrated into the generated code (but the code generator triggers cargo expand).
2. the log dependency is not pulled (results in an (*ERROR*) when cargo expand
3. code generation for the Stream_sink is not triggered, leading to

error[E0599]: the method `add` exists for struct `StreamSink<Log2DartLogRecord>`, but its trait bounds were not satisfied
   --> src/api/logging_example.rs:122:30
    |
122 |                               .add(record.into())
    |                                ^^^ method cannot be called on `StreamSink<Log2DartLogRecord>` due to unsatisfied trait bounds
...
170 |               pub struct Log2DartLogRecord {
    |               --- doesn't satisfy `Log2DartLogRecord: SseEncode`
...
212 |       setup_logging!();
    |       ---------------- in this macro invocation
    |
   ::: src/frb_generated.rs:36:1
    |
36  | / flutter_rust_bridge::frb_generated_boilerplate!(
37  | |     default_stream_sink_codec = SseCodec,
38  | |     default_rust_opaque = RustOpaqueMoi,
39  | |     default_rust_auto_opaque = RustAutoOpaqueMoi,
40  | | );
    | |_- method `add` not found for this struct
    |
note: trait bound `Log2DartLogRecord: SseEncode` was not satisfied
   --> src/frb_generated.rs:36:1
    |
36  | / flutter_rust_bridge::frb_generated_boilerplate!(
37  | |     default_stream_sink_codec = SseCodec,
38  | |     default_rust_opaque = RustOpaqueMoi,
39  | |     default_rust_auto_opaque = RustAutoOpaqueMoi,
40  | | );
    | | ^ unsatisfied trait bound introduced here
    | |_|
    |
note: the trait `SseEncode` must be implemented
   --> src/frb_generated.rs:36:1
    |
36  | / flutter_rust_bridge::frb_generated_boilerplate!(
37  | |     default_stream_sink_codec = SseCodec,
38  | |     default_rust_opaque = RustOpaqueMoi,
39  | |     default_rust_auto_opaque = RustAutoOpaqueMoi,
40  | | );
    | |_^
    = help: items from traits can only be used if the trait is implemented and in scope
    = note: the following trait defines an item `add`, perhaps you need to implement it:
            candidate #1: `Add`
    = note: this error originates in the macro `setup_logging` which comes from the expansion of the macro `flutter_rust_bridge::frb_generated_boilerplate` (in Nightly builds, run with -Z macro-backtrace for more info)

I implemented the macro in frb_rust/src/log_2_dart.rs first. Had the log crate defined in frb_rust/Cargo.toml.
But this results in (2) when running ../../frb_internal generate-run-frb-codegen-command-generate --package frb_example--dart_loggingfrb_example/dart_logging` directory.

So, to go step-by-step, I moved the logging code back to dart_logging, but as a macro. But here I get the error in (3.).

Not sure to progress now :( Probably the code-generation in the macro within the frb_example/dart_loggingproject doesn't work, see 3.
But if the code is in frb_rust, the log dependency is not found, leading to the problem in 2.

[patmuk]

Oh, I just realized that I made my latest commits branching off that branch ... fixing this now so you can see the code.

Looks something else got messed up as well ... as dart_minimal isn't working ... have to clean up the code, please wait for my next message, you can ignore what I wrote above for now!

[fzyzcjy]

the log dependency

try to re-export the log in frb_rust/frb_generated.rs and use flutter_rust_bridge::for_generated::log instead of log::...

[fzyzcjy]

Looking at https://github.com/fzyzcjy/flutter_rust_bridge/pull/2303/files#diff-ef1c2facb4c5b6480560c71382b03598efa060ea377d481598450f76b342a19e seems you are not doing the macro way. When saying macro way, I mean put everything inside the macro, such that it is exactly as if the user has copy-paste each and every related code to their package.

No worries! By implementing that correctly I guess it would be ok

[patmuk]

When saying macro way, I mean put everything inside the macro, such that it is exactly as if the user has copy-paste each and every related code to their package.

Yes - I did not want to "throw away" that code, so I checked-out the revision before that changed and committed the new code I am writing based on that ... however, these commits are local only, just realized that now.

But as I probably broke something along the way, and I am behind master, I am now re-implementing my changes on the latest master and creat a new branch. Thus it is cleaner - was making too many edits back-and-forth in this branch :(

Thanks for the re-export tip!

[patmuk]

Closed as surpassed by the better approach in #2308