I often print code and review it away from a computer. I can’t be distracted by “tinkering” with the code and I find that drawing on it helps me visualise what is going on. The payload formatters are retrieved from Azure Storage blob which have a default retry policy, the Azure IoT Hub DeviceClient methods have a default retry policy, the Myriota Cloud API SendMessage has retries (Implemented with Polly) and if the CS-Script compilation fails there is nothing that can be done so the code could be simplified.
The Azure IoT Hub downlink message handler was a partial class and part of implementation of the IDeviceConnectionCache which was a hangover from one of the initial versions.
internal partial class DeviceConnectionCache : IDeviceConnectionCache
{
public async Task AzureIoTHubMessageHandler(Message message, object userContext)
{
Models.DeviceConnectionContext context = (Models.DeviceConnectionContext)userContext;
_logger.LogInformation("Downlink- IoT Hub TerminalId:{termimalId} LockToken:{LockToken}", context.TerminalId, message.LockToken);
I replaced the IDeviceConnectionCache interface with IIoTHubDownlink which was declare in a new file in the interfaces folder.
namespace devMobile.IoT.MyriotaAzureIoTConnector.Connector
{
public interface IIoTHubDownlink
{
public Task AzureIoTHubMessageHandler(Message message, object userContext);
}
}
Then had to inject all the required dependencies which had been implemented in one of the other partial class files.
internal class IoTHubDownlink : IIoTHubDownlink
{
private readonly ILogger<IoTHubDownlink> _logger;
private readonly IPayloadFormatterCache _payloadFormatterCache;
private readonly IMyriotaModuleAPI _myriotaModuleAPI;
public IoTHubDownlink(ILogger<IoTHubDownlink> logger, IPayloadFormatterCache payloadFormatterCache, IMyriotaModuleAPI myriotaModuleAPI)
{
_logger = logger;
_payloadFormatterCache = payloadFormatterCache;
_myriotaModuleAPI = myriotaModuleAPI;
}
...
}
The implementation had been extracted to a separate class so it had to be constructed by the Dependency Injection plumbing.
...
services.AddSingleton<IPayloadFormatterCache, PayloadFormatterCache>();
services.AddSingleton<IIoTHubDownlink, IoTHubDownlink>();
services.AddSingleton<IIoTCentralDownlink, IoTCentralDownlink>();
services.AddOptions<Models.MyriotaSettings>().Configure<IConfiguration>((settings, configuration) =>
{
configuration.GetSection("Myriota").Bind(settings);
});
services.AddSingleton<IMyriotaModuleAPI, MyriotaModuleAPI>();
...
The lifetime of the Microsoft.Azure.Devices.Client.Message was being managed manually which seemed a bit odd.
public async Task AzureIoTHubMessageHandler(Message message, object userContext)
{
Models.DeviceConnectionContext context = (Models.DeviceConnectionContext)userContext;
_logger.LogInformation("Downlink- IoT Hub TerminalId:{termimalId} LockToken:{LockToken}", context.TerminalId, message.LockToken);
// Use default formatter and replace with message specific formatter if configured.
string payloadFormatter;
if (!message.Properties.TryGetValue(Constants.IoTHubDownlinkPayloadFormatterProperty, out payloadFormatter) || string.IsNullOrEmpty(payloadFormatter))
{
payloadFormatter = context.PayloadFormatterDownlink;
}
_logger.LogInformation("Downlink- IoT Hub TerminalID:{termimalId} LockToken:{LockToken} Payload formatter:{payloadFormatter} ", context.TerminalId, message.LockToken, payloadFormatter);
try
{
...
}
finally
{
// Mop up the non managed resources of message
message.Dispose();
}
}
I replaced this with a with a “using” which “automagically” manages the lifetime of any non-managed resources. I also added string Locktoken variable for DeviceClient.RejectAsync and DeviceClient.CompletedAsync so that the “using” could be inside the try/catch (there is scope to reduce the amount of code in the “using”)
public async Task AzureIoTHubMessageHandler(Message message, object userContext)
{
Models.DeviceConnectionContext context = (Models.DeviceConnectionContext)userContext;
_logger.LogInformation("Downlink- IoT Hub TerminalId:{TermimalId} LockToken:{lockToken}", context.TerminalId, message.LockToken);
// broken out so using for message only has to be inside try
string lockToken = message.LockToken;
try
{
using (message)
{
...
}
catch (Exception ex)
{
_logger.LogError(ex, "Downlink- IoT Hub TerminalID:{TerminalId} LockToken:{lockToken} MessageHandler processing failed", context.TerminalId, lockToken);
await context.DeviceClient.RejectAsync(lockToken);
}
}
}
The handling of the Encoding.UTF8.GetString and JObject.Parse payload was broken. If the Encoding.UTF8.GetString threw an exception there was no point in calling the JObject.Parse
// If this fails payload broken
byte[] messageBytes = message.GetBytes();
// This will fail for some messages, payload formatter gets bytes only
string messageText = string.Empty;
try
{
messageText = Encoding.UTF8.GetString(messageBytes);
}
catch (ArgumentException aex)
{
_logger.LogInformation("Downlink- IoT Hub TerminalID:{TerminalId} LockToken:{LockToken} messageBytes:{2} not valid Text", context.TerminalId, message.LockToken, BitConverter.ToString(messageBytes));
}
// This will fail for some messages, payload formatter gets bytes only
JObject? messageJson = null;
try
{
messageJson = JObject.Parse(messageText);
}
catch ( JsonReaderException jex)
{
_logger.LogInformation("Downlink- IoT Hub TerminalID:{TerminalId} LockToken:{LockToken} messageText:{2} not valid json", context.TerminalId, message.LockToken, BitConverter.ToString(messageBytes));
}
The Encoding.UTF8.GetString and JObject.Parse are now processed in a single Try with a specialised catch handling.
// These will fail for some messages, then payload formatter gets bytes only
string messageText = string.Empty;
JObject? messageJson = null;
try
{
messageText = Encoding.UTF8.GetString(messageBytes);
messageJson = JObject.Parse(messageText);
}
catch (ArgumentException aex)
{
_logger.LogInformation("Downlink- IoT Hub TerminalID:{TerminalId} LockToken:{lockToken} messageBytes:{messageBytes} not valid text exception:{Message}", context.TerminalId, lockToken, BitConverter.ToString(messageBytes), aex.Message);
}
catch (JsonReaderException jex)
{
_logger.LogInformation("Downlink- IoT Hub TerminalID:{TerminalId} LockToken:{lockToken} messageText:{messageText} not valid json exception:{Message}", context.TerminalId, lockToken, messageText, jex.Message);
}
// This shouldn't fail, but it could for lots of different reasons, invalid path to blob, syntax error, interface broken etc.
IFormatterDownlink payloadFormatter = await _payloadFormatterCache.DownlinkGetAsync(payloadFormatterName);
This refactored code now looks an awful lot like the “sunny days” code checked in on the 3rd of November.
devMobile's blog 