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

Breaking Change in MqttClientTcpOptions with 4.3.5.1141 #2005

Closed
rido-min opened this issue May 21, 2024 · 5 comments · Fixed by #2009
Closed

Breaking Change in MqttClientTcpOptions with 4.3.5.1141 #2005

rido-min opened this issue May 21, 2024 · 5 comments · Fixed by #2009
Labels
bug Something isn't working

Comments

@rido-min
Copy link
Member

With 4.3.5.1141 MqttClientTcpOptions Server and Port are marked as obsolete, however those properties return null.

We are extending the MqttClientOptionsBuilder with more .WithXXX methods, to verify those methods we have unit tests like.

 MqttConnectionSettings mqttConnectionSettings = new("localhost")
  {
      ClientId = "clientId",
      TcpPort = 4343,
      Username = "user",
      KeepAlive = TimeSpan.FromSeconds(15),
      CleanStart = false,
  };
  MqttClientOptions options = mqttClientOptionsBuilder.WithMqttConnectionSettings(mqttConnectionSettings).Build();
  Assert.NotNull(options);
  Assert.False(options.CleanSession);
  Assert.Equal("clientId", options.ClientId);
  Assert.Equal(TimeSpan.FromSeconds(15), options.KeepAlivePeriod);
  MqttClientTcpOptions tcpOptions = (MqttClientTcpOptions)options.ChannelOptions;
  Assert.Equal("localhost", tcpOptions.Server);
  Assert.Equal(4343, tcpOptions.Port);

The last two asserts fail with Null.

I tried to use RemoteEndpoint instead, but it does not include properties for Server and Port

Probably related, when connecting the traces show Trying to connect with server 'Unspecified/localhost:1883' (note the Unspecified, as this seems to be RemoteEndpoint.ToString() )

This change is not mentioned in the Release Notes

@rido-min rido-min added the bug Something isn't working label May 21, 2024
@chkr1011
Copy link
Collaborator

The origin of this issue was adding support for unix sockets which exposes the base remote endpoint instead of a dedicated server and port property. In your case you must use a DnsEndPoint for the RemoteEndpoint as the WithTcpServer does. I restored the Server and Port properties because it would lead to a breaking change but unfortunately the behavior of these properties is still different. I will have a look how to restore the behavior...

@chkr1011
Copy link
Collaborator

I restored the old behavior. Please test build 4.3.5.1148 from MyGet and let me know if it works.

@rido-min
Copy link
Member Author

yep, it works. Thanks for the quick update!!

BTW, we will keep the other workaround of casting the RemoteEndpoint to DnsEndPoint to avoid the obsolete warning.

@chkr1011 chkr1011 linked a pull request May 23, 2024 that will close this issue
@lukas-mertens
Copy link

Was this actually published in a release already? Because I don't see a 1148 release, the latest for me is a 1141 release.

@chkr1011
Copy link
Collaborator

It was released a few seconds ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants