EdgeHub fails to parse valid D2C creation time UTC string

stefanb2 commented 6 months ago

Expected Behavior

I follow the System Properties of D2C IoT messages table and add a creation time property to event messages. Those messages will be routed

from my Azure IoT Edge module (FROM /message/modules/<MODULE>/outputs/<OUTPUT>)
via edgeHub
to the Azure IoT Hub (INTO $upstream)

Message creation & sending:

// e.g. "2024-05-23T07:50:52.598Z"
const now = new Date().toISOString();
const message = new Message(JSON.stringify(data));

message.properties.add('iothub-creation-time-utc', now);

client.sendOutputEvent(output, message);

This works fine when module uses MQTT protocol to connect to edgeHub:

import { Mqtt } from 'azure-iot-device-mqtt'
import { ModuleClient } from 'azure-iot-device'

client = await ModuleClient.fromEnvironment(Mqtt)

Current Behavior

When I switch the module from MQTT to AMQP protocol (NOTE: no other changes to the code!):

import { Amqp } from 'azure-iot-device-amqp'
import { ModuleClient } from 'azure-iot-device'

client = await ModuleClient.fromEnvironment(Amqp)

then messages are no longer routed from the Azure IoT Edge device to the Azure IoT Hub.

Instead you can now see an exception in edgeHub log, f.ex.

System.FormatException: String '2024-05-23T07:50:52.598Z' was not recognized as a valid DateTime.

The traceback points to this line in the edgeHub source code in DeviceClientMessageConverter.cs:

if (inputMessage.SystemProperties.TryGetNonEmptyValue(SystemProperties.CreationTime, out string creationTime))
{
  message.CreationTimeUtc = DateTime.ParseExact(creationTime, "o", CultureInfo.InvariantCulture);
}

The "o" format in the above line only seems to accept YYYY-MM-DDTHH:MM:SS.1234567Z as valid. I confirmed this by applying the following workaround to my code:

message.properties.add('iothub-creation-time-utc', now.replace(/Z$/, '0000Z'));

With the workaround in place the errors disappeared from the log and messages were routed again from the Azure IoT Edge device to the Azure IoT Hub.

Steps to Reproduce

create an Azure IoT Edge device
create a custom module with AMQP as protocol
see the code snippets in the above description for the message creation
deploy custom module to Azure IoT Edge device
monitor events from the Azure IoT Edge device on the Azure IoT Hub

Context (Environment)

Device Information

Host OS: Debian GNU/Linux 12 (bookworm) (tailored build of Raspberry Pi OS)
Architecture: arm64
Container OS: Linux containers

Runtime Versions

aziot-edged: 1.5.0
Edge Agent: 1.5
Edge Hub: 1.5
Docker/Moby: 26.1.2

Additional Information

What is the intention behind the strict DateTime.ParseExact(..., "o", ...) ("The round-trip ("O", "o") format specifier" according to .NET documentation)? It should accept any valid UTC ISO-8601 string.

damonbarry commented 4 weeks ago

@stefanb2 Thanks for reporting this. It does appear to be a bug in Edge Hub: I believe the example in the docs would also cause the exception you reported because it doesn't include a fraction value before the "Z".

I'm a little reluctant to make any changes here because this code has been in place since the beginning of the project. I worry that any changes here might break someone else who relied on the existing behavior. But perhaps it would be good to investigate whether "downgrading" the parsing call from DateTime.ParseExact to DateTime.Parse could correct this problem without breaking anything.

ryanwinter commented 3 weeks ago

Closing this as "won't fix" as there is a workaround and changing the behaviour may introduce unexpected consequences.

Please reopen if you are unable to work around this issue.

stefanb2 commented 3 weeks ago

Closing this as "won't fix" as there is a workaround and changing the behaviour may introduce unexpected consequences.

This solution is not acceptable.

Pointing to a workaround is invalid, because this broken behavior is not documented.

Please reopen if you are unable to work around this issue.

That option is disabled: "You do not have permission to reopen this issue".

ryanwinter commented 3 weeks ago

Reopening issue, sorry I didnt mean to dismiss this issue so quickly, I think I misunderstood the documentation explicitly not aligning with the functionality.

Talking to @damonbarry, perhaps the codepath that @stefanb2 pointed, we could a catch the exception from ParseExact, and then try again with a Parse. This might reduce a chance of a regression?

jlian commented 2 weeks ago

We are investigating more.

In the meantime, @PatAltimore and @bishal41 can you help with correcting the doc issue @damonbarry mentioned? Also can we document this as a known issue?

jlian commented 5 days ago

@ancaantochi and I talked about this. Seems like replacing the call from DateTime.ParseExact to DateTime.Parse might be a breaking change or at least risky for existing clients FYI @damonbarry

@ancaantochi said that's it's probably worth looking into why it doesn't work for AMQP even though the call for MQTT is the same. Might have something to do with the data format handling for AMQP, like maybe it's handled for MQTT but not AMQP. @yophilav would you be able help look into this a bit further?

Azure / iotedge