fix: log SJIS decoding errors instead of silently discarding them

Add SJISToUTF8Lossy() that wraps SJISToUTF8() and logs decode errors at
slog.Debug level. Replace all 31 call sites across 17 files that previously
discarded the error with `_, _ =`. This makes garbled text from malformed
SJIS client data debuggable without adding noise at default log levels.

cmd/protbot/scenario/chat.go

@@ -62,8 +62,8 @@ func ListenChat(ch *protocol.ChannelConn, cb ChatCallback) {
 		_ = pbf.ReadUint16() // flags
 		_ = pbf.ReadUint16() // senderNameLen
 		_ = pbf.ReadUint16() // messageLen
-		msg, _ := stringsupport.SJISToUTF8(pbf.ReadNullTerminatedBytes())
-		sender, _ := stringsupport.SJISToUTF8(pbf.ReadNullTerminatedBytes())
+		msg := stringsupport.SJISToUTF8Lossy(pbf.ReadNullTerminatedBytes())
+		sender := stringsupport.SJISToUTF8Lossy(pbf.ReadNullTerminatedBytes())
 
 		cb(ChatMessage{
 			ChatType:   chatType,

common/stringsupport/string_convert.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"log/slog"
 	"strconv"
 	"strings"
 
@@ -40,6 +41,16 @@ func SJISToUTF8(b []byte) (string, error) {
 	return string(result), nil
 }
 
+// SJISToUTF8Lossy decodes Shift-JIS bytes to a UTF-8 string, logging
+// any decoding error at debug level instead of returning it.
+func SJISToUTF8Lossy(b []byte) string {
+	s, err := SJISToUTF8(b)
+	if err != nil {
+		slog.Debug("SJIS decode failed", "error", err, "raw_len", len(b))
+	}
+	return s
+}
+
 // ToNGWord converts a UTF-8 string into a slice of uint16 values in the
 // Shift-JIS byte-swapped format used by the MHF NG-word (chat filter) system.
 func ToNGWord(x string) []uint16 {

common/stringsupport/string_convert_test.go

@@ -461,6 +461,25 @@ func BenchmarkCSVElems(b *testing.B) {
 	}
 }
 
+func TestSJISToUTF8Lossy(t *testing.T) {
+	// Valid SJIS (ASCII subset) decodes correctly.
+	got := SJISToUTF8Lossy([]byte("Hello"))
+	if got != "Hello" {
+		t.Errorf("SJISToUTF8Lossy(valid) = %q, want %q", got, "Hello")
+	}
+
+	// Truncated multi-byte SJIS sequence (lead byte 0x82 without trail byte)
+	// does not panic and returns some result (lossy).
+	got = SJISToUTF8Lossy([]byte{0x82})
+	_ = got // must not panic
+
+	// Nil input returns empty string.
+	got = SJISToUTF8Lossy(nil)
+	if got != "" {
+		t.Errorf("SJISToUTF8Lossy(nil) = %q, want %q", got, "")
+	}
+}
+
 func TestUTF8ToSJIS_UnsupportedCharacters(t *testing.T) {
 	// Regression test for PR #116: Characters outside the Shift-JIS range
 	// (e.g. Lenny face, cuneiform) previously caused a panic in UTF8ToSJIS,

docs/technical-debt.md

@@ -102,17 +102,7 @@ Suggested split: `repo_guild.go` (core CRUD), `repo_guild_members.go`, `repo_gui
 
 ~~**a) `fmt.Sprintf` inside structured logger calls (6 sites):**~~ **Fixed.** All 6 sites now use `zap.Uint32`/`zap.Uint8`/`zap.String` structured fields instead of `fmt.Sprintf`.
 
-**b) 20 silently discarded SJIS encoding errors in packet parsing:**
-
-The pattern `m.Field, _ = stringsupport.SJISToUTF8(...)` appears across:
-- `network/binpacket/msg_bin_chat.go:43-44`
-- `network/mhfpacket/msg_mhf_apply_bbs_article.go:33-35`
-- `network/mhfpacket/msg_mhf_send_mail.go:38-39`
-- `network/mhfpacket/msg_mhf_update_guild_message_board.go:41-42,50-51`
-- `server/channelserver/model_character.go:175`
-- And 7+ more packet files
-
-A malformed SJIS string from a client yields an empty string with no log output, making garbled text impossible to debug. The error return should at least be logged at debug level.
+~~**b) 20+ silently discarded SJIS encoding errors in packet parsing:**~~ **Fixed.** All call sites now use `SJISToUTF8Lossy()` which logs decode errors at `slog.Debug` level.
 
 ### ~~6. Inconsistent transaction API~~ (Fixed)
 
@@ -151,5 +141,5 @@ Based on impact and the momentum from recent repo-interface refactoring:
 4. **Split `repo_guild.go`** — last oversized file after the recent refactoring push
 5. ~~**Fix `fmt.Sprintf` in logger calls**~~ — **Done**
 6. ~~**Add `LoopDelay` Viper default**~~ — **Done**
-7. **Log SJIS decoding errors** — improves debuggability for text issues
+7. ~~**Log SJIS decoding errors**~~ — **Done**
 8. ~~**Standardize on `BeginTxx`**~~ — **Done**

network/binpacket/msg_bin_chat.go

@@ -40,8 +40,8 @@ func (m *MsgBinChat) Parse(bf *byteframe.ByteFrame) error {
 	m.Flags = bf.ReadUint16()
 	_ = bf.ReadUint16() // lenSenderName
 	_ = bf.ReadUint16() // lenMessage
-	m.Message, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
-	m.SenderName, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Message = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
+	m.SenderName = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

network/mhfpacket/msg_mhf_apply_bbs_article.go

@@ -30,9 +30,9 @@ func (m *MsgMhfApplyBbsArticle) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Cl
 	m.AckHandle = bf.ReadUint32()
 	m.Unk0 = bf.ReadUint32()
 	m.Unk1 = bf.ReadBytes(16)
-	m.Name, _ = stringsupport.SJISToUTF8(bfutil.UpToNull(bf.ReadBytes(32)))
-	m.Title, _ = stringsupport.SJISToUTF8(bfutil.UpToNull(bf.ReadBytes(128)))
-	m.Description, _ = stringsupport.SJISToUTF8(bfutil.UpToNull(bf.ReadBytes(256)))
+	m.Name = stringsupport.SJISToUTF8Lossy(bfutil.UpToNull(bf.ReadBytes(32)))
+	m.Title = stringsupport.SJISToUTF8Lossy(bfutil.UpToNull(bf.ReadBytes(128)))
+	m.Description = stringsupport.SJISToUTF8Lossy(bfutil.UpToNull(bf.ReadBytes(256)))
 	return nil
 }
 

network/mhfpacket/msg_mhf_create_guild.go

@@ -25,7 +25,7 @@ func (m *MsgMhfCreateGuild) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Client
 	m.AckHandle = bf.ReadUint32()
 	bf.ReadUint16() // Zeroed
 	bf.ReadUint16() // Name length
-	m.Name, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Name = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

network/mhfpacket/msg_mhf_create_joint.go

@@ -27,7 +27,7 @@ func (m *MsgMhfCreateJoint) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Client
 	m.GuildID = bf.ReadUint32()
 	bf.ReadUint16() // Zeroed
 	bf.ReadUint16() // Name length
-	m.Name, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Name = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

network/mhfpacket/msg_mhf_enumerate_house.go

@@ -30,7 +30,7 @@ func (m *MsgMhfEnumerateHouse) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Cli
 	bf.ReadUint16() // Zeroed
 	lenName := bf.ReadUint8()
 	if lenName > 0 {
-		m.Name, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+		m.Name = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	}
 	return nil
 }

network/mhfpacket/msg_mhf_load_house.go

@@ -32,7 +32,7 @@ func (m *MsgMhfLoadHouse) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientCo
 	m.CheckPass = bf.ReadBool()
 	bf.ReadUint16() // Zeroed
 	bf.ReadUint8()  // Password length
-	m.Password, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Password = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

network/mhfpacket/msg_mhf_operate_warehouse.go

@@ -32,7 +32,7 @@ func (m *MsgMhfOperateWarehouse) Parse(bf *byteframe.ByteFrame, ctx *clientctx.C
 	lenName := bf.ReadUint8()
 	bf.ReadUint16() // Zeroed
 	if lenName > 0 {
-		m.Name, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+		m.Name = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	}
 	return nil
 }

network/mhfpacket/msg_mhf_send_mail.go

@@ -35,8 +35,8 @@ func (m *MsgMhfSendMail) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientCon
 	bf.ReadUint16() // Zeroed
 	m.Quantity = bf.ReadUint16()
 	m.ItemID = bf.ReadUint16()
-	m.Subject, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
-	m.Body, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Subject = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
+	m.Body = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

network/mhfpacket/msg_mhf_update_guild_message_board.go

@@ -38,8 +38,8 @@ func (m *MsgMhfUpdateGuildMessageBoard) Parse(bf *byteframe.ByteFrame, ctx *clie
 		m.StampID = bf.ReadUint32()
 		m.TitleLength = bf.ReadUint32()
 		m.BodyLength = bf.ReadUint32()
-		m.Title, _ = stringsupport.SJISToUTF8(bf.ReadBytes(uint(m.TitleLength)))
-		m.Body, _ = stringsupport.SJISToUTF8(bf.ReadBytes(uint(m.BodyLength)))
+		m.Title = stringsupport.SJISToUTF8Lossy(bf.ReadBytes(uint(m.TitleLength)))
+		m.Body = stringsupport.SJISToUTF8Lossy(bf.ReadBytes(uint(m.BodyLength)))
 	case 1:
 		m.PostID = bf.ReadUint32()
 	case 2:
@@ -47,8 +47,8 @@ func (m *MsgMhfUpdateGuildMessageBoard) Parse(bf *byteframe.ByteFrame, ctx *clie
 		bf.ReadBytes(8)
 		m.TitleLength = bf.ReadUint32()
 		m.BodyLength = bf.ReadUint32()
-		m.Title, _ = stringsupport.SJISToUTF8(bf.ReadBytes(uint(m.TitleLength)))
-		m.Body, _ = stringsupport.SJISToUTF8(bf.ReadBytes(uint(m.BodyLength)))
+		m.Title = stringsupport.SJISToUTF8Lossy(bf.ReadBytes(uint(m.TitleLength)))
+		m.Body = stringsupport.SJISToUTF8Lossy(bf.ReadBytes(uint(m.BodyLength)))
 	case 3:
 		m.PostID = bf.ReadUint32()
 		bf.ReadBytes(8)

network/mhfpacket/msg_mhf_update_house.go

@@ -30,7 +30,7 @@ func (m *MsgMhfUpdateHouse) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Client
 	bf.ReadUint8() // Zeroed
 	bf.ReadUint8() // Zeroed
 	bf.ReadUint8() // Password length
-	m.Password, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	m.Password = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	return nil
 }
 

server/channelserver/handlers_data.go

@@ -194,7 +194,7 @@ func handleMsgMhfLoaddata(s *Session, p mhfpacket.MHFPacket) {
 	_, _ = bf.Seek(88, io.SeekStart)
 	name := bf.ReadNullTerminatedBytes()
 	s.server.userBinary.Set(s.charID, 1, append(name, []byte{0x00}...))
-	s.Name, _ = stringsupport.SJISToUTF8(name)
+	s.Name = stringsupport.SJISToUTF8Lossy(name)
 }
 
 func handleMsgMhfSaveScenarioData(s *Session, p mhfpacket.MHFPacket) {

server/channelserver/handlers_guild_info.go

@@ -287,14 +287,14 @@ func handleMsgMhfEnumerateGuild(s *Session, p mhfpacket.MHFPacket) {
 		if err == nil {
 			switch pkt.Type {
 			case mhfpacket.ENUMERATE_GUILD_TYPE_GUILD_NAME:
-				searchName, _ := stringsupport.SJISToUTF8(pkt.Data2.ReadNullTerminatedBytes())
+				searchName := stringsupport.SJISToUTF8Lossy(pkt.Data2.ReadNullTerminatedBytes())
 				for _, guild := range tempGuilds {
 					if strings.Contains(guild.Name, searchName) {
 						guilds = append(guilds, guild)
 					}
 				}
 			case mhfpacket.ENUMERATE_GUILD_TYPE_LEADER_NAME:
-				searchName, _ := stringsupport.SJISToUTF8(pkt.Data2.ReadNullTerminatedBytes())
+				searchName := stringsupport.SJISToUTF8Lossy(pkt.Data2.ReadNullTerminatedBytes())
 				for _, guild := range tempGuilds {
 					if strings.Contains(guild.LeaderName, searchName) {
 						guilds = append(guilds, guild)
@@ -364,14 +364,14 @@ func handleMsgMhfEnumerateGuild(s *Session, p mhfpacket.MHFPacket) {
 		tempAlliances, err = s.server.guildRepo.ListAlliances()
 		switch pkt.Type {
 		case mhfpacket.ENUMERATE_ALLIANCE_TYPE_ALLIANCE_NAME:
-			searchName, _ := stringsupport.SJISToUTF8(pkt.Data2.ReadNullTerminatedBytes())
+			searchName := stringsupport.SJISToUTF8Lossy(pkt.Data2.ReadNullTerminatedBytes())
 			for _, alliance := range tempAlliances {
 				if strings.Contains(alliance.Name, searchName) {
 					alliances = append(alliances, alliance)
 				}
 			}
 		case mhfpacket.ENUMERATE_ALLIANCE_TYPE_LEADER_NAME:
-			searchName, _ := stringsupport.SJISToUTF8(pkt.Data2.ReadNullTerminatedBytes())
+			searchName := stringsupport.SJISToUTF8Lossy(pkt.Data2.ReadNullTerminatedBytes())
 			for _, alliance := range tempAlliances {
 				if strings.Contains(alliance.ParentGuild.LeaderName, searchName) {
 					alliances = append(alliances, alliance)

server/channelserver/handlers_guild_ops.go

@@ -108,7 +108,7 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
 			doAckSimpleFail(s, pkt.AckHandle, make([]byte, 4))
 			return
 		}
-		guild.Comment, _ = stringsupport.SJISToUTF8(pkt.Data2.ReadNullTerminatedBytes())
+		guild.Comment = stringsupport.SJISToUTF8Lossy(pkt.Data2.ReadNullTerminatedBytes())
 		if err := s.server.guildRepo.Save(guild); err != nil {
 			s.logger.Error("Failed to save guild comment", zap.Error(err))
 		}
@@ -168,7 +168,7 @@ func handleMsgMhfOperateGuild(s *Session, p mhfpacket.MHFPacket) {
 }
 
 func handleRenamePugi(s *Session, bf *byteframe.ByteFrame, guild *Guild, num int) {
-	name, _ := stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	name := stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	switch num {
 	case 1:
 		guild.PugiName1 = name

server/channelserver/handlers_session.go

@@ -489,7 +489,7 @@ func handleMsgMhfTransitMessage(s *Session, p mhfpacket.MHFPacket) {
 		bf.ReadUint16() // term length
 		maxResults = bf.ReadUint16()
 		bf.ReadUint8() // Unk
-		term, _ = stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+		term = stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	case 3:
 		_ip := bf.ReadBytes(4)
 		ip = fmt.Sprintf("%d.%d.%d.%d", _ip[3], _ip[2], _ip[1], _ip[0])

server/channelserver/model_character.go

@@ -172,7 +172,7 @@ const (
 )
 
 func (save *CharacterSaveData) updateStructWithSaveData() {
-	save.Name, _ = stringsupport.SJISToUTF8(bfutil.UpToNull(save.decompSave[saveFieldNameOffset : saveFieldNameOffset+saveFieldNameLen]))
+	save.Name = stringsupport.SJISToUTF8Lossy(bfutil.UpToNull(save.decompSave[saveFieldNameOffset : saveFieldNameOffset+saveFieldNameLen]))
 	if save.decompSave[save.Pointers[pGender]] == 1 {
 		save.Gender = true
 	} else {

server/signserver/session.go

@@ -155,7 +155,7 @@ func (s *Session) handlePSSGN(bf *byteframe.ByteFrame) {
 
 func (s *Session) handlePSNLink(bf *byteframe.ByteFrame) {
 	_ = bf.ReadNullTerminatedBytes() // Client ID
-	credStr, _ := stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	credStr := stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	credentials := strings.Split(credStr, "\n")
 	tok := string(bf.ReadNullTerminatedBytes())
 	uid, resp := s.server.validateLogin(credentials[0], credentials[1])
@@ -195,8 +195,8 @@ func (s *Session) handlePSNLink(bf *byteframe.ByteFrame) {
 }
 
 func (s *Session) handleDSGN(bf *byteframe.ByteFrame) {
-	user, _ := stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
-	pass, _ := stringsupport.SJISToUTF8(bf.ReadNullTerminatedBytes())
+	user := stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
+	pass := stringsupport.SJISToUTF8Lossy(bf.ReadNullTerminatedBytes())
 	_ = string(bf.ReadNullTerminatedBytes()) // Unk
 	s.authenticate(user, pass)
 }