Skip to content

Commit

Permalink
Fix use of JsOptions timeout (#808)
Browse files Browse the repository at this point in the history
  • Loading branch information
scottf authored Aug 16, 2023
1 parent 020ea14 commit 607e4c9
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 16 deletions.
8 changes: 2 additions & 6 deletions src/NATS.Client/JetStream/JetStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ private PublishAck PublishSyncInternal(string subject, byte[] data, MsgHeader hd
return null;
}

Duration timeout = options == null ? JetStreamOptions.RequestTimeout : options.StreamTimeout;

return ProcessPublishResponse(Conn.Request(msg, timeout.Millis), options);
return ProcessPublishResponse(Conn.Request(msg, Timeout), options);
}

private async Task<PublishAck> PublishAsyncInternal(string subject, byte[] data, MsgHeader hdr, PublishOptions options)
Expand All @@ -102,9 +100,7 @@ private async Task<PublishAck> PublishAsyncInternal(string subject, byte[] data,
return null;
}

Duration timeout = options == null ? JetStreamOptions.RequestTimeout : options.StreamTimeout;

var result = await Conn.RequestAsync(msg, timeout.Millis).ConfigureAwait(false);
var result = await Conn.RequestAsync(msg, Timeout).ConfigureAwait(false);
return ProcessPublishResponse(result, options);
}

Expand Down
2 changes: 1 addition & 1 deletion src/NATS.Client/JetStream/JetStreamBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ protected JetStreamBase(IConnection connection, JetStreamOptions options)
Conn = connection;
JetStreamOptions = options ?? JetStreamOptions.DefaultJsOptions;
Prefix = JetStreamOptions.Prefix;
Timeout = JetStreamOptions.RequestTimeout.Millis;
Timeout = JetStreamOptions.RequestTimeout?.Millis ?? Conn.Opts.Timeout;
}

internal static ServerInfo ServerInfoOrException(IConnection conn)
Expand Down
12 changes: 7 additions & 5 deletions src/NATS.Client/JetStream/JetStreamOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using NATS.Client.Internals;
using static NATS.Client.Internals.JetStreamConstants;
using static NATS.Client.Internals.NatsConstants;
Expand All @@ -20,7 +21,9 @@ namespace NATS.Client.JetStream
{
public sealed class JetStreamOptions
{
[Obsolete("This property is obsolete. The connection options request timeout is used as the default", false)]
public static readonly Duration DefaultTimeout = Duration.OfMillis(Defaults.Timeout);

public static readonly JetStreamOptions DefaultJsOptions = Builder().Build();

private JetStreamOptions(JetStreamOptionsBuilder b)
Expand Down Expand Up @@ -94,7 +97,7 @@ public static JetStreamOptionsBuilder Builder(JetStreamOptions jso)
public sealed class JetStreamOptionsBuilder
{
internal string _jsPrefix;
internal Duration _requestTimeout = DefaultTimeout;
internal Duration _requestTimeout;
internal bool _publishNoAck;
internal bool _optOut290ConsumerCreate;

Expand Down Expand Up @@ -160,7 +163,7 @@ public JetStreamOptionsBuilder WithDomain(string domain)
/// <returns>The JetStreamOptionsBuilder</returns>
public JetStreamOptionsBuilder WithRequestTimeout(Duration requestTimeout)
{
_requestTimeout = EnsureNotNullAndNotLessThanMin(requestTimeout, Duration.Zero, DefaultTimeout);
_requestTimeout = requestTimeout;
return this;
}

Expand All @@ -169,9 +172,9 @@ public JetStreamOptionsBuilder WithRequestTimeout(Duration requestTimeout)
/// </summary>
/// <param name="requestTimeoutMillis">The request timeout in millis.</param>
/// <returns>The JetStreamOptionsBuilder</returns>
public JetStreamOptionsBuilder WithRequestTimeout(long requestTimeoutMillis)
public JetStreamOptionsBuilder WithRequestTimeout(long requestTimeoutMillis)
{
_requestTimeout = EnsureDurationNotLessThanMin(requestTimeoutMillis, Duration.Zero, DefaultTimeout);
_requestTimeout = requestTimeoutMillis < 0 ? null : Duration.OfMillis(requestTimeoutMillis);
return this;
}

Expand Down Expand Up @@ -201,7 +204,6 @@ public JetStreamOptionsBuilder WithOptOut290ConsumerCreate(bool optOut) {
/// <returns>The JetStreamOptions object.</returns>
public JetStreamOptions Build()
{
_requestTimeout = _requestTimeout ?? DefaultTimeout;
return new JetStreamOptions(this);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/IntegrationTests/TestObjectStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public void TestObjectStoreOptionsBuilderCoverage() {

private void AssertOso(ObjectStoreOptions oso) {
JetStreamOptions jso = oso.JSOptions;
Assert.Equal(DefaultJsOptions.RequestTimeout, jso.RequestTimeout);
Assert.Null(jso.RequestTimeout);
Assert.Equal(DefaultJsOptions.Prefix, jso.Prefix);
Assert.Equal(DefaultJsOptions.IsDefaultPrefix, jso.IsDefaultPrefix);
Assert.Equal(DefaultJsOptions.IsPublishNoAck, jso.IsPublishNoAck);
Expand Down
5 changes: 2 additions & 3 deletions src/Tests/UnitTests/JetStream/TestJetStreamOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// limitations under the License.

using System;
using NATS.Client;
using NATS.Client.Internals;
using NATS.Client.JetStream;
using Xunit;
Expand All @@ -27,15 +26,15 @@ public void TestBuilder()
{
// default
JetStreamOptions jso = JetStreamOptions.Builder().Build();
Assert.Equal(Duration.OfMillis(Defaults.Timeout), jso.RequestTimeout);
Assert.Null(jso.RequestTimeout);
Assert.Equal(DefaultApiPrefix, jso.Prefix);
Assert.True(jso.IsDefaultPrefix);
Assert.False(jso.IsPublishNoAck);
Assert.False(jso.IsOptOut290ConsumerCreate);

// default copy
jso = JetStreamOptions.Builder(jso).Build();
Assert.Equal(Duration.OfMillis(Defaults.Timeout), jso.RequestTimeout);
Assert.Null(jso.RequestTimeout);
Assert.Equal(DefaultApiPrefix, jso.Prefix);
Assert.True(jso.IsDefaultPrefix);
Assert.False(jso.IsPublishNoAck);
Expand Down

0 comments on commit 607e4c9

Please sign in to comment.