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

Inconsistent Timeout and TimeoutFrame behavior #298

Open
TORISOUP opened this issue Feb 1, 2025 · 2 comments
Open

Inconsistent Timeout and TimeoutFrame behavior #298

TORISOUP opened this issue Feb 1, 2025 · 2 comments

Comments

@TORISOUP
Copy link
Contributor

TORISOUP commented Feb 1, 2025

The behavior of Timeout and TimeoutFrame immediately after calling Subscribe() appears to be inconsistent.
This issue is raised for confirmation in case this behavior is unintended.

TimeoutFrame starts its timer immediately upon subscription. As a result, it times out even if OnNext is never issued.
However, Timeout does not start its timer until OnNext is issued at least once. This means that if OnNext is never issued, the timer will wait indefinitely.

For comparison, UniRx's Timeout starts the timer immediately after subscription.

// R3.Timeout, using NUnit

using var subject = new R3.Subject<int>();

var results =
    subject.Timeout(TimeSpan.FromMilliseconds(100), TimeProvider.System)
        .Materialize()
        .ToLiveList();

// not timeout
await Task.Delay(TimeSpan.FromMilliseconds(300));

// No messages received (subscription is still active)
CollectionAssert.IsEmpty(results);
// R3.TimeoutFrame,  using NUnit

var fakeFrameProvider = new FakeFrameProvider();
using var subject = new R3.Subject<int>();

var results =
    subject.TimeoutFrame(3, fakeFrameProvider)
        .Materialize()
        .ToLiveList();

// Timeout
fakeFrameProvider.Advance(5);

Assert.AreEqual(R3.NotificationKind.OnCompleted, results[0].Kind);
Assert.IsInstanceOf<TimeoutException>(results[0].Error);
// UniRx.Timeout, using NUnit

using var subject = new UniRx.Subject<int>();

var results = new List<UniRx.Notification<int>>();

subject.Timeout(TimeSpan.FromMilliseconds(100))
    .Materialize()
    .Subscribe(results.Add);

// Timeout
await Task.Delay(TimeSpan.FromMilliseconds(200));

Assert.AreEqual(UniRx.NotificationKind.OnError, results[0].Kind);
Assert.IsInstanceOf<TimeoutException>(results[0].Exception);
@neuecc
Copy link
Member

neuecc commented Feb 3, 2025

Thank you.
In R3, we were thinking of standardizing the timer initialization to occur at the first OnNext across the board.
For example, this applies to Chunk(TimeSpan) and ChunkFrame.
Therefore, I believe the current behavior of TimeoutFrame is a bug, and I'd like to fix it.

@TORISOUP
Copy link
Contributor Author

TORISOUP commented Feb 3, 2025

I understand that the behavior on the Timeout side is correct and the TimeoutFrame side is currently wrong.
I will keep that in mind for now and use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants