Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report proper errors instead of NREs during trait and weapon linting #20082

Merged
merged 4 commits into from Jun 26, 2022

Conversation

abcdefg30
Copy link
Member

Inspired by #20080. Both Game.CreateObject and modData.ObjectCreator.FindType return null when given an unknown type. That leads to walls of errors like:

OpenRA.Utility(1,1): Error: Missing Type: AsdInfo
OpenRA.Utility(1,1): Error: Missing Type: LeaveSmudgesWarhead
OpenRA.Utility(1,1): Error: Missing Type: BulletsInfo
[...]
OpenRA.Utility(1,1): Error: OpenRA.Mods.Common.Lint.CheckUnknownTraitFields failed with exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at OpenRA.Mods.Common.Lint.CheckUnknownTraitFields.CheckActors(IEnumerable`1 actors, Action`1 emitError, ModData modData) in \OpenRA\OpenRA.Mods.Common\Lint\CheckUnknownTraitFields.cs:line 77
   at OpenRA.Mods.Common.Lint.CheckUnknownTraitFields.OpenRA.Mods.Common.Lint.ILintPass.Run(Action`1 emitError, Action`1 emitWarning, ModData modData) in \OpenRA\OpenRA.Mods.Common\Lint\CheckUnknownTraitFields.cs:line 24
   at OpenRA.Mods.Common.UtilityCommands.CheckYaml.OpenRA.IUtilityCommand.Run(Utility utility, String[] args) in \OpenRA\OpenRA.Mods.Common\UtilityCommands\CheckYaml.cs:line 72
[...]
OpenRA.Utility(1,1): Error: Failed with exception: System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.)
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in \OpenRA\OpenRA.Game\FieldLoader.cs:line 533
   at OpenRA.GameRules.WeaponInfo.LoadProjectile(MiniYaml yaml) in \OpenRA\OpenRA.Game\GameRules\WeaponInfo.cs:line 143
   at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in \OpenRA\OpenRA.Game\FieldLoader.cs:line 547
   at OpenRA.GameRules.WeaponInfo..ctor(MiniYaml content) in \OpenRA\OpenRA.Game\GameRules\WeaponInfo.cs:line 135
   at OpenRA.Ruleset.<>c.<LoadDefaults>b__12_3(MiniYamlNode k) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 134
   at OpenRA.Exts.ToDictionaryWithConflictLog[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, String debugName, Func`2 logKey, Func`2 logValue) in \OpenRA\OpenRA.Game\Exts.cs:line 415
   at OpenRA.Ruleset.MergeOrDefault[T](String name, IReadOnlyFileSystem fileSystem, IEnumerable`1 files, MiniYaml additional, IReadOnlyDictionary`2 defaults, Func`2 makeObject, Func`2 filterNode) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 118
   at OpenRA.Ruleset.<>c__DisplayClass12_0.<LoadDefaults>b__0() in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 133
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__272_0(Object obj)
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout)
   at OpenRA.Ruleset.LoadDefaults(ModData modData) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 160
   at OpenRA.ModData.<.ctor>b__32_2() in \OpenRA\OpenRA.Game\ModData.cs:line 110
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at OpenRA.ModData.get_DefaultRules() in \OpenRA\OpenRA.Game\ModData.cs:line 46
   at OpenRA.Mods.Common.UtilityCommands.CheckYaml.OpenRA.IUtilityCommand.Run(Utility utility, String[] args) in \OpenRA\OpenRA.Mods.Common\UtilityCommands\CheckYaml.cs:line 64
[...]
OpenRA.Utility(1,1): Error: Failed with exception: System.AggregateException: One or more errors occurred. (Object reference not set to an instance of an object.)
 ---> System.NullReferenceException: Object reference not set to an instance of an object.
   at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in \OpenRA\OpenRA.Game\FieldLoader.cs:line 533
   at OpenRA.GameRules.WeaponInfo.LoadWarheads(MiniYaml yaml) in \OpenRA\OpenRA.Game\GameRules\WeaponInfo.cs:line 153
   at OpenRA.FieldLoader.Load(Object self, MiniYaml my) in \OpenRA\OpenRA.Game\FieldLoader.cs:line 547
   at OpenRA.GameRules.WeaponInfo..ctor(MiniYaml content) in \OpenRA\OpenRA.Game\GameRules\WeaponInfo.cs:line 135
   at OpenRA.Ruleset.<>c.<LoadDefaults>b__12_3(MiniYamlNode k) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 134
   at OpenRA.Exts.ToDictionaryWithConflictLog[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, String debugName, Func`2 logKey, Func`2 logValue) in \OpenRA\OpenRA.Game\Exts.cs:line 415
   at OpenRA.Ruleset.MergeOrDefault[T](String name, IReadOnlyFileSystem fileSystem, IEnumerable`1 files, MiniYaml additional, IReadOnlyDictionary`2 defaults, Func`2 makeObject, Func`2 filterNode) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 118
   at OpenRA.Ruleset.<>c__DisplayClass12_0.<LoadDefaults>b__0() in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 133
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.<>c.<.cctor>b__272_0(Object obj)
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout)
   at OpenRA.Ruleset.LoadDefaults(ModData modData) in \OpenRA\OpenRA.Game\GameRules\Ruleset.cs:line 160
   at OpenRA.ModData.<.ctor>b__32_2() in \OpenRA\OpenRA.Game\ModData.cs:line 110
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at System.Lazy`1.get_Value()
   at OpenRA.ModData.get_DefaultRules() in \OpenRA\OpenRA.Game\ModData.cs:line 46
   at OpenRA.Mods.Common.UtilityCommands.CheckYaml.OpenRA.IUtilityCommand.Run(Utility utility, String[] args) in \OpenRA\OpenRA.Mods.Common\UtilityCommands\CheckYaml.cs:line 64

Now just the location is provided without a huge stacktrace (since we don't crash in the first place):

Testing mod: Red Alert
OpenRA.Utility(1,1): Error: Missing Type: AsdInfo
OpenRA.Utility(1,1): Error: Missing Type: LeaveSmudgesWarhead
OpenRA.Utility(1,1): Error: Missing Type: BulletsInfo
[...]
OpenRA.Utility(1,1): Error: ra|rules/infantry.yaml:70 defines unknown trait `Asd`.
OpenRA.Utility(1,1): Error: ra|weapons/ballistics.yaml:20 defines unknown warhead `LeaveSmudges`.
OpenRA.Utility(1,1): Error: ra|weapons/ballistics.yaml:38 defines unknown projectile `Bullets`.
[...]

Testcases included as extra commit.

@abcdefg30 abcdefg30 added this to the Next release milestone Jun 22, 2022
penev92
penev92 previously approved these changes Jun 24, 2022
Copy link
Member

@penev92 penev92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

pchote
pchote previously approved these changes Jun 26, 2022
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO its a bit odd that most cases generate "Error: Missing Type" instead of the custom messages, but this is still an improvement over the existing behaviour.

Please remove testcase and merge.

@abcdefg30
Copy link
Member Author

IMO its a bit odd that most cases generate "Error: Missing Type" instead of the custom messages, but this is still an improvement over the existing behaviour.

That is generated by

ObjectCreator.MissingTypeAction = s => EmitError($"Missing Type: {s}");

@pchote pchote merged commit c1822d1 into OpenRA:bleed Jun 26, 2022
@abcdefg30 abcdefg30 deleted the lintCrashes branch June 27, 2022 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants