Skip to content

Better guidance when WhenSagaTimesOut is used without previous RequestTimeout#87

Merged
timbussmann merged 1 commit intomasterfrom
hotfix-5.2.1
Jul 25, 2016
Merged

Better guidance when WhenSagaTimesOut is used without previous RequestTimeout#87
timbussmann merged 1 commit intomasterfrom
hotfix-5.2.1

Conversation

@danielmarbach
Copy link
Contributor

Who's affected

  • Users trying to use WhenSagaTimesOut without previous call to When or WhenHandle

Symptoms

Test.Saga<TimeoutSaga>()
                    .ExpectTimeoutToBeSetIn<MyTimeout>((state, expiresIn) => expiresIn == TimeSpan.FromDays(1))
                    .WhenSagaTimesOut()

will throw a KeyNotFoundException coming from the TimeoutManager. The problem is that this exception doesn't guide the user in any way to do the right thing.

@danielmarbach
Copy link
Contributor Author

@Particular/nservicebus-maintainers is it safe to assume we will never enqueue a null timeout state?

@timbussmann
Copy link
Contributor

is this worth a hotfix? What about v6?

@danielmarbach
Copy link
Contributor Author

I would release it as a patch. Not a hotfix.

@danielmarbach
Copy link
Contributor Author

Happy to rebase this to current release branch. I added it as a patch because a customer was falling over that exception and didn't know how to resolve it. The consequence was, I received an email. So fixing this would potentially prevent similar interactions in the future because it is much clearer how the API should be called.

@timbussmann
Copy link
Contributor

Happy to rebase this to current release branch

👍 to that. The approach in core was to fix non-criticals on the release branch. I think we should follow that process in testing.

@danielmarbach
Copy link
Contributor Author

Pre-release doesn't have this problem. It throws an ExpectedInvocation exception.

@danielmarbach
Copy link
Contributor Author

Let's release this small patch next week?

@timbussmann timbussmann merged commit d7bd621 into master Jul 25, 2016
@timbussmann timbussmann deleted the hotfix-5.2.1 branch July 25, 2016 13:40
@timbussmann
Copy link
Contributor

@danielmarbach should we copy the test over to develop?

@danielmarbach
Copy link
Contributor Author

If you think there is value in it why not.

@timbussmann timbussmann added the Bug label Aug 5, 2016
@timbussmann timbussmann added this to the 5.2.1 milestone Aug 5, 2016
@timbussmann
Copy link
Contributor

5.2.1 is now released

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants