Elmish.wpf: Binding.subModelWin stops Cmd.OfFunc.perform from working

[ChrisPritchard]

Hello! This is a bit of an odd one, which might be due to some misunderstanding of how Elmish works, but here goes:

I have an app with three windows: a main window that is the entry point, and two sub windows that get opened or closed based on events. These are both controlled via code like:

"TableOfContents" |> Binding.subModelWin (
        (fun m -> m.tableOfContents |> WindowState.ofOption), 
        snd, 
        TableOfContentsMessage,
        TableOfContents.bindings,
        (Codex.Views.TableOfContents))

I also have save file functionality, triggered like so:

| SaveNovel -> 
        model, Cmd.OfFunc.perform saveFile "" id

With the saveFile function adapted from another issue answer here:

let saveFile fileName =
    Application.Current.Dispatcher.Invoke(fun () ->
        let dialog = SaveFileDialog (Filter = fileFilter, FileName = fileName)
        let result = dialog.ShowDialog ()
        if result.HasValue && result.Value
            then SaveFileSelected dialog.FileName
            else FileSelectCanceled
    )

Now my problem: the save functionality works fine when no other windows are open, but the moment I show a sub window, the result of Cmd.OfFunc.perform saveFile "" id is ignored. If I close the subwindow, the save functionality starts working again.

Cmd.OfFunc.perform creates a [bind] list, with bind being a func to run. It seems this list is just ignored if a subwindow is open, as if I just returned Cmd.none.

Any ideas? The code is on github but private. I can open it up if that would help.

[cmeeren]

You have to open the dialog asynchronously in order to not block the Elmish dispatch loop. Check out the readme FAQ entry (toward the bottom), the new FileDialogs sample, an/or the later comments in #147. :)

[ChrisPritchard]

@cmeeren while useful, that unfortunately does not solve the problem. When a subModelWin is open, the cmd list is still not getting processed, whether it was constructed via Cmd.ofFunc.perform or Cmd.OfAsyncImmediate.perform

[ChrisPritchard]

The project is here, if you want to see the behaviour: https://github.com/ChrisPritchard/Codex. Just clone then run (its netcore 3.0), and note the 'Save Current Novel' works when the TOC is closed, but does nothing when open.

[ChrisPritchard]

I'm sure its something dumb - though on my end or in the framework I'm not sure :) The project has two sub windows (TOC and Scene Editor), and the weird behaviour only occurs if one (the TOC) is open. I wonder if its something to do with bindings getting...odd with the one that breaks it, as that has a much large model with lots of binding.

[cmeeren]

Thanks! Since the behaviour here seems to be complex (in the sense that it depends on more than one thing), would you be able to write a minimal repro? I'm very busy the next few days, but having a minimal "press here to repro" solution with nothing else going on would make it more likely that I can take a look at it.

[ChrisPritchard]

The project I have linked is in its earlier stages, so hopefully its minimalistic enough to very easily repro the behaviour. I will keep looking too - Ill add Elmish.WPF directly to the solution to run through and if I find out the problem I'll report back.

[cmeeren]

Thanks a lot!

[ChrisPritchard]

I found the issue @cmeeren once I copied and referenced Elmish.WPF directly, and its not related to dialogs or subModelWin. The window that causes the problem has a subModelSeq in it, which uses id for its 'get id' function. Unfortunately, the model has several record elements that are structurally identical, and this causes this line in ViewModel.fs to crash: https://github.com/elmish/Elmish.WPF/blob/master/src/Elmish.WPF/ViewModel.fs#L518.

The crash appears to silently end the processing of the cmd, resulting in no expected behaviour presenting.

So...mistake on my part in using id as the getid for elements that are identical, however this might represent an ideological issue with F# vs C# - structural vs reference comparison. In my case its just that my test data is dumb, however, I wonder if in general use people might run into this more often with more simple lists?

Anyway, interesting issue. I'm happy to close this.

[cmeeren]

The docs on getId currently say this:

Gets a unique identifier for a sub-model.

To me, "unique" makes it clear that the ID must be distinct among its siblings, though it may just be that I'm steeped in F# and Elmish.WPF and take this for granted. If you have suggestions for improving the docs, suggestions/PRs are welcome!

The crash appears to silently end the processing of the cmd, resulting in no expected behaviour presenting.

Are you getting any exceptions logged if you turn on logging in the Elmish.WPF config?

[ChrisPritchard]

No unfortunately. I'm running it like this:

[<EntryPoint; STAThread>]
let main _ =
  Program.mkProgramWpf init MainWindow.update MainWindow.bindings
  |> Program.withConsoleTrace
  |> Program.withDebugTrace
  |> Program.runWindowWithConfig { ElmConfig.Default with LogConsole = true } (Codex.Views.MainWindow ())

But get nothing, just the printing of the updated model following the processing of the update call. No stack trace - possibly because setState is running on a background thread maybe?

Comment about the docs on getId are valid - and once I saw the exception I immediately figured out where in my code (my use of id) that the issue was occurring. However, understandably, the fact its compiled and run semi okay with this glaring fault (because the exceptions were dying silently) is not ideal :)

[cmeeren]

the fact its compiled and run semi okay with this glaring fault (because the exceptions were dying silently) is not ideal :)

I agree, though I don't see how this can be prevented in the general sense. It would be like List.sortBy (fun x -> x.Id) vs. List.sortBy id, which is generally possible no matter how strongly typed your language/architecture/API.

Any suggestions are welcome, though 😊

possibly because setState is running on a background thread maybe?

It's running on the UI thread.

[ChrisPritchard]

I couldn't find a try catch discarding the exception anywhere - weird that it just got swallowed.

Suggestion wise, hmm. I thought maybe the getId function which has the sig fun 'model -> 'id could be changed to fun index 'model -> 'id to make it easy for simplistic models to just use the index for a unique identifier.

That might make it simpler to use, and head off developers making this mistake by accident? My thought process that led to the issue was copying the examples, which use id - the signature of subModelSeq and subModelWin is a bit complicated (largely due to overloads, in my subjective opinion) so I relied heavily on the samples for working code.

Of course to avoid breaking changes, adding such a getId signature would involve _further_ overloads...so... I don't know. Its fine the way it is :D

[cmeeren]

Reopening this because I'd like to know why the exception is just swallowed.

[ChrisPritchard]

Okdoke. Stack trace:

   at System.ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException[T](T key)
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at Elmish.WPF.ViewModel`2.updateValue(String bindingName, model newModel, VmBinding`2 binding) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 513
   at <StartupCode$Elmish-WPF>[email protected](KeyValuePair`2 _arg3) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 581
   at Microsoft.FSharp.Primitives.Basics.List.filter[T](FSharpFunc`2 predicate, FSharpList`1 l) in E:\A\_work\130\s\src\fsharp\FSharp.Core\local.fs:line 495
   at Microsoft.FSharp.Collections.ListModule.Filter[T](FSharpFunc`2 predicate, FSharpList`1 list) in E:\A\_work\130\s\src\fsharp\FSharp.Core\list.fs:line 417
   at Elmish.WPF.ViewModel`2.UpdateModel(model newModel) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 578
   at Elmish.WPF.ViewModel`2.updateValue(String bindingName, model newModel, VmBinding`2 binding) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 517
   at <StartupCode$Elmish-WPF>[email protected](KeyValuePair`2 _arg3) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 581
   at Microsoft.FSharp.Primitives.Basics.List.filter[T](FSharpFunc`2 predicate, FSharpList`1 l) in E:\A\_work\130\s\src\fsharp\FSharp.Core\local.fs:line 495
   at Microsoft.FSharp.Collections.ListModule.Filter[T](FSharpFunc`2 predicate, FSharpList`1 list) in E:\A\_work\130\s\src\fsharp\FSharp.Core\list.fs:line 417
   at Elmish.WPF.ViewModel`2.UpdateModel(model newModel) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 578
   at Elmish.WPF.ViewModel`2.updateValue(String bindingName, model newModel, VmBinding`2 binding) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 517
   at <StartupCode$Elmish-WPF>[email protected](KeyValuePair`2 _arg3) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 581
   at Microsoft.FSharp.Primitives.Basics.List.filter[T](FSharpFunc`2 predicate, FSharpList`1 l) in E:\A\_work\130\s\src\fsharp\FSharp.Core\local.fs:line 493
   at Microsoft.FSharp.Collections.ListModule.Filter[T](FSharpFunc`2 predicate, FSharpList`1 list) in E:\A\_work\130\s\src\fsharp\FSharp.Core\list.fs:line 417
   at Elmish.WPF.ViewModel`2.UpdateModel(model newModel) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 578
   at Elmish.WPF.ViewModel`2.updateValue(String bindingName, model newModel, VmBinding`2 binding) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 507
   at <StartupCode$Elmish-WPF>[email protected](KeyValuePair`2 _arg3) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 581
   at Microsoft.FSharp.Primitives.Basics.List.filter[T](FSharpFunc`2 predicate, FSharpList`1 l) in E:\A\_work\130\s\src\fsharp\FSharp.Core\local.fs:line 493
   at Microsoft.FSharp.Collections.ListModule.Filter[T](FSharpFunc`2 predicate, FSharpList`1 list) in E:\A\_work\130\s\src\fsharp\FSharp.Core\list.fs:line 417
   at Elmish.WPF.ViewModel`2.UpdateModel(model newModel) in C:\dev\Codex\Elmish.WPF\ViewModel.fs:line 578
   at [email protected](model model, FSharpFunc`2 dispatch) in C:\dev\Codex\Elmish.WPF\Program.fs:line 25
   at [email protected](msg msg)

[ChrisPritchard]

Somewhere in there is the "meh, nothing to see here, carry on"

[cmeeren]

Thanks a lot! I'll investigate when I am able (anyone is welcome to have a look at this before then.)

[TysonMN]

The exception should be displayed in WPF's logging that is sent to System.Diagnostics.Debug. We can also log this issue, include more helpful information, and improve the robustness by returning false from updateValue so that other bindings are still updated.

I can add a PR for this after #151 is completed.

P.S.

this causes this line in ViewModel.fs to crash: https://github.com/elmish/Elmish.WPF/blob/master/src/Elmish.WPF/ViewModel.fs#L518.

If you select "Copy permalink" and paste that link here as plain text (as you did), then it displays like this:
https://github.com/elmish/Elmish.WPF/blob/789cbc304719f34f931a451dafa1b58141af833e/src/Elmish.WPF/ViewModel.fs#L518

[cmeeren]

I can add a PR for this after #151 is completed.

Thanks a lot! I'll leave this to you then.

If you select "Copy permalink"

Yep, and you can also get that by pressing Y to make the browser URL refer to the exact commit you're viewing.

[ChrisPritchard]

I might rename the issue to be more appropriate for the actual problem, if thats okay?

[cmeeren]

Personally I would find the record to be more confusing then, since the issue title would not match the initial description and most of the discussion. I guess the "right" thing to do would be to open another, more focused issue. I personally have no need for it, but I'll leave those decisions to @bender2k14 since he will work on this.

[TysonMN]

Either keeping this issue with its current title or creating a new one with more accurate title is fine with me.

[TysonMN]

Suggestion wise, hmm. I thought maybe the getId function which has the sig fun 'model -> 'id could be changed to fun index 'model -> 'id to make it easy for simplistic models to just use the index for a unique identifier.

The index is not identifying because of the possibility of reordering. For an example, see the SubModelSeq sample.

[TysonMN]

The exception should be displayed in WPF's logging that is sent to System.Diagnostics.Debug.

Actually, it depends on where the dispatch came from. If the dispatch came from user interaction (like in the OneWaySeq sample), then it will be logged by WPF to System.Diagnostics.Debug. The dispatch could also come from a subscription, a command from init, and a command from update. Furthermore, the command could be created with Cmd.ofSub or some other way. Without testing to confirm, I think those cases would either will lead to Elmish logging an error, crashing the program, or a new message being dispatched.

[TysonMN]

The exception should be displayed in WPF's logging that is sent to System.Diagnostics.Debug.

Actually, it depends on where the dispatch came from. If the dispatch came from user interaction (like in the OneWaySeq sample), then it will be logged by WPF to System.Diagnostics.Debug. The dispatch could also come from a subscription, a command from init, and a command from update. Furthermore, the command could be created with Cmd.ofSub or some other way. Without testing to confirm, I think those cases would either will lead to Elmish logging an error, crashing the program, or a new message being dispatched.

_ACTUALLY_, this is why testing is a good thing.

In all cases, the exception is caught by the Elmish dispatch loop and sent to System.Diagnostics.Debug by default.

[TysonMN]

Reopening this because I'd like to know why the exception is just swallowed.

So the simple answer is that Elmish caught the exception and logged it to its onError function, which defaults to System.Diagnostics.Trace.