Azure / azure-iot-sdk-csharp

A C# SDK for connecting devices to Microsoft Azure IoT services
Other
466 stars 493 forks source link

[Bug Report] Proxy with MQTT means no channel can get opened #1934

Closed sjoerd222888 closed 3 years ago

sjoerd222888 commented 3 years ago

With https://github.com/Azure/azure-iot-sdk-csharp/pull/1326 the issue https://github.com/Azure/azure-iot-sdk-csharp/issues/1063 got resolved.

However that meant in the case having a proxy defined in the MqttTransportHandler then _serverAddresses is set to an empty array in OpenInternalAsyn

    if (IsProxyConfigured())
            {
                // No need to do a DNS lookup since we have the proxy address already

#if NET451
                _serverAddresses = new IPAddress[0];
#else
                _serverAddresses = Array.Empty<IPAddress>();
#endif
        }
            else
            {
#if NET451
                _serverAddresses = Dns.GetHostEntry(_hostName).AddressList;
#else
                _serverAddresses = await Dns.GetHostAddressesAsync(_hostName).ConfigureAwait(false);
#endif
            }

But now when a channel is openend with

_channel = await _channelFactory(_serverAddresses, ProtocolGatewayPort).ConfigureAwait(true);

So we pass the _serverAddresses, which is an empty array in the proxy case. In CreateChannelFactory we iterate over the _serverAddresses

foreach (IPAddress address in addresses)
{
    try
    {
        if (Logging.IsEnabled)
            Logging.Info(this, $"Connecting to {address}", nameof(CreateChannelFactory));

        channel = await bootstrap.ConnectAsync(address, port).ConfigureAwait(true);
        break;
    }
    catch (AggregateException ae)
    {
        ae.Handle((ex) =>
        {
            if (ex is ConnectException) // We will handle DotNetty.Transport.Channels.ConnectException
            {
                if (Logging.IsEnabled)
                    Logging.Error(this, $"ConnectException trying to connect to {address}: {ex}", nameof(CreateChannelFactory));

                return true;
            }

            return false; // Let anything else stop the application.
        });
    }
    catch (ConnectException ex)
    {
        // same as above, we will handle DotNetty.Transport.Channels.ConnectException
        if (Logging.IsEnabled)
            Logging.Error(this, $"ConnectException trying to connect to {address}: {ex}", nameof(CreateChannelFactory));
    }
}

return channel ?? throw new IotHubCommunicationException("MQTT channel open failed.");

But as this array is empty it it won't even iterate so no connections is opened at all.

We are faced with the MQTT not working in case you have defined a proxy. I currently think what I wrote here is the reason for this. Do you agree?

timtay-microsoft commented 3 years ago

We are faced with the MQTT not working in case you have defined a proxy. I currently think what I wrote here is the reason for this. Do you agree?

This SDK does not support using proxies over MQTT. It only supports proxies over MQTT_WS, HTTPS and AMQPS_WS, so this doesn't seems like a bug to me. If anything, the SDK should probably be throwing an UnsupportedOperationException here to prevent users from trying this.

Is there a particular use case that you are going for here that I'm misunderstanding though? And if so, can you provide some repro code that demonstrates this issue?

sjoerd222888 commented 3 years ago

Thanks for the quick response. What you say absolutely make sense. The code I was faced does not make sense in the first place.