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

Inbox do not handle timeouts correctly #431

Closed
HCanber opened this issue Sep 26, 2014 · 3 comments
Closed

Inbox do not handle timeouts correctly #431

HCanber opened this issue Sep 26, 2014 · 3 comments

Comments

@HCanber
Copy link
Contributor

HCanber commented Sep 26, 2014

For me Inbox_have_maximum_queue_size fails sometimes.

The test ends with this part that fails:

//The inbox should be empty now, so receiving should result in a timeout                
Assert.Throws<TimeoutException>(() =>
{
    var received=_inbox.Receive(TimeSpan.FromSeconds(1));
    Log.Error("Received "+received);
});

Inbox times out, but depending on where and how it does it different things happens.

Receive is implemented like this:

var task=Receiver.Ask(new Get(DateTime.Now + timeout), Timeout.InfiniteTimeSpan);
if (task.Wait(timeout))
{
    return task.Result;
}
else
{
    var fmt = string.Format("Inbox {0} didn't received a response message in specified timeout {1}", 
        Receiver.Path, timeout);
    throw new TimeoutException(fmt);
}

If task.Wait times out the code returns false, and the expected exception is thrown and the test passes.
If the inbox times out internally (when handling the Kick message) the Receiver gets a Status.Failure(new TimeoutException("Deadline passed")) and the test failes.

Which is correct? To send a Failure message to receiver or raise a TimeoutException?

@HCanber HCanber added this to the Version 1 milestone Sep 26, 2014
@annymsMthd
Copy link
Contributor

Changes were merged in 6c77299 to handle this. Please review and ensure this was the correct way to resolve this issue

@HCanber
Copy link
Contributor Author

HCanber commented Apr 19, 2015

Looks ok to me.

@Horusiath
Copy link
Contributor

Closed by #857

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

No branches or pull requests

3 participants