Critical: Bug in date handling

[davidkdb]

Hi,

In a webhook handler we are parsing the Order object.

This is on the Customer object in Order:

                "accepts_marketing_updated_at": "0000-12-31T18:03:24-05:50",
		"marketing_opt_in_level": "unknown",
		"tax_exemptions": [],
		"email_marketing_consent": {
			"state": "subscribed",
			"opt_in_level": "unknown",
			"consent_updated_at": "0000-12-31T18:03:24-05:50"
		},

The Json desrialize fails on consent_updated_at

I see in your code that on accepts_marketing_updated_at you are using some null value handling, but that's not on consent_updated_at

Please add this handling to consent_updated_at also because our code fails all the time now on this order.

For now we have to replace the text, and retry the Json deserialize.

This issue is mentioned here: https://community.shopify.com/c/shopify-apis-and-sdks/receiving-webhooks-with-invalid-dates/m-p/1793324

[clement911]

I've encountered the same issue with a few webhooks on Friday (I'm the person that authored the post you mentioned above).

IMO this is a bug from Shopify, not ShopifySharp.
In their response, they confirmed that they had deployed a bug that resulted in some invalid dates being recorded in the API.

I also authored the InvalidDateToNullConverter to cope with these invalid dates, as Shopify has sent these before.

It's kind of a hack though.
In some situations it is probably better to throw an error than to convert the invalid date to null. Or, they might be better strategies. For example, in the instances I've encountered, the date part is messed up (e.g. 0000-12-21) but the time part seems valid (.e.g. 18:03:24-05:50). Maybe it is smarter to create a new datetime, with the current date and the time returned from the API. That will not always be correct, but maybe better than a null.

I've sometimes thought that ShopifySharp should have a way to raise important events for the host application to listen to.
That would be a good use case. ShopifySharp could raise an event saying that an invalid date was received and ask the application what it should do. Set to NULL? Throw an error? Use today's date + the time part from the API? call back a custom function to create a date?
This would also be useful for the host application to raise an alert if needed and log the error.

I think there are other important events that ShopifySharp could issue.
For example, it could let the host application know when rate limit has been breached and it starts waiting for the leaky bucket to replenish.
It could also be useful to log various errors and warnings.
Another interesting event might be that the user posted a graphQL query without specifying the query cost (which can lead to suboptimal bucket use).

@nozzlegear any thoughts?

[nozzlegear]

Should be fixed in 5.18.7, @davidkdb thanks for opening this issue and bringing it to my attention!

[davidkdb]

BTW: After the fix, accepts_marketing_updated_at seems to not work anymore with invalid null date. So what was working, is not working anymore.