Skip to content

Commit

Permalink
No longer throw exception when receiving non success CONNACK
Browse files Browse the repository at this point in the history
  • Loading branch information
chkr1011 committed May 15, 2024
1 parent 4adfca1 commit f9030e8
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public async Task Return_Non_Success()

var client = testEnvironment.CreateClient();

var response = await client.ConnectAsync(testEnvironment.CreateDefaultClientOptionsBuilder().WithoutThrowOnNonSuccessfulConnectResponse().Build());
var response = await client.ConnectAsync(testEnvironment.CreateDefaultClientOptionsBuilder().Build());

Assert.IsNotNull(response);
Assert.AreEqual(MqttClientConnectResultCode.QuotaExceeded, response.ResultCode);
Expand Down
6 changes: 4 additions & 2 deletions Source/MQTTnet.Tests/Server/General.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,10 @@ public async Task Deny_Connection()
return CompletedTask.Instance;
};

var connectingFailedException = await Assert.ThrowsExceptionAsync<MqttConnectingFailedException>(() => testEnvironment.ConnectClient());
Assert.AreEqual(MqttClientConnectResultCode.NotAuthorized, connectingFailedException.ResultCode);
var client = testEnvironment.CreateClient();
var response = await client.ConnectAsync(testEnvironment.CreateDefaultClientOptions());

Assert.AreEqual(MqttClientConnectResultCode.NotAuthorized, response.ResultCode);
}
}

Expand Down
17 changes: 8 additions & 9 deletions Source/MQTTnet.Tests/Server/Security_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public async Task Do_Not_Affect_Authorized_Clients()
using (var testEnvironment = CreateTestEnvironment())
{
testEnvironment.IgnoreClientLogErrors = true;

await testEnvironment.StartServer();

var publishedApplicationMessages = new List<MqttApplicationMessage>();
Expand Down Expand Up @@ -81,15 +81,15 @@ await invalidClient.ConnectAsync(
}

await LongTestDelay();

await validClient.PublishStringAsync("HELLO 2");

await LongTestDelay();

await validClient.PublishStringAsync("HELLO 3");

await LongTestDelay();

Assert.AreEqual(3, publishedApplicationMessages.Count);
Assert.AreEqual(1, testEnvironment.Server.GetClientsAsync().GetAwaiter().GetResult().Count);
}
Expand Down Expand Up @@ -133,7 +133,6 @@ public async Task Use_Username_Null_Password_Empty()
var ex = await Assert.ThrowsExceptionAsync<MqttConnectingFailedException>(async () => await client.ConnectAsync(clientOptions));
Assert.IsInstanceOfType(ex.InnerException, typeof(MqttProtocolViolationException));
Assert.AreEqual("Error while authenticating. If the User Name Flag is set to 0, the Password Flag MUST be set to 0 [MQTT-3.1.2-22].", ex.Message, false);
Assert.AreEqual(MqttClientConnectResultCode.UnspecifiedError, ex.ResultCode);
}
}

Expand Down Expand Up @@ -164,8 +163,8 @@ async Task TestCredentials(string userName, string password)

var clientOptions = new MqttClientOptionsBuilder().WithTcpServer("127.0.0.1", testEnvironment.ServerPort).WithCredentials(userName, password).Build();

var ex = await Assert.ThrowsExceptionAsync<MqttConnectingFailedException>(() => client.ConnectAsync(clientOptions));
Assert.AreEqual(MqttClientConnectResultCode.BadUserNameOrPassword, ex.Result.ResultCode);
var response = await client.ConnectAsync(clientOptions);
Assert.AreEqual(MqttClientConnectResultCode.BadUserNameOrPassword, response.ResultCode);
}
}
}
Expand Down
60 changes: 24 additions & 36 deletions Source/MQTTnet.Tests/Server/Server_Reference_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,51 +4,39 @@

using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using MQTTnet.Adapter;
using MQTTnet.Client;
using MQTTnet.Formatter;
using MQTTnet.Implementations;
using MQTTnet.Internal;
using MQTTnet.Protocol;

namespace MQTTnet.Tests.Server
namespace MQTTnet.Tests.Server;

[TestClass]
public sealed class Server_Reference_Tests : BaseTestClass
{
[TestClass]
public sealed class Server_Reference_Tests : BaseTestClass
[TestMethod]
public async Task Server_Reports_With_Reference_Server()
{
[TestMethod]
public async Task Server_Reports_With_Reference_Server()
using (var testEnvironment = CreateTestEnvironment())
{
using (var testEnvironment = CreateTestEnvironment())
testEnvironment.IgnoreClientLogErrors = true;

var server = await testEnvironment.StartServer();

server.ValidatingConnectionAsync += e =>
{
testEnvironment.IgnoreClientLogErrors = true;

var server = await testEnvironment.StartServer();

server.ValidatingConnectionAsync += e =>
{
e.ReasonCode = MqttConnectReasonCode.ServerMoved;
e.ServerReference = "new_server";
return CompletedTask.Instance;
};

try
{
var client = testEnvironment.CreateClient();

await client.ConnectAsync(new MqttClientOptionsBuilder()
.WithProtocolVersion(MqttProtocolVersion.V500)
.WithTcpServer("127.0.0.1", testEnvironment.ServerPort)
.Build());

Assert.Fail();
}
catch (MqttConnectingFailedException e)
{
Assert.AreEqual(MqttClientConnectResultCode.ServerMoved, e.ResultCode);
Assert.AreEqual("new_server", e.Result.ServerReference);
}
}
e.ReasonCode = MqttConnectReasonCode.ServerMoved;
e.ServerReference = "new_server";
return CompletedTask.Instance;
};

var client = testEnvironment.CreateClient();

var response = await client.ConnectAsync(
new MqttClientOptionsBuilder().WithProtocolVersion(MqttProtocolVersion.V500).WithTcpServer("127.0.0.1", testEnvironment.ServerPort).Build());

Assert.AreEqual(MqttClientConnectResultCode.ServerMoved, response.ResultCode);
Assert.AreEqual("new_server", response.ServerReference);
}
}
}
8 changes: 1 addition & 7 deletions Source/MQTTnet/Adapter/MqttConnectingFailedException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,13 @@
// See the LICENSE file in the project root for more information.

using System;
using MQTTnet.Client;
using MQTTnet.Exceptions;

namespace MQTTnet.Adapter;

public sealed class MqttConnectingFailedException : MqttCommunicationException
{
public MqttConnectingFailedException(string message, Exception innerException, MqttClientConnectResult connectResult) : base(message, innerException)
public MqttConnectingFailedException(string message, Exception innerException) : base(message, innerException)
{
Result = connectResult;
}

public MqttClientConnectResult Result { get; }

public MqttClientConnectResultCode ResultCode => Result?.ResultCode ?? MqttClientConnectResultCode.UnspecifiedError;
}
20 changes: 1 addition & 19 deletions Source/MQTTnet/Client/MqttClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,6 @@ public async Task<MqttClientConnectResult> ConnectAsync(MqttClientOptions option
}
catch (Exception exception)
{
if (exception is MqttConnectingFailedException connectingFailedException)
{
connectResult = connectingFailedException.Result;
}

_disconnectReason = (int)MqttClientDisconnectOptionsReason.UnspecifiedError;

_logger.Error(exception, "Error while connecting with server");
Expand Down Expand Up @@ -479,20 +474,7 @@ async Task<MqttClientConnectResult> Authenticate(IMqttChannelAdapter channelAdap
}
catch (Exception exception)
{
throw new MqttConnectingFailedException($"Error while authenticating. {exception.Message}", exception, null);
}

// This is no feature. It is basically a backward compatibility option and should be removed in the future.
// The client should not throw any exception if the transport layer connection was successful and the server
// did send a proper ACK packet with a non success response.
if (options.ThrowOnNonSuccessfulConnectResponse)
{
if (result.ResultCode != MqttClientConnectResultCode.Success)
{
_logger.Warning(
"Client will now throw an _MqttConnectingFailedException_. This is obsolete and will be removed in the future. Consider setting _ThrowOnNonSuccessfulResponseFromServer=False_ in client options.");
throw new MqttConnectingFailedException($"Connecting with MQTT server failed ({result.ResultCode}).", null, result);
}
throw new MqttConnectingFailedException($"Error while authenticating. {exception.Message}", exception);
}

_logger.Verbose("Authenticated MQTT connection with server established.");
Expand Down
5 changes: 0 additions & 5 deletions Source/MQTTnet/Client/Options/MqttClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ public sealed class MqttClientOptions
/// </summary>
public uint SessionExpiryInterval { get; set; }

/// <summary>
/// Gets or sets whether an exception should be thrown when the server has sent a non success ACK packet.
/// </summary>
public bool ThrowOnNonSuccessfulConnectResponse { get; set; } = true;

/// <summary>
/// Gets or sets the timeout which will be applied at socket level and internal operations.
/// The default value is the same as for sockets in .NET in general.
Expand Down
10 changes: 0 additions & 10 deletions Source/MQTTnet/Client/Options/MqttClientOptionsBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,16 +230,6 @@ public MqttClientOptionsBuilder WithoutPacketFragmentation()
return this;
}

/// <summary>
/// The client will not throw an exception when the MQTT server responds with a non success ACK packet.
/// This will become the default behavior in future versions of the library.
/// </summary>
public MqttClientOptionsBuilder WithoutThrowOnNonSuccessfulConnectResponse()
{
_options.ThrowOnNonSuccessfulConnectResponse = false;
return this;
}

public MqttClientOptionsBuilder WithProtocolType(ProtocolType protocolType)
{
_tcpOptions.ProtocolType = protocolType;
Expand Down

0 comments on commit f9030e8

Please sign in to comment.