modio / modio-unity-legacy

Unity Plugin for integrating mod.io - a modding API for game developers
https://mod.io
MIT License
66 stars 16 forks source link

System.DateTime.ToString can throw exceptions (used in ModIO.ValueFormatting.FormatValue) #85

Open AlexanderLuck opened 2 years ago

AlexanderLuck commented 2 years ago

Hi, in our Unity Windows IL2CPP 2020 LTS project we have received automated exception reports from users who obviously have some kind of odd Windows culture setting. Calls to System.DateTime can throw exceptions. We have fixed usages in our code but the mod.io plugin also accesses this. Based on feedback of players we think this may happening for players with Indonesian OS setup.

We have resorted to wrapping access to code that does access SystemDate time with fallback to enforcing culture to use UTC and exception handling. It may be useful to do the same in this package.

The stack trace that showed up in our system is:

: ArgumentOutOfRangeException: Not a valid calendar for the given culture. Parameter name: value System.Globalization.DateTimeFormatInfo.set_Calendar (System.Globalization.Calendar value) (at <00000000000000000000000000000000>:0) System.Globalization.CultureInfo.get_DateTimeFormat () (at <00000000000000000000000000000000>:0) System.Globalization.DateTimeFormatInfo.get_CurrentInfo () (at <00000000000000000000000000000000>:0) System.DateTime.ToString (System.String format) (at <00000000000000000000000000000000>:0) ModIO.ValueFormatting.FormatValue (System.Object value, ModIO.ValueFormatting+Method method, System.String toStringParameter) (at <00000000000000000000000000000000>:0) ModIO.UI.UserProfileFieldDisplay.OnEnable () (at <00000000000000000000000000000000>:0) UnityEngine.Object.Instantiate[T] (T original) (at <00000000000000000000000000000000>:0) ModIO.UI.UIUtilities.SetInstanceCount[T] (UnityEngine.Transform container, T template, System.String instanceName, System.Int32 instanceCount, T[]& instanceArray, System.Boolean reactivateAll) (at <00000000000000000000000000000000>:0) ModIO.UI.ModfileContainer.DisplayModfiles (System.Collections.Generic.IList`1[T] modfiles) (at <00000000000000000000000000000000>:0) ModIO.UI.ModReleaseHistoryDisplay+<>c__DisplayClass9_0.b__0 (ModIO.RequestPage`1[T] r) (at <00000000000000000000000000000000>:0) System.Action`1[T].Invoke (T obj) (at <00000000000000000000000000000000>:0) ModIO.APIClient+<>c__DisplayClass18_0`1[T].b__0 (System.String responseBody) (at <00000000000000000000000000000000>:0) System.Action`1[T].Invoke (T obj) (at <00000000000000000000000000000000>:0) ModIO.APIClient.HandleGetResponse (UnityEngine.AsyncOperation operation) (at <00000000000000000000000000000000>:0) System.Action`1[T].Invoke (T obj) (at <00000000000000000000000000000000>:0) UnityEngine.AsyncOperation.InvokeCompletionEvent () (at <00000000000000000000000000000000>:0) UnityEngine.Object:Instantiate(T) ModIO.UI.UIUtilities:SetInstanceCount(Transform, T, String, Int32, T[]&, Boolean) ModIO.UI.ModfileContainer:DisplayModfiles(IList`1) ModIO.UI.<>c__DisplayClass9_0:b__0(RequestPage`1) System.Action`1:Invoke(T) ModIO.<>c__DisplayClass18_0`1:b__0(String) System.Action`1:Invoke(T) ModIO.APIClient:HandleGetResponse(AsyncOperation) UnityEngine.AsyncOperation:InvokeCompletionEvent()
modio-jackson commented 2 years ago

Hi, thanks for logging the issue. Are you able to send me any reference links, or code snippets such that I can look to resolve this within our codebase? Although I believe I understand the concept at a high level (that on some systems the value being assigned to DateTimeFormatInfo.Calendar is invalid - MSDN Documentation), I'm not sure I fully understand the issue or the solution you've implemented. Is this on Windows 10? Windows 11? Or is it other OS?

AlexanderLuck commented 2 years ago

Hi,

Our problem main issue was that we have been using DateTime.Now to log times at certain points in the game. The unhandled exception froze players in the loading screen. Using DateTime.ToString was only a bonus for logs and therefore we just cut that from our code and as a precaution used regular UnityEngine.Time functionality instead of DateTime where possible.

Our access to DateTime only happens in try/catch static functions, a proposed implementation is at the end of my post.

We have exception reporting from 4 different users about Mod.Io using Windows 10. In my experience with Microsoft on the topic of language/culture settings I'd assume this is issue is also present on 11 and probably a few versions back as well.

In the case of the Mod.Io the problem actually only seems to be the ToString in the following line of ValueFormatting.cs:

displayString = ServerTimeStamp.ToLocalDateTime((int)value).ToString(toStringParameter);

Presumable working with a function like this should address the problem:

public static string DateTimeToString(DateTime dateTime, string format) { try { return dateTime.ToString(format); } catch (Exception e) { //some adequate warning log }

return dateTime.ToString(format, CultureInfo.InvariantCulture);

}

modio-jackson commented 2 years ago

Hi, Thanks for providing the additional information, I really appreciate the time you've taken to write this up. I'll make sure that a fix for this is in the next release of the Plugin.