Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
msschl opened this issue May 18, 2022 · 12 comments
Closed
Labels

Comments

@msschl
Copy link

msschl commented May 18, 2022

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 msschl added the bug label May 18, 2022
@msschl
Copy link
Author

msschl commented May 18, 2022

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
Copy link
Contributor

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
Copy link
Author

msschl commented May 18, 2022

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
Copy link
Contributor

tomasfil commented Jun 1, 2022

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
Copy link
Author

msschl commented Jun 2, 2022

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
Copy link
Author

msschl commented Jun 2, 2022

@tomasfil Here is a repro repository

@glennawatson
Copy link
Contributor

This one is likely associated with dotnet/reactive#1256

@msschl
Copy link
Author

msschl commented Jun 8, 2022

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
Copy link
Contributor

tomasfil commented Jun 8, 2022

Yeaah, I think this is related to
#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
Copy link
Contributor

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

@msschl
Copy link
Author

msschl commented Jun 10, 2022

Fixed by #3246

ObservableAsPropertyHelper is now working as expected.

@msschl msschl closed this as completed Jun 10, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants