From 74798fc8b3ebee008e40ee8aa2988d0ea95374ce Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Fri, 27 Feb 2026 11:21:37 +0100 Subject: [PATCH] fix(channelserver): return error from Save() to prevent misleading success logs CharacterSaveData.Save() silently returned on failure (nil decompressed data, compression error, DB error) while the caller unconditionally logged "Saved character data successfully". This made diagnosing save failures difficult (ref #163). Save() now returns an error, and all six call sites check it. The success log in saveAllCharacterData only fires when the save actually persisted. --- server/channelserver/handlers_character.go | 11 ++++++++--- server/channelserver/handlers_character_test.go | 4 +++- server/channelserver/handlers_data.go | 5 ++++- server/channelserver/handlers_distitem.go | 4 +++- server/channelserver/handlers_guild_ops.go | 4 +++- server/channelserver/handlers_session.go | 9 ++++++++- server/channelserver/handlers_tower.go | 4 +++- 7 files changed, 32 insertions(+), 9 deletions(-) diff --git a/server/channelserver/handlers_character.go b/server/channelserver/handlers_character.go index 3199ff37b..d5fd4d94d 100644 --- a/server/channelserver/handlers_character.go +++ b/server/channelserver/handlers_character.go @@ -3,6 +3,7 @@ package channelserver import ( "database/sql" "errors" + "fmt" cfg "erupe-ce/config" "erupe-ce/network/mhfpacket" @@ -46,12 +47,12 @@ func GetCharacterSaveData(s *Session, charID uint32) (*CharacterSaveData, error) return saveData, nil } -func (save *CharacterSaveData) Save(s *Session) { +func (save *CharacterSaveData) Save(s *Session) error { if save.decompSave == nil { s.logger.Warn("No decompressed save data, skipping save", zap.Uint32("charID", save.CharID), ) - return + return errors.New("no decompressed save data") } if !s.kqfOverride { @@ -66,7 +67,7 @@ func (save *CharacterSaveData) Save(s *Session) { err := save.Compress() if err != nil { s.logger.Error("Failed to compress savedata", zap.Error(err)) - return + return fmt.Errorf("compress savedata: %w", err) } } else { // Saves were not compressed @@ -75,11 +76,15 @@ func (save *CharacterSaveData) Save(s *Session) { if err := s.server.charRepo.SaveCharacterData(save.CharID, save.compSave, save.HR, save.GR, save.Gender, save.WeaponType, save.WeaponID); err != nil { s.logger.Error("Failed to update savedata", zap.Error(err), zap.Uint32("charID", save.CharID)) + return fmt.Errorf("save character data: %w", err) } if err := s.server.charRepo.SaveHouseData(s.charID, save.HouseTier, save.HouseData, save.BookshelfData, save.GalleryData, save.ToreData, save.GardenData); err != nil { s.logger.Error("Failed to update user binary house data", zap.Error(err)) + return fmt.Errorf("save house data: %w", err) } + + return nil } func handleMsgMhfSexChanger(s *Session, p mhfpacket.MHFPacket) { diff --git a/server/channelserver/handlers_character_test.go b/server/channelserver/handlers_character_test.go index f88a205f3..40047c7ae 100644 --- a/server/channelserver/handlers_character_test.go +++ b/server/channelserver/handlers_character_test.go @@ -476,7 +476,9 @@ func TestCharacterSaveData_Save_Integration(t *testing.T) { saveData.WeaponID = 1234 // Save it - saveData.Save(s) + if err := saveData.Save(s); err != nil { + t.Fatalf("Save failed: %v", err) + } // Reload and verify var hr, gr uint16 diff --git a/server/channelserver/handlers_data.go b/server/channelserver/handlers_data.go index d90ac040f..940475347 100644 --- a/server/channelserver/handlers_data.go +++ b/server/channelserver/handlers_data.go @@ -91,7 +91,10 @@ func handleMsgMhfSavedata(s *Session, p mhfpacket.MHFPacket) { } if characterSaveData.Name == s.Name || s.server.erupeConfig.RealClientMode <= cfg.S10 { - characterSaveData.Save(s) + if err := characterSaveData.Save(s); err != nil { + s.logger.Error("Failed to save character data", zap.Error(err)) + return + } s.logger.Info("Wrote recompressed savedata back to DB.") } else { _ = s.rawConn.Close() diff --git a/server/channelserver/handlers_distitem.go b/server/channelserver/handlers_distitem.go index 8d3c8fb76..da0258c2d 100644 --- a/server/channelserver/handlers_distitem.go +++ b/server/channelserver/handlers_distitem.go @@ -156,7 +156,9 @@ func handleMsgMhfAcquireDistItem(s *Session, p mhfpacket.MHFPacket) { saveData, err := GetCharacterSaveData(s, s.charID) if err == nil { saveData.RP += uint16(item.Quantity) - saveData.Save(s) + if err := saveData.Save(s); err != nil { + s.logger.Error("Failed to save RP from dist item", zap.Error(err)) + } } } } diff --git a/server/channelserver/handlers_guild_ops.go b/server/channelserver/handlers_guild_ops.go index 89c9a6c0d..2be04dad5 100644 --- a/server/channelserver/handlers_guild_ops.go +++ b/server/channelserver/handlers_guild_ops.go @@ -183,7 +183,9 @@ func handleDonateRP(s *Session, amount uint16, guild *Guild, _type int) []byte { } } saveData.RP -= amount - saveData.Save(s) + if err := saveData.Save(s); err != nil { + s.logger.Error("Failed to save RP after guild donation", zap.Error(err)) + } switch _type { case 0: if err := s.server.guildRepo.AddRankRP(guild.ID, amount); err != nil { diff --git a/server/channelserver/handlers_session.go b/server/channelserver/handlers_session.go index 2df1d9e7e..1395bc7e9 100644 --- a/server/channelserver/handlers_session.go +++ b/server/channelserver/handlers_session.go @@ -194,7 +194,14 @@ func saveAllCharacterData(s *Session, rpToAdd int) error { } // Save to database (main savedata + user_binary) - characterSaveData.Save(s) + if err := characterSaveData.Save(s); err != nil { + s.logger.Error("Failed to save character data", + zap.Error(err), + zap.Uint32("charID", s.charID), + zap.String("name", s.Name), + ) + return err + } // Save auxiliary data types // Note: Plate data saves immediately when client sends save packets, diff --git a/server/channelserver/handlers_tower.go b/server/channelserver/handlers_tower.go index 346b3b18a..a910cae8a 100644 --- a/server/channelserver/handlers_tower.go +++ b/server/channelserver/handlers_tower.go @@ -376,7 +376,9 @@ func handleMsgMhfPostTenrouirai(s *Session, p mhfpacket.MHFPacket) { sd, err := GetCharacterSaveData(s, s.charID) if err == nil && sd != nil { sd.RP -= pkt.DonatedRP - sd.Save(s) + if err := sd.Save(s); err != nil { + s.logger.Error("Failed to save RP after tower donation", zap.Error(err)) + } result, err := s.server.towerService.DonateGuildTowerRP(pkt.GuildID, pkt.DonatedRP) if err != nil { s.logger.Error("Failed to process tower RP donation", zap.Error(err))