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

Exception when disabling heartbeat with v7 #1756

Open
Socolin opened this issue Jan 3, 2025 · 4 comments · May be fixed by #1773
Open

Exception when disabling heartbeat with v7 #1756

Socolin opened this issue Jan 3, 2025 · 4 comments · May be fixed by #1773
Assignees
Milestone

Comments

@Socolin
Copy link

Socolin commented Jan 3, 2025

Describe the bug

Looks like ConnectionFactory.RequestedHeartbeat no longer accept Timespan.Zero as documented

        /// <summary>
        /// Default value for desired heartbeat interval. Default is 60 seconds,
        /// TimeSpan.Zero means "heartbeats are disabled".
        /// </summary>
RabbitMQ.Client.Exceptions.BrokerUnreachableException : None of the specified endpoints were reachable
  ----> System.AggregateException : One or more errors occurred. (Timeout can be only be set to 'System.Threading.Timeout.Infinite' or a value > 0. (Parameter 'value'))
  ----> System.ArgumentOutOfRangeException : Timeout can be only be set to 'System.Threading.Timeout.Infinite' or a value > 0. (Parameter 'value')
   at RabbitMQ.Client.ConnectionFactory.CreateConnectionAsync(IEndpointResolver endpointResolver, String clientProvidedName, CancellationToken cancellationToken)

Crash in ConfigureFrameHandler this line

 fh.ReadTimeout = RequestedHeartbeat;

Reproduction steps

var connectionFactory = new ConnectionFactory
{
	Uri = uri,
	AutomaticRecoveryEnabled = true,
	RequestedHeartbeat = TimeSpan.Zero,
};

_connection = await connectionFactory.CreateConnectionAsync("some-name");

Expected behavior

Should not crash, or crash with a clear error message if this is not valid anymore.

Additional context

No response

@Socolin Socolin added the bug label Jan 3, 2025
@lukebakken lukebakken removed the bug label Jan 3, 2025
@lukebakken
Copy link
Contributor

lukebakken commented Jan 3, 2025

Thanks for taking the time to report the incorrect XML docs. I'll update the XML docs, but this is pretty clear:

System.ArgumentOutOfRangeException : Timeout can be only be set to 'System.Threading.Timeout.Infinite' or a value > 0. (Parameter 'value')

Also, disabling the heartbeat is a bad practice.

@lukebakken lukebakken self-assigned this Jan 3, 2025
@lukebakken lukebakken added this to the 7.1.0 milestone Jan 3, 2025
@Socolin
Copy link
Author

Socolin commented Jan 3, 2025

Well, the exception message is not very clear, since we don't know what timeout is wrong, would be nice to get a message saying this is related to the heartbeat.

It was old test for a library, I know this is bad practice, I just spent 30 minutes trying to understand what was causing this issue.

@lukebakken
Copy link
Contributor

would be nice to get a message saying this is related to the heartbeat

Ah, yes, I agree. Thanks for pointing that out.

@michaelklishin michaelklishin changed the title Crash when disabling heartbeat with v7 Exception when disabling heartbeat with v7 Jan 4, 2025
lukebakken added a commit that referenced this issue Jan 23, 2025
Fixes #1756

* Add test that demonstrates the issue.
@lukebakken lukebakken linked a pull request Jan 23, 2025 that will close this issue
@lukebakken
Copy link
Contributor

#1774

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

Successfully merging a pull request may close this issue.

2 participants