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.
This commit is contained in:
Houmgaor
2026-02-27 11:21:37 +01:00
parent 178a008e25
commit 74798fc8b3
7 changed files with 32 additions and 9 deletions

View File

@@ -3,6 +3,7 @@ package channelserver
import ( import (
"database/sql" "database/sql"
"errors" "errors"
"fmt"
cfg "erupe-ce/config" cfg "erupe-ce/config"
"erupe-ce/network/mhfpacket" "erupe-ce/network/mhfpacket"
@@ -46,12 +47,12 @@ func GetCharacterSaveData(s *Session, charID uint32) (*CharacterSaveData, error)
return saveData, nil return saveData, nil
} }
func (save *CharacterSaveData) Save(s *Session) { func (save *CharacterSaveData) Save(s *Session) error {
if save.decompSave == nil { if save.decompSave == nil {
s.logger.Warn("No decompressed save data, skipping save", s.logger.Warn("No decompressed save data, skipping save",
zap.Uint32("charID", save.CharID), zap.Uint32("charID", save.CharID),
) )
return return errors.New("no decompressed save data")
} }
if !s.kqfOverride { if !s.kqfOverride {
@@ -66,7 +67,7 @@ func (save *CharacterSaveData) Save(s *Session) {
err := save.Compress() err := save.Compress()
if err != nil { if err != nil {
s.logger.Error("Failed to compress savedata", zap.Error(err)) s.logger.Error("Failed to compress savedata", zap.Error(err))
return return fmt.Errorf("compress savedata: %w", err)
} }
} else { } else {
// Saves were not compressed // 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 { 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)) 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 { 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)) 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) { func handleMsgMhfSexChanger(s *Session, p mhfpacket.MHFPacket) {

View File

@@ -476,7 +476,9 @@ func TestCharacterSaveData_Save_Integration(t *testing.T) {
saveData.WeaponID = 1234 saveData.WeaponID = 1234
// Save it // Save it
saveData.Save(s) if err := saveData.Save(s); err != nil {
t.Fatalf("Save failed: %v", err)
}
// Reload and verify // Reload and verify
var hr, gr uint16 var hr, gr uint16

View File

@@ -91,7 +91,10 @@ func handleMsgMhfSavedata(s *Session, p mhfpacket.MHFPacket) {
} }
if characterSaveData.Name == s.Name || s.server.erupeConfig.RealClientMode <= cfg.S10 { 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.") s.logger.Info("Wrote recompressed savedata back to DB.")
} else { } else {
_ = s.rawConn.Close() _ = s.rawConn.Close()

View File

@@ -156,7 +156,9 @@ func handleMsgMhfAcquireDistItem(s *Session, p mhfpacket.MHFPacket) {
saveData, err := GetCharacterSaveData(s, s.charID) saveData, err := GetCharacterSaveData(s, s.charID)
if err == nil { if err == nil {
saveData.RP += uint16(item.Quantity) 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))
}
} }
} }
} }

View File

@@ -183,7 +183,9 @@ func handleDonateRP(s *Session, amount uint16, guild *Guild, _type int) []byte {
} }
} }
saveData.RP -= amount 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 { switch _type {
case 0: case 0:
if err := s.server.guildRepo.AddRankRP(guild.ID, amount); err != nil { if err := s.server.guildRepo.AddRankRP(guild.ID, amount); err != nil {

View File

@@ -194,7 +194,14 @@ func saveAllCharacterData(s *Session, rpToAdd int) error {
} }
// Save to database (main savedata + user_binary) // 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 // Save auxiliary data types
// Note: Plate data saves immediately when client sends save packets, // Note: Plate data saves immediately when client sends save packets,

View File

@@ -376,7 +376,9 @@ func handleMsgMhfPostTenrouirai(s *Session, p mhfpacket.MHFPacket) {
sd, err := GetCharacterSaveData(s, s.charID) sd, err := GetCharacterSaveData(s, s.charID)
if err == nil && sd != nil { if err == nil && sd != nil {
sd.RP -= pkt.DonatedRP 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) result, err := s.server.towerService.DonateGuildTowerRP(pkt.GuildID, pkt.DonatedRP)
if err != nil { if err != nil {
s.logger.Error("Failed to process tower RP donation", zap.Error(err)) s.logger.Error("Failed to process tower RP donation", zap.Error(err))