From 4d04a1af028bce7b5c2e4829eab849fcd0e03ce7 Mon Sep 17 00:00:00 2001 From: argonui <92067588+argonui@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:13:08 -0500 Subject: [PATCH] Propagate swallowed read errors in mod generate tryPut discarded the error from reading a *_path target, so a mod referencing a missing file silently produced an empty value. It now returns an error when a present, non-empty *_path pointer fails to resolve, while staying lenient for absent optional keys. The dropped error from ListFilesAndFolders is also propagated so an unreadable objects dir no longer masquerades as "no objects". Fixes #97 Co-Authored-By: Claude Opus 5 --- mod/generate.go | 39 +++++++++++++++++++--------- mod/generate_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/mod/generate.go b/mod/generate.go index f26a066..fc7136c 100644 --- a/mod/generate.go +++ b/mod/generate.go @@ -85,15 +85,21 @@ func (m *Mod) generate(raw types.J) error { ext := "_path" for _, stringbased := range ExpectedStr { - tryPut(&m.Data, stringbased+ext, stringbased, luaGet) + if err := tryPut(&m.Data, stringbased+ext, stringbased, luaGet); err != nil { + return err + } } for _, objbased := range ExpectedObj { - tryPut(&m.Data, objbased+ext, objbased, plainObj) + if err := tryPut(&m.Data, objbased+ext, objbased, plainObj); err != nil { + return err + } } for _, objarraybased := range ExpectedObjArr { - tryPut(&m.Data, objarraybased+ext, objarraybased, objArray) + if err := tryPut(&m.Data, objarraybased+ext, objarraybased, objArray); err != nil { + return err + } } lh := handler.NewLuaHandler() @@ -123,7 +129,10 @@ func (m *Mod) generate(raw types.J) error { } objOrder := []string{} - files, _, _ := m.Objdirs.ListFilesAndFolders("") + files, _, err := m.Objdirs.ListFilesAndFolders("") + if err != nil { + return fmt.Errorf("ListFilesAndFolders(\"\"): %v", err) + } hasObjects := len(files) > 0 err = file.ForceParseIntoStrArray(&m.Data, "ObjectStates_order", &objOrder) @@ -156,19 +165,19 @@ func (m *Mod) Print(basename string) error { } } -func tryPut(d *types.J, from, to string, fun func(string) (interface{}, error)) { +func tryPut(d *types.J, from, to string, fun func(string) (interface{}, error)) error { if d == nil { log.Println("Nil objects") - return + return nil } var o interface{} - fromFile, ok := (*d)[from] - if !ok { + fromFile, pathPresent := (*d)[from] + if !pathPresent { fromFile = "" if _, ok := (*d)[to]; ok { - // if there is not special key, but there is existant key, don't replace anything. - return + // if there is no special key, but there is an existing key, don't replace anything. + return nil } } filename, ok := fromFile.(string) @@ -177,9 +186,15 @@ func tryPut(d *types.J, from, to string, fun func(string) (interface{}, error)) filename = "" } - o, _ = fun(filename) - // ignore error for now + o, err := fun(filename) + if err != nil && pathPresent && filename != "" { + // A present *_path key naming a real file that fails to read is a broken + // pointer and must fail loudly. An absent key (empty filename) is a normal + // optional field, so stay lenient and fall through with the zero value. + return fmt.Errorf("could not resolve %q for key %q: %v", filename, to, err) + } (*d)[to] = o delete((*d), from) + return nil } diff --git a/mod/generate_test.go b/mod/generate_test.go index f6d2711..e309458 100644 --- a/mod/generate_test.go +++ b/mod/generate_test.go @@ -3,6 +3,7 @@ package mod import ( "ModCreator/tests" "ModCreator/types" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -288,3 +289,62 @@ func TestGenerate(t *testing.T) { }) } } + +// TestGenerateBrokenPointer ensures that a present *_path key naming a file that +// cannot be read fails loudly rather than silently producing an empty value. +func TestGenerateBrokenPointer(t *testing.T) { + rootff := &tests.FakeFiles{ + Data: map[string]types.J{ + "config.json": map[string]interface{}{ + // points at a lua file that was never written to disk + "LuaScriptState_path": "missing/does-not-exist.luascriptstate", + }, + }, + } + m := Mod{ + RootRead: rootff, + RootWrite: rootff, + Lua: &tests.FakeFiles{Fs: map[string]string{}}, + Modsettings: &tests.FakeFiles{}, + Objs: &tests.FakeFiles{}, + Objdirs: &tests.FakeFiles{}, + } + err := m.GenerateFromConfig() + if err == nil { + t.Fatalf("expected error for broken *_path pointer, got nil") + } + if !strings.Contains(err.Error(), "missing/does-not-exist.luascriptstate") { + t.Errorf("error should name the missing file, got: %v", err) + } +} + +// TestGenerateAbsentOptionalKey ensures that an optional key that is simply +// absent from config (no *_path and no inline value) stays lenient and does not +// error. +func TestGenerateAbsentOptionalKey(t *testing.T) { + rootff := &tests.FakeFiles{ + Data: map[string]types.J{ + "config.json": map[string]interface{}{ + "SaveName": "a mod with no externalized optional fields", + }, + }, + } + m := Mod{ + RootRead: rootff, + RootWrite: rootff, + Lua: &tests.FakeFiles{Fs: map[string]string{}}, + Modsettings: &tests.FakeFiles{}, + Objs: &tests.FakeFiles{}, + Objdirs: &tests.FakeFiles{}, + } + if err := m.GenerateFromConfig(); err != nil { + t.Fatalf("absent optional keys should not error, got: %v", err) + } + if got := m.Data["SaveName"]; got != "a mod with no externalized optional fields" { + t.Errorf("SaveName not preserved, got %v", got) + } + // An absent optional string key is still filled with its zero value. + if got, ok := m.Data["LuaScriptState"]; !ok || got != "" { + t.Errorf("absent optional key LuaScriptState should be zero-valued, got %v (present=%v)", got, ok) + } +}