spring-amqp icon indicating copy to clipboard operation
spring-amqp copied to clipboard

Quiz - what happens on convertSendAndReceiveAsType timeout? a) null is returned b) AmqpException is thrown c) TimeoutException is thrown

Open bastiat opened this issue 3 years ago • 3 comments

Enhancement

Regarding javadoc of eg convertSendAndReceiveAsType(...)

Returns: the response if there is one. [I suggest adding: null on timeout] Throws: AmqpException - if there is a problem.

As for me documentation doesn't state it clearly that on timeout null is returned. It's inconsistent with Future.get(long timeout, TimeUnit unit)

		RabbitTemplate: 
		@Nullable
		public Message get(long timeout, TimeUnit unit) throws InterruptedException {
			try {
				return this.future.get(timeout, unit);
			}
			catch (ExecutionException e) {
				throw RabbitExceptionTranslator.convertRabbitAccessException(e.getCause()); // NOSONAR lost stack trace
			}
			catch (TimeoutException e) {
				return null;
			}
		}

What was the design decision behind it? Why is returning null considered better than throwing TimeoutException?

bastiat avatar Dec 17 '21 14:12 bastiat

Throwing TimeoutException means exposing an internal API which is not a goal of this "send-n-receive" protocol. No one pursued consistency with Future.get() over here.

That's just a decision to make it in messaging style: if there is no data in time, then there is no data. Why should one throw a timeout exception while it has nothing to do with data?

See these methods for more info:

	/**
	 * Specify the timeout in milliseconds to be used when waiting for a reply Message when using one of the
	 * sendAndReceive methods. The default value is defined as {@link #DEFAULT_REPLY_TIMEOUT}. A negative value
	 * indicates an indefinite timeout. Not used in the plain receive methods because there is no blocking receive
	 * operation defined in the protocol.
	 *
	 * @param replyTimeout the reply timeout in milliseconds
	 *
	 * @see #sendAndReceive(String, String, Message)
	 * @see #convertSendAndReceive(String, String, Object)
	 */
	public void setReplyTimeout(long replyTimeout) {

	/**
	 * Subclasses can implement this to be notified that a reply has timed out.
	 * @param correlationId the correlationId
	 * @since 2.1.2
	 */
	protected void replyTimedOut(String correlationId) {

artembilan avatar Dec 17 '21 15:12 artembilan

Thx for explaining. I would suggest putting it straight in javadoc. If there is one might be enigmatic if it were to mean returning null on timeout.

Returns: the response if there is one , null on timeout.

bastiat avatar Dec 17 '21 16:12 bastiat

I would suggest putting it straight in javadoc.

Sure! Feel free to contribute such a fix: https://github.com/spring-projects/spring-amqp/blob/main/CONTRIBUTING.adoc !

artembilan avatar Dec 17 '21 16:12 artembilan