reactiveui / ReactiveUI

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms that is inspired by functional reactive programming. ReactiveUI allows you to abstract mutable state away from your user interfaces, express the idea around a feature in one readable place and improve the testability of your application.
https://www.reactiveui.net
MIT License
8.04k stars 1.12k forks source link

[Bug]: ObservableAsPropertyHelper stops working after command throws exception #3261

Closed msschl closed 2 years ago

msschl commented 2 years ago

Describe the bug 🐞

public class ViewModel : ReactiveObject
{
    private string? errorMessage;

    private readonly ObservableAsPropertyHelper<bool> isExecuting;

    public ViewModel()
    {
        var canExecuteCommand = this.WhenAnyValue(
            x => x.ConditionOne,
            x => x.ConditionTwo,
            (conditionOne, conditionTwo) =>
            {
                return conditionOne >= 0 && conditionTwo is false;
            });
        this.Command = ReactiveCommand.CreateFromTask(token => Task.Run(() => AsyncWork(token)), canExecuteCommand);
        this.Command
            .IsExecuting
            .ToProperty(this, x => x.IsExecuting, out this.isExecuting);
        this.ConnectOrDisconnectCommand
            .ThrownExceptions
            .Subscribe(ex =>
            {
                this.logger.LogError(ex, "Exception message...");
                this.ErrorMessage = ex.Message;
            });
    }

    public string? ErrorMessage
    {
        get => this.errorMessage;
        private set => this.RaiseAndSetIfChanged(ref this.errorMessage, value);
    }

    public bool IsExecuting => this.isExecuting?.Value ?? false;

    public ReactiveCommand<Unit, Unit> Command { get; }
}

When the command throws an exception the next time the command starts running the ObservableAsPropertyHelper won't notify whether the command is running or not.

Expected behavior

I would expect the ObservableAsPropertyHelper to still work as expected and notify the UI when the command is running even though the previous execution threw an exception.

IDE

Visual Studio 2022

Operating system

Windows

Version

No response

Device

Windows PC

ReactiveUI Version

18.0.10

Additional information ℹ️

No response

msschl commented 2 years ago

As a side note and not related to this bug:

When specifying the outputScheduler prameter as RxApp.TaskpoolScheduler on the CreateFromTask method, I would expect the task to be scheduled on the passed in scheduler i.e.:

ReactiveCommand.CreateFromTask(token => AsyncWork(token), canExecuteCommand, outputScheduler: RxApp.TaskpoolScheduler);

However, this is not the case. This seems pretty counterintuitive. I had to search this on GH and found out that you need to add the extra ceremony of Task.Run(() => AsyncWork(token)) to schedule the task on the thread pool.

See #813

glennawatson commented 2 years ago

Re your side note The parameter name is outputScheduler, it's the thread that the contents is scheduled on since reactive commands are also observables. It's also how the documentation is worded.

Will read more about your actual bug won't have a reply immediately under the pump a bit with real life stuff at the moment.

msschl commented 2 years ago

Re your side note The parameter name is outputScheduler, it's the thread that the contents is scheduled on since reactive commands are also observables. It's also how the documentation is worded.

Ah, now I got it. Thanks, @glennawatson for the clarification. However, I still think that many others will fall into the same pitfall.

tomasfil commented 2 years ago

I think the issue is with the ToProperty might run on wrong thread. Try putting .ObserveOn(RxApp.MainThreadScheduler) before . ToProperty and see if it works.

And are you able to build reproduction repository?

msschl commented 2 years ago

I think the issue is with the ToProperty might run on wrong thread. Try putting .ObserveOn(RxApp.MainThreadScheduler) before . ToProperty and see if it works.

I've already done this. Still the same issue. I am running on ReactiveUI.WPF version 18.0.10 and ReactiveUI.Validation version 2.2.1

Will try to create a repro repository.

msschl commented 2 years ago

@tomasfil Here is a repro repository

glennawatson commented 2 years ago

This one is likely associated with https://github.com/dotnet/reactive/issues/1256

msschl commented 2 years ago

When would this be fixed? It is quite annoying having a task be canceled with a task cancelation token and then the UI stops working correctly.

tomasfil commented 2 years ago

Yeaah, I think this is related to https://github.com/reactiveui/ReactiveUI/pull/3246

Since then there was no release. I have noticed in commits that @glennawatson is preparing some kind of release, so once new update is out, then I believe this should work.

glennawatson commented 2 years ago

Need to fix the build again. I'll see if I can get a new release our this weekend

msschl commented 2 years ago

Fixed by #3246

ObservableAsPropertyHelper is now working as expected.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.