refactor(sdk): introduce NonZeroIggyDuration for retries and heartbeats

[ethanlin01x]

Which issue does this PR address?

Relates to #3776 (Found while reviewing)

Rationale

Zero-valued durations reach loops that spin or panic. A zero retry interval panics tokio::time::interval, and a zero heartbeat interval turns the heartbeat task into a continuous ping loop. IggyDuration::from_str maps 0, none, disabled and unlimited to the same zero, so a user asking to turn something off arrives there.

A duration that paces a loop is a different thing from a duration that measures a delay, so it gets its own type. NonZeroIggyDuration rejects the zero where the value is built, which means no call site has to remember the rule and every binding that funnels through the Rust SDK gets the guarantee.

What changed?

New type NonZeroIggyDuration (core/common/src/utils/duration.rs): TryFrom<IggyDuration>, TryFrom<u64>, TryFrom<Duration>, FromStr, Display and serde, all rejecting zero and the three aliases that parse as zero. Serialization emits whole microseconds, so a duration shorter than that is rejected as well. The error is a typed NonZeroDurationError.

Fields that now hold it:

• heartbeat_interval on the TCP, QUIC, WebSocket and HTTP client configs and connection string options, plus BinaryTransport::get_heartbeat_interval and SystemClient::heartbeat_interval. The field is new on HttpClientConfig, where the connection string option was parsed and thrown away.
• reconnection.interval on all three transports
• IggyConsumer init_retry_interval and polling_retry_interval
• ProducerCore send_retries_interval
• AutoCommit::Interval, AutoCommit::IntervalOrWhen and AutoCommit::IntervalOrAfter

Zero stays legal where it means something, and this is now pinned by tests: reestablish_after (zero reconnects immediately), poll_interval (zero means no throttling) and the background producer linger_time.

The Python and PHP bindings follow the same types.

Breaking changes

• The config fields above change type. Construction becomes NonZeroIggyDuration::from_str("5s")? instead of IggyDuration::from_str("5s")?.
• A connection string with heartbeat_interval=none|disabled|unlimited|0 or reconnection_interval=0 now fails with InvalidConnectionString instead of silently spinning. This includes the HTTP connection string, where the option was parsed and discarded.
• A zero AutoCommit interval is now rejected. The three interval variants take NonZeroIggyDuration, so Rust rules it out at compile time and the Python and PHP bindings raise at the call. Zero stored the offsets in a busy loop; committing on every poll is spelled AutoCommit::When or AutoCommit::After.
• Python: TcpReconnectionConfig(interval=timedelta(0)) is now rejected whatever the retry policy is.

Local Execution

• Passed
• Pre-commit hooks ran

AI Usage

1. Which tools? Claude
2. Scope of usage? help implement and review this PR
3. How did you verify the generated code works correctly? Unit tests pin each rejected value and each still-legal zero
4. Can you explain every line of the code if asked? Yes, all the changes are checked by the human.

[codecov]

Codecov Report

❌ Patch coverage is 86.54292% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.01%. Comparing base (4c475df) to head (67802b9).

Files with missing lines	Patch %	Lines
core/common/src/utils/duration.rs	90.90%	16 Missing and 1 partial ⚠️
core/sdk/src/client_provider.rs	26.31%	8 Missing and 6 partials ⚠️
core/sdk/src/clients/client_builder.rs	33.33%	8 Missing ⚠️
core/sdk/src/tcp/tcp_client.rs	83.33%	2 Missing and 2 partials ⚠️
...guration/quic_config/quic_client_config_builder.rs	0.00%	2 Missing ⚠️
...ebsocket_config/websocket_client_config_builder.rs	0.00%	2 Missing ⚠️
core/sdk/src/quic/quic_client.rs	88.23%	0 Missing and 2 partials ⚠️
core/sdk/src/websocket/websocket_client.rs	85.71%	0 Missing and 2 partials ⚠️
...tion/http_config/http_connection_string_options.rs	92.85%	0 Missing and 1 partial ⚠️
...ion/quic_config/quic_client_reconnection_config.rs	50.00%	1 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3891      +/-   ##
============================================
- Coverage     84.03%   84.01%   -0.02%     
  Complexity     1358     1358              
============================================
  Files          1215     1215              
  Lines        170548   170820     +272     
  Branches     138292   138577     +285     
============================================
+ Hits         143315   143512     +197     
- Misses        23381    23422      +41     
- Partials       3852     3886      +34     

Components	Coverage Δ
Rust Core	84.90% <86.02%> (-0.04%)	⬇️
Java SDK	66.67% <ø> (ø)
C# SDK	74.96% <ø> (ø)
Python SDK	90.10% <100.00%> (-0.03%)	⬇️
PHP SDK	85.65% <91.66%> (+1.16%)	⬆️
Node SDK	95.81% <ø> (ø)
Go SDK	68.29% <ø> (ø)

Files with missing lines	Coverage Δ
core/common/src/traits/binary_impls/system.rs	100.00% <100.00%> (ø)
core/common/src/traits/system_client.rs	0.00% <ø> (ø)
...pes/configuration/auth_config/connection_string.rs	95.95% <100.00%> (ø)
...es/configuration/http_config/http_client_config.rs	100.00% <100.00%> (ø)
...guration/http_config/http_client_config_builder.rs	86.53% <100.00%> (+4.03%)	⬆️
...es/configuration/quic_config/quic_client_config.rs	100.00% <100.00%> (ø)
...tion/quic_config/quic_connection_string_options.rs	70.16% <100.00%> (+1.10%)	⬆️
...ypes/configuration/tcp_config/tcp_client_config.rs	100.00% <100.00%> (ø)
...figuration/tcp_config/tcp_client_config_builder.rs	68.04% <100.00%> (ø)
...ation/tcp_config/tcp_client_reconnection_config.rs	100.00% <100.00%> (ø)
... and 29 more

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ethanlin01x]

/request-review @hubcio

[ethanlin01x]

/ready

[haubur]

The problem is real for consumers:

• panic on a 0 IggyDuration:

  iggy/core/sdk/src/clients/consumer.rs

  Line 328 in ec456d2

  let mut timer = time::interval(interval.get_duration());

• and (not) sleeping with 0 on the background store offset at interval task loop. :

  iggy/core/sdk/src/clients/consumer.rs

  Line 496 in ec456d2

  _ = sleep(interval.get_duration()) => {}

producers:

• panic on a 0 IggyDuration:

  iggy/core/sdk/src/clients/producer.rs

  Line 265 in ec456d2

  timer.get_or_insert_with(|| tokio::time::interval(interval.get_duration()));

and connection lifecycle:

• continuous heartbeat spins:

  iggy/core/sdk/src/clients/client.rs

  Line 298 in f42f5ab

  sleep(heartbeat_interval.get_duration()).await

Clarification:

• The problem occurs if manually setting a IggyDuration equal to zero.
• No default path actually sets an IggyDuration with zero.
• panics occur for IggyDuration in retry_intervals not for heartbeat_intervals

Solution:

• Having IggyDuration falling to 0 for "unlimited", "disabled" and "none" is a little misleading.
• Since it's a common type there might be unobserved behavior in other places should we change it now.
• To avoid panics, I would prefer not checking in methods calls as done in the PR but enforcing the proposed guards on a type level e.g. by introducing a NonZeroIggyDuration.

[ethanlin01x]

@haubur

Thanks for the review.

I agree that using NonZeroIggyDuration is cleaner than checking in each method. My concern is that some of these are cross-field rules, so I am not sure a type can enforce them. For example, interval = 0 is still valid when max_retries is bounded. Changing the field type would also be a breaking change for the SDK API.

So I would prefer to keep the current approach. What do you think?

[haubur]

@ethanlin01x fair points.

• A NonZeroIggyDuration assumes that 0 intervals are actually undesirable, which I would argue makes sense. I think its reasonable to enforce waiting a bit before retrying (instead of rapid fire). You can still set the duration to some very low time period if you want that, but that's intended behavior than not implicit as it is now.
• It would be a breaking API change, true. If thats something we want at this point is a question to the maintainers I guess, because there is a release coming up this week AFAIK.

[spetz]

@haubur breaking changes are totally fine at this stage, as we're very close to the next release, with lots of breaking changes anyway :)
And yes, I do agree, there should be at least like a 1 ms wait before the retries (while on the other hand, no wait at all for data producer/consumer running as a busy loop is totally fine for specific use cases).

[haubur]

Thanks @spetz! So @ethanlin01x lets have a NonZeroIggyDuration for retries and heartbeats, while keeping the busy loop for AutoCommit on an interval = 0. Would you want to rewrite? Otherwise, I can also take this.

[ethanlin01x]

@haubur Thanks, I just rewrote the whole PR to introduce NonZeroIggyDuration

[haubur]

Thanks! I add some minor comments, basically just location and making sure that the IggyDuration and NonZeroIggyDuration API will align.

[ethanlin01x]

@haubur thanks, all four comments are addressed in 9f07b53 and 71f3484.

[ethanlin01x]

/request-review @spetz

[spetz]

Looks solid overall, just a few minor improvements left :)

[ethanlin01x]

/ready

[ethanlin01x]

/ready