search

sillsdev / web-languageforge

Language Forge: Online Collaborative Dictionary Building on the Web and Phone.
https://languageforge.org
MIT License
44 stars 29 forks source link

kap-flex on hold #1166

Closed megahirt closed 3 years ago

megahirt commented 3 years ago

Project code: kap-flex

Error message:

Putting project 'kap-flex' on hold due to unhandled exception: 
System.ArgumentException: An item with the same key has already been added. Key: kap-GE
  at System.Collections.Generic.Dictionary`2[TKey,TValue].TryInsert (TKey key, TValue value, System.Collections.Generic.InsertionBehavior behavior) [0x0015a] in <c5a54dbe91cb4981b725403e1a171987>:0 
  at System.Collections.Generic.Dictionary`2[TKey,TValue].Add (TKey key, TValue value) [0x00000] in <c5a54dbe91cb4981b725403e1a171987>:0 
  at LfMerge.Core.DataConverters.ConvertLcmToMongoLexicon.LcmWsToLfWs () [0x0009f] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/DataConverters/ConvertLcmToMongoLexicon.cs:617 
  at LfMerge.Core.DataConverters.ConvertLcmToMongoLexicon..ctor (LfMerge.Core.ILfProject lfProject, LfMerge.Core.Logging.ILogger logger, LfMerge.Core.MongoConnector.IMongoConnection connection, SIL.Progress.IProgress progress, LfMerge.Core.MongoConnector.MongoProjectRecordFactory projectRecordFactory) [0x00079] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/DataConverters/ConvertLcmToMongoLexicon.cs:65 
  at LfMerge.Core.Actions.TransferLcmToMongoAction.DoRun (LfMerge.Core.ILfProject project) [0x00104] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/Actions/TransferLcmToMongoAction.cs:64 
  at LfMerge.Core.Actions.Action.Run (LfMerge.Core.ILfProject project) [0x0010d] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/Actions/Action.cs:142 
  at LfMerge.Core.Actions.SynchronizeAction.DoRun (LfMerge.Core.ILfProject project) [0x003d4] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/Actions/SynchronizeAction.cs:196 
  at LfMerge.Core.Actions.Action.Run (LfMerge.Core.ILfProject project) [0x0010d] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge.Core/Actions/Action.cs:142 
  at LfMerge.Program.RunAction (System.String projectCode, LfMerge.Core.Actions.ActionNames currentAction) [0x000ed] in /build/lfmerge-7000072-5wzzi7/lfmerge-7000072-2.0.71~origin-live.16.24/src/LfMerge/Program.cs:128

User reported via issues@languageforge.org

megahirt commented 3 years ago

cf. https://github.com/sillsdev/LfMerge/blob/fa591fcc4b02074079b13ac81caae16211073cfd/src/LfMerge.Core/DataConverters/ConvertLcmToMongoLexicon.cs#L617

Here is the offending line. It assumes that additions to the dictionary are always unique, but for whatever reason there is a duplicate coming out of LcmWs. I suggest wrapping this in an if statement that continues the loop if the key already exists in the dictionary.

lfWsList.Add(LcmWs.LanguageTag, lfWs);
rmunn commented 3 years ago

That is surprising behavior from a C# dictionary in my opinion. I would have expected .Add() to replace an existing value, not throw an exception. I just looked it up and found that where dict.Add(key, value) throws if key already exists, dict[key] = value does not throw, but replaces the value as I would have expected. So this fix will be simple.

megahirt commented 3 years ago

I figured you would know a better way than to add an if statement. Thanks. Of course I don't understand how the FLEx project data has duplicate WS (same language tag) but we can't chase down every unexpected thing in FW and instead make LFMerge more robust.

rmunn commented 3 years ago

Agreed. I'm not going to chase down why there are two WSes with the same tag; I'm just going to let one of them overwrite the other and move on. If it happens to be the "wrong" one that arrives second and "wins", then the project's Mercurial repo will contain its history in enough detail that some tech support person (probably me) can sort it out manually later on.

rmunn commented 3 years ago

Will be fixed by https://github.com/sillsdev/LfMerge/pull/140 and https://github.com/sillsdev/LfMerge/pull/141 once they're merged and released.

jasonleenaylor commented 3 years ago

I agree with not trying to track it down right now. I'm not looking to change your priorities, however, it might be useful to know if it is because the same language appears both the vernacular and analysis lists.

megahirt commented 3 years ago

Jason, that's a really good point that I hadn't thought of that seems likely, especially if lcmWs is a joined collection of analysis and vernacular WSs. Thanks for that insight.

rmunn commented 3 years ago

Just did a very quick check, and the kap writing system is only listed as a vernacular system in the current version (Mercurial tip) of the project. However, the Language Forge project at the time of the failed Send/Receive was about 60 commits back from the tip, and it's possible that the commit the LF project was at had kap in the analysis writing system list due to a mistake that was later corrected. I don't want to spend the time to check that theory now, but this is a note to myself to check on that once I finish my current work.

megahirt commented 3 years ago

@rmunn 's fix shipped yesterday in v1.9 and so I did a sync just now and it succeeded - nice work Robin!