SpryFox/DarkConfig

Better error messages when failing to reify objects

grahamboree opened this issue · 6 comments

Frequently, when an error occurs reifying an object with DarkConfig, an exception will be thrown with a deep internal callstack, and no indication of where the error originated from in the config:

ArgumentException: An empty string is not considered a valid value.
System.Enum.Parse (System.Type enumType, System.String value, Boolean ignoreCase) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System/Enum.cs:615)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:138)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:458)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:378)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:320)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:304)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:260)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:458)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:378)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:320)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:304)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:260)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:458)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:421)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:320)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options) (at /Users/graham/Documents/workspace/DarkConfig/Assets/DarkConfig/ConfigReifier.cs:304)

Having better error messages that will give line/column numbers would be extremely useful in tracking down and fixing the config issue causing this.

rdw commented

Totally agreed. It's actually pretty quick to fix these, we really just need to wrap the places where an exception is thrown. DocNodes have a SourceInformation property which contains the file and line number.

I guess we could just have one big try/catch wrapping the contents of ValueOfType.

Nice! The error message that gets shown in Unity though is still the original exception, which isn't as user-friendly as it could be. The stack trace has the line number/col a few lines down, but it's there, which is 800000 times nicer than not having it!

ArgumentException: The requested value 'xxxxxxxxxxxxxxxx' was not found.
System.Enum.Parse (System.Type enumType, System.String value, Boolean ignoreCase) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System/Enum.cs:692)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options)
Rethrow as ParseException: Exception based on document starting at: File: xxxxx v-1, Line: 27, Col: 15, Idx: 626
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options)
rdw commented

Right, yeah. I'd love to fix that, but I'm not sure how to without completely clobbering the inner exception. Which might be the right thing to do! I looks like Unity prints these exceptions by walking the linked list of inner exceptions to find the innermost, then prints the Message and StackTrace of each of them back up to the parent.

rdw commented

Hm OK I may have come up with something incredibly hacky but working, check this out. Here's the exception class:


    public class ParseException : Exception {
        Exception privateInner;

        public ParseException(string message, Exception inner)
            : base(inner.Message + "\n" + message) {

            privateInner = inner;
        }

        public override string StackTrace {
            get {
                return privateInner.StackTrace + "\n-----\n" + base.StackTrace;
            }
        }
    }

And here's the stacktrace it'll generate. Check out how deeply nested those can be. I tried not re-throwing every ParseException down the line because it's absurd, but then we lose the very top of the stack and don't know where in our code we chose to call Reify.


ParseException: Unknown char: k
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 15, Col: 25, Idx: 244
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 15, Col: 21, Idx: 240
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 15, Col: 21, Idx: 240
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 14, Col: 11, Idx: 210
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 14, Col: 9, Idx: 208
Exception based on document starting at: File: /Users/breath/Documents/DarkConfigOSS/Assets/Demo/Resources/Configs/Planes/Whip, Line: 2, Col: 5, Idx: 10
Exception based on document starting at: ComposedDocNode Dictionary
System.Double.Parse (System.String s, NumberStyles style, IFormatProvider provider) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System/Double.cs:209)
System.Single.Parse (System.String s, IFormatProvider provider) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System/Single.cs:193)
System.Convert.ToSingle (System.String value, IFormatProvider provider) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System/Convert.cs:1703)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:98)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.Reify[Single] (System.Single& obj, DarkConfig.DocNode doc, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:32)
DarkConfig.DocNodeExtensions.As[Single] (DarkConfig.DocNode doc) (at Assets/DarkConfig/DocNodeExtensions.cs:27)
DarkConfig.DocNodeExtensions.AsFloat (DarkConfig.DocNode doc) (at Assets/DarkConfig/DocNodeExtensions.cs:14)
DarkConfig.UnityFromDocs.FromVector2 (System.Object obj, DarkConfig.DocNode value) (at Assets/DarkConfig/Unity/UnityFromDocs.cs:23)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:149)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.Reify[Vector2] (UnityEngine.Vector2& obj, DarkConfig.DocNode doc, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:32)
Location.FromDoc (.Location existing, DarkConfig.DocNode doc) (at Assets/Demo/Scripts/Location.cs:24)
System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:222)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:460)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options) (at Assets/DarkConfig/ConfigReifier.cs:423)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:322)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:306)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:260)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:242)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.SetField (System.Reflection.FieldInfo f, System.Object& obj, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:460)
DarkConfig.ConfigReifier.SetFieldsOnObject (System.Type type, System.Object& obj, DarkConfig.DocNode doc, ConfigOptions options) (at Assets/DarkConfig/ConfigReifier.cs:423)
DarkConfig.ConfigReifier.SetFieldsOnObject[Object] (System.Object& obj, DarkConfig.DocNode dict, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:322)
DarkConfig.ConfigReifier.CreateInstance (System.Type t, DarkConfig.DocNode dict, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:306)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:260)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:210)
-----
DarkConfig.ConfigReifier.ValueOfType (System.Type fieldType, System.Object existing, DarkConfig.DocNode value, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:281)
DarkConfig.ConfigReifier.Reify[Dictionary`2] (System.Collections.Generic.Dictionary`2& obj, DarkConfig.DocNode doc, Nullable`1 options) (at Assets/DarkConfig/ConfigReifier.cs:32)
DarkConfig.Config.Apply[Dictionary`2] (System.String filename, System.Collections.Generic.Dictionary`2& obj) (at Assets/DarkConfig/Config.cs:78)
PlaneCard.LoadConfigs () (at Assets/Demo/Scripts/PlaneCard.cs:53)
LoadGame.StartGame () (at Assets/Demo/Scripts/LoadGame.cs:39)
DarkConfig.ConfigFileManager+<Preload>c__AnonStorey2.<>m__0 () (at Assets/DarkConfig/ConfigFileManager.cs:93)
DarkConfig.FileSource.Preload (System.Action callback) (at Assets/DarkConfig/FileSource.cs:48)
DarkConfig.ConfigFileManager.Preload (System.Action callback) (at Assets/DarkConfig/ConfigFileManager.cs:72)
DarkConfig.Config.Preload (System.Action callback) (at Assets/DarkConfig/Config.cs:43)
LoadGame+<WaitAndStartGame>c__Iterator0.MoveNext () (at Assets/Demo/Scripts/LoadGame.cs:28)
UnityEngine.SetupCoroutine.InvokeMoveNext (IEnumerator enumerator, IntPtr returnValueAddress) (at /Users/builduser/buildslave/unity/build/Runtime/Export/Coroutines.cs:17)

I mean, I wouldn't want anyone to think that we were writing Java here. :D

Hahahah I love it. Kinda weird, but makes the error message in Unity much nicer, and isn't intractable if an engineer wants to dig through the stack trace. 👍

rdw commented

Gonna resolve this. If you find any more stacks that don't have line numbers, please open an issue for them! We'll clobber them all.