post-kerbin-mining-corporation/DynamicBatteryStorage

Harden initializers that use config database searching.

neitsa opened this issue · 8 comments

neitsa commented

Hi Chris,

  • System: Windows 10 x64
  • KSP: v1.7.3
  • Dynamic Battery Storage: v2.0.5.0

I keep getting this exception:

[EXC 15:20:03.505] InvalidOperationException: Operation is not valid due to the current state of the object
	System.Linq.Enumerable.Single[ConfigNode] (IEnumerable`1 source, System.Func`2 predicate, Fallback fallback)
	System.Linq.Enumerable.Single[ConfigNode] (IEnumerable`1 source, System.Func`2 predicate)
	DynamicBatteryStorage.ModuleCryoTankPowerHandler.GetFuelTypes ()
	DynamicBatteryStorage.ModuleCryoTankPowerHandler.Initialize (.PartModule pm)
	DynamicBatteryStorage.VesselElectricalData.SetupDataHandler (.PartModule pm)
	DynamicBatteryStorage.VesselData.RefreshData (Boolean fromScratch, System.Collections.Generic.List`1 vesselParts)
	DynamicBatteryStorage.VesselData..ctor (System.Collections.Generic.List`1 vesselParts)
	DynamicBatteryStorage.VesselElectricalData..ctor (System.Collections.Generic.List`1 vesselParts)
	DynamicBatteryStorage.VesselDataManager.RefreshVesselData ()
	DynamicBatteryStorage.VesselDataManager.FixedUpdate ()

https://github.com/ChrisAdderley/DynamicBatteryStorage/blob/63f8a4aecf14ba5e11e9dd88805f64db28fe6dcb/Source/DynamicBatteryStorage/Handlers/Power/NFTPowerHandlers.cs#L103-L111

As far as I can tell Single raises an InvalidOperationException on the following conditions:

  • No element satisfies the condition in predicate.
  • More than one element satisfies the condition in predicate.
  • The source sequence is empty.

I can't tell exactly which one of the above conditions happens here.

Please tell me if you need more information.

Thanks a lot!

Log file (2.8 MB; warning 768 MB when decompressed):
KSP.zip

First occurrence at line 51593.

ChrisAdderley commented

I can add some more error handing around it, but it looks like the most likely culprit is a broken config block in a part somewhere. The three conditions you describe are:

  • No element satisfies the condition in predicate. -> broken config for a boiloff fuel type
  • More than one element satisfies the condition in predicate -> Two identical boiloff fuel types specified
  • The source sequence is empty.. -> No boiloff fuel types specified

All of these will actually break CryoTanks but more silently.

I can't immediately reproduce this with a standard install of KSP + CryoTanks and DBS together.

Can you get me a log of the incident with the DebugMode of this mod activated, and a description of the circumstances that this occurs in? If it is a broken config I need to determine what part it is.

neitsa commented

Thank you very much for your prompt response!

I changed the following line in DynamicBatteryStorageSettings.cfg (I didn't change anything else in the config):

DYNAMICBATTERYSTORAGE
{
  // Log extensive debug messages
  DebugMode = true

And then started the game; basically, once in the "KSC view" I switched to the tracking station and picked a vessel randomly, see log line 58297 ([FLIGHT GLOBALS]: Switching To Vessel Sat-Mun-Biome-Scan). The first InvalidOperationException is logged at line 58468.

Please tell me if you need anything else.

409KB (~ 13MB uncompressed): KSP.zip

PS: I just wanted to say that I'm pretty sure it's not a problem in your mod, although I'd like to found the culprit. Thanks a lot for your help and thanks a lot for your amazing mods!

ChrisAdderley commented

No problem. Even if it's not mine with a mod like this I have to at least look into it!

Something is definitely breaking CryoTanks, which is cascading through DBS.

I found a similar CT initialization error here:

[LOG 07:38:43.362] fuelTank.long
[ERR 07:38:43.365] Module ModuleCryoTank threw during OnStart: System.InvalidOperationException: Operation is not valid due to the current state of the object
  at System.Linq.Enumerable.Single[ConfigNode] (IEnumerable`1 source, System.Func`2 predicate, Fallback fallback) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.Single[ConfigNode] (IEnumerable`1 source, System.Func`2 predicate) [0x00000] in <filename unknown>:0 
  at SimpleBoiloff.ModuleCryoTank.OnStart (StartState state) [0x00000] in <filename unknown>:0 
  at Part.ModulesOnStart () [0x00000] in <filename unknown>:0

I'm pretty sure it's KSPI-E. Specifically, this file. Multiple BOILOFFCONFIGS should be inside a single module block instead of having multiple modules defined. This will probably break that Single call.

I could fix this by switching to a FirstOrDefault Linq item or something similar, and probably will, but the fact remains that this way of specifying the configs will give unpredictable results in the game, so I'll contact FreeThinker and hopefully they can resolve the patch for the next version of KSPIE.

neitsa commented

Damn, you're incredible.

Not only you took time to look at this in details but found the problem and then reported it.

I know, by experience, that taking care of the various conflicts that arise on heavily modded installation of KSP takes time and, well, everyone deserves to have a life outside of KSP-related stuff.

I was thinking about doing a PR to KSPI-E but it seems that I don't have anything to do now :)

I just want to say a big thank you for the time you put on this problem and the time you take to build, improve and maintain your mods!

ChrisAdderley commented

Aw thanks! Just part of the job...

Anyways I'll keep this ticket open (with the new title) so I remember to harden that interface's error checking next time I have some time.
Info for future Chris:

  • ModuleCryoTankHander uses a messy method to detect cryo fuel types on a target
  • Improve this method if possible
  • Alternately, harden this method to catch errors better (needs change to CT as well)
  • 🚀1
ChrisAdderley commented

A comprehensive fix was released with CryoTanks 1.3 and DBS 2.0.6

neitsa commented

Thanks a lot for the fix. Much appreciated!