After the upgrade to 4 0 0 things seem to be

Question

< Casey Brooks> After the upgrade to 4 0 0 things seem to be mostly working but I am able to quite easily reproduce a `ClosedSendChannelException` in the browser after which my VMs stop working Its not clear from the console log where the exception is coming from Is it perhaps an incompatibility with Kotlin 1 9 20 4 0 0 appears to be compiled against 1 9 10

Casey Brooks · Answer

Can you please provide a sample repo so I can take a look

rocketraman · Answer

The exception appears to happen during a navigation so I m going to try to migrate my routing to ballast navigation and see if the issue still happens If it still does I ll try to create a reproducer

rocketraman · Answer

Yep still happening with ballast navigation

rocketraman · Answer

Ok the problem appears to be when doing this in a Composable ``` LaunchedEffect vm Or Unit vm send Inputs Initialize ``` Any thoughts on why this would be a problem < Casey Brooks> It worked fine with ballast 2 x

rocketraman · Answer

Switching to sending the `Inputs Initialize` via the `BootstrapInterceptor` results in the same error though with better logging as the error is caught by Ballast ``` SignIn Uncaught error Channel was closed SignIn Ballast error capture input sequence types are BallastLoggingException ClosedSendChannelException Channel was closed at protoOf doResume 5yljmg k$ webpack internal kotlin ballast ballast logging js 336 31 at protoOf invoke 2dr2y6 k$ webpack internal kotlin ballast ballast logging js 302 16 at onEach$o$collect$slambda l as $action 1 webpack internal kotlin ballast ballast logging js 631 16 at protoOf doResume 5yljmg k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 16461 34 at protoOf invoke oz8tte k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 16443 16 at sam$kotlinx coroutines flow FlowCollector$0 10 l as function 1 webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 16502 16 at protoOf emit t92u1f k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 16432 17 at protoOf emit t92u1f k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 21105 29 at protoOf doResume 5yljmg k$ webpack internal kotlin ballast ballast api js 4040 57 at protoOf invoke m1n198 k$ webpack internal kotlin ballast ballast api js 4025 16 Caused by ClosedSendChannelException Channel was closed at protoOf get sendException qpq1ry k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 8743 37 at onClosedSend webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 5532 31 at protoOf doResume 5yljmg k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 7733 37 at protoOf send 44jogj k$ webpack internal kotlin kotlinx coroutines kotlinx coroutines core js ir js 8168 16 at protoOf enqueue 9t430b k$ webpack internal kotlin ballast ballast api js 1950 30 at protoOf doResume 5yljmg k$ webpack internal kotlin ballast ballast api js 3576 88 at protoOf enqueueQueued vbcpz3 k$ webpack internal kotlin ballast ballast api js 3865 16 at protoOf sendToQueue bkzj57 k$ webpack internal kotlin ballast ballast api js 5405 30 at protoOf doResume 5yljmg k$ webpack internal kotlin ballast ballast utils js 230 48 at protoOf resumeWith b9cu3x k$ webpack internal kotlin kotlin kotlin stdlib js 6846 34 ```

rocketraman · Answer

< Casey Brooks> Figured it out my vm configs are mostly singletons that come from my DI system I don t know if this is the intended behavior or not for ballast but having the configs be singletons did work fine in 2 x However in 4 x it causes the exception seen above I ve created a repro for you here

Casey Brooks · Answer

The intention has always been that the VM configs and config builders are not reused between VMs Even the same VM type which creates multiple instances should still create new configs Your DI setup should manage them as factories not singletons <https copper leaf github io ballast wiki usage architecture dependency injection|This page> in the docs does show those builders be provided as factories rather than singleton but it s definitely not very clear I will work on updating the docs to clarify that

rocketraman · Answer

Yep I made the change to `factory` as soon as I figured out what was happening but it was a bit surprising in the sense that configs are not normally tied to lifecycles

Casey Brooks · Answer

The config is basically an ephemeral class that is used to populate the VM essentially just a wrapper class to avoid a huge constructor It doesn t really live beyond the creation of the VM but it does contain values that are directly related to the lifecycle of the VM and may hold state related to the VM instance InputStragegy interceptors etc You should always treat it as a factory to ensure that those values are recreated when the VM itself is recreated But I can definitely see the confusion here The docs really only show the usage of the builder being applied directly to the VM s constructor and doesn t really explain much about the config class itself

rocketraman · Answer

If that is so the API really should be changed to just accept config parameters in the constructor That way there is no way for the caller to mess things up by having the lifecycle of the config be different from the lifecycle of the VM

rocketraman · Answer

Or alternatively the VM implementation should think of configs as templates from which it can make copies of things it needs with its own lifecycle this would be my preference

Casey Brooks · Answer

That s basically how the config builder has been treated so it seem like that s basically just duplication of the construction logic not to mention a ton of work to update all the existing classes interceptors to themselves be factories rather than concrete instances But I do see now how that would help ensure it cannot be mis configured But I also don t feel like this is necessarily an unusual pattern the way it is Take Ktor HTTP clients for example It also uses a pattern of a builder DSL which creates a Config class that s tied directly to the HTTP client Granted HTTP clients themselves are usually singletons and maybe that s the distinction and confusion here

rocketraman · Answer

> Granted HTTP clients themselves are usually singletons and maybe that s the distinction and confusion here Yep that s right

rocketraman · Answer

Also the failure mode was a bit nasty It took me quite a few hours to debug why it wasn t working

rocketraman · Answer

I can t think of an example of a popular API in which an object which is intended to be created each time it is used uses a configuration DSL which is also tied to the same lifecycle Config is generally config not state

rocketraman · Answer

The other option is to take the builder itself as the parameter to the constructor

Casey Brooks · Answer

I don t think that would fix the problem though since the builder doesn t create new instances of the classes which hold state I think it would need to be updated so that Interceptors and the other stateful classes themselves are factories similar to how they re declared in Ktor I don t have a ton of time right now through the end of the year for such a major change but if this is something you d like to poke around at I m definitely open to that contribution

rocketraman · Answer

You re right just tried it and using the builder as a builder on each construction of the VM doesn t work either That s even worse from an API affordance perspective

Casey Brooks · Answer

Yeah again I wasn t considering the use case of creating the config separately from the VM instance I was mostly thinking of it as little more than a big mutable constructor The only real difference between the builder and the actual config class is `var` vs `val` properties

rocketraman · Answer

I ll create an issue so at least future people that run into this and google for it might find the solution quickly and perhaps over time someone can adjust the API or implementation

rocketraman · Answer

And here it is <https github com copper leaf ballast issues 62>

rocketraman · Answer

Maybe another option for an API change that would not introduce drastic changes but would make the API affordances clearer would be take a lambda parameter `configCreator gt BallastViewModelConfiguration lt Inputs Events State gt ` in the VM constructor The intended usage is then obvious to the caller and for current API users this would be an easy change to make It can be introduced into 4 0 by deprecating the old constructor and then removing the old method in 5 x