Allow users to set the maxLogLevel

[raoulmillais]

when users run into trouble with smoldot we have no way of asking them to capture more detailed logs from smoldot. we need a way to control the smoldot maxLogLevel when creating a client. E.g. #964

[raoulmillais]

I think making the log levels configurable isn't easy to do with the way connect has been designed. I thought about it hard for a couple of hours and put it to one side because i couldn't see an obvious solution without a big refactor or nasty trade offs.

I started hacking together an approach that would optionally allow passing the ClientOptions to createSclient. I pushed my WIP branch so you can see where I was going. Diff is here

The trade-offs of this approach that didn't sit right with me were:

• The extensionScClient and smoldotScClient no longer have identical signatures - I figured this is acceptable if I expose the createSmoldotScClient separately (which I did in my branch)
• For the case where createSmoldotScClient is called twice giving the illusion of two separate clients when there is actually a single (cached) smoldot client then the ClientOptions would have to be ignored on the second invocation. This use case is documented. That seems to me to be very surprising weird behaviour to an API consumer - they would reasonably expect the options to be honoured if they were passed regardless of which invocation.

I thought about making this work without that surprising behaviour by modifying smoldot to add a call which could change the options of an already running client. This just pushes the weird behaviour somewhere else though because it changes the options for all clients (we tell the library authors they can treat it as if they have 2 sc clients but really they only have one)

Ultimately I don't think this can be done nicely because of the way things are done at the moment. The loglevel is already configured to not filter out debug (but the developer has to remember to unfliter them in the console). But I think we should reconsider the docs for createScClient ... it feels wrong to me saying that its perfectly valid to call that twice. There is only really one ScClient pretending or alluding that they can treat it as multiple clients feels confusing to me. I think it's better to be upfront and say "there is only one ever one sc client" you can call this multiple times but it's always going to return the cached version and its not intended to be used that way. But maybe I'm missing something here.

cc @tomaka

[tomaka]

This is not about passing a ClientOptions, and indeed you cannot pass a ClientOptions. This is only about configuring maxLogLevel.

The options that you pass to createSmoldotScClient are a completely different thing than the options you pass to smoldot. Do as if they were completely unrelated and that the fact that they would both contain maxLogLevel is a happy little coincidence.

[raoulmillais]

Yeah that's an approach I'd also already considered but it doesn't address the concern I've outlined above .. we still end up with weird behaviour of the API if createScClient is called twice with different options

[raoulmillais]

If we do want to allow people to pass in a different maxLogLevel I think we also have to tell people not to call createScClient more than once and maybe even remove the reference counting and throw instead for multiple calls

[tomaka]

Users should be able pass a logCallback and maxLogLevel to createScClient.
Then you pass a log callback to smoldot, and this callback dispatches logs to the various log callbacks that have been registered.
If no logCallback/maxLogLevel is provided when calling createScClient, then it goes to console

[tomaka]

Actually, it's more appropriate to pass these to addChain so that the moment when the logs stop corresponds to removing the chain

[raoulmillais]

We can't add these to addChain the logCallback and the maxLogLevel can only be specified when the client is created .. this is the problem I'm referring to ... substrate connect instantiates the client once, caches it and reference counts how often its had chains added to it. Smoldot doesn't provide a way to have different options for different chains as things stand

[raoulmillais]

Maybe I need to restate things to make it clearer. At the time addChain is called it might be too late to say what logging options because substrate connect might have already initialised the smoldot client with the logging options for a previous call of addChain. That IMO is very surprising to the developer because it violates the expected contract - "i gave this function some options the second time I called it: why were they ignored"

[tomaka]

it might be too late to say what logging options because substrate connect might have already initialised the smoldot client with the logging options for a previous call of addChain.

And you solve that by passing a logCallback and maxLogLevel unconditionally to smoldot

As I've said above: the options that you pass to smoldot and the options passed to createScClient should be seen as two completely unrelated things. There's absolutely no need to pass the same logCallback and maxLogLevel to smoldot as were passed to createScClient.

[raoulmillais]

Moved to a draft PR so its easier to discuss and updated with some of the points in this discussion @tomaka

[tomaka]

After the change in this repo we'd also need to update the ScProvider as well

[wirednkod]

After the change in this repo we'd also need to update the ScProvider as well

his is done here
This, eventually was not implemented (at this point) at API due to the fact that it doesn't make logical sense to pass a config that works half of the time (meaning - only when extension is not present)

[wirednkod]

This issue can close as #1027 is merged

[tomaka]

We still need to add the option to the public API
Right now it's still not possible to do this, from a pragmatic standpoint, to set this max log level when using PolkadotJS