-
Notifications
You must be signed in to change notification settings - Fork 29
Eventbus send with timeout #77
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
Conversation
I've just added another commit to introduce the Message.fail() method that the Java API now provides, too |
@@ -30,71 +61,160 @@ class EventBus(protected[this] val internal: JEventBus) extends VertxWrapper { | |||
|
|||
sealed private trait SendOrPublish | |||
private case class Publish(address: String, value: MessageData) extends SendOrPublish | |||
private case class Send[X](address: String, value: MessageData, replyHandler: Option[Handler[JMessage[X]]]) extends SendOrPublish | |||
private case class Send[X](address: String, value: MessageData, | |||
replyHandler: Option[Either[Handler[JMessage[X]], Handler[AsyncResult[JMessage[X]]]]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These types are getting a bit too verbose. What about we use type aliases to make them more readable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really care as these are only used in this class and cannot be accessed from outside.
Minor things, the rest looks fine. |
@galderz if you're okay with this now, then please merge. I'll add the PR for the unregistering handler right away then. |
vertx.eventBus.sendWithTimeout("hello", "late-reply-within-timeout", 100, { | ||
case Success(result) => fail("Should not receive a message within time, but got one: " + result.body) | ||
case Failure(ex) => | ||
assertThat("exception should be of kind ReplyException", ex, instanceOf(classOf[ReplyException])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can leave the String message out (up to you...) e.g.
assertThat(new Exception, instanceOf(classOf[ArrayIndexOutOfBoundsException]))
Throws:
Exception in thread "main" java.lang.AssertionError:
Expected: an instance of java.lang.ArrayIndexOutOfBoundsException
got: <java.lang.Exception>
Something for 2.1M2-SNAPSHOT since there was a bugfix for send / reply with timeouts. Please review and merge if everything okay.