From 3415733cf07dcf787160f1f27f0b44a267d97fff Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 29 Dec 2025 07:29:50 +0000 Subject: [PATCH] make mysql error logging consistent Consistently propagate database errors to the client, making the client responsible for logging them. --- irc/channel.go | 3 ++ irc/client.go | 13 ++++++-- irc/mysql/history.go | 70 ++++++++++++++++++++++++++------------------ irc/server.go | 7 +++-- 4 files changed, 58 insertions(+), 35 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index ade4b5b4..7f8f12a0 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -730,6 +730,9 @@ func (channel *Channel) AddHistoryItem(item history.Item, account string) (err e status, target, _ := channel.historyStatus(channel.server.Config()) if status == HistoryPersistent { err = channel.server.historyDB.AddChannelItem(target, item, account) + if err != nil { + channel.server.logger.Error("history", "could not add channel message to history", err.Error()) + } } else if status == HistoryEphemeral { channel.history.Add(item) } diff --git a/irc/client.go b/irc/client.go index 42a6dc95..b5133b5a 100644 --- a/irc/client.go +++ b/irc/client.go @@ -1769,7 +1769,10 @@ func (client *Client) addHistoryItem(target *Client, item history.Item, details, } if cStatus == HistoryPersistent || tStatus == HistoryPersistent { targetedItem.CfCorrespondent = "" - client.server.historyDB.AddDirectMessage(details.nickCasefolded, details.account, tDetails.nickCasefolded, tDetails.account, targetedItem) + err = client.server.historyDB.AddDirectMessage(details.nickCasefolded, details.account, tDetails.nickCasefolded, tDetails.account, targetedItem) + if err != nil { + client.server.logger.Error("history", "could not add direct message to history", err.Error()) + } } return nil } @@ -1795,14 +1798,18 @@ func (client *Client) listTargets(start, end history.Selector, limit int) (resul } } persistentExtras, err := client.server.historyDB.ListChannels(chcfnames) - if err == nil && len(persistentExtras) != 0 { + if err != nil { + client.server.logger.Error("history", "could not list persistent channels", err.Error()) + } else if len(persistentExtras) != 0 { extras = append(extras, persistentExtras...) } _, cSeq, err := client.server.GetHistorySequence(nil, client, "") if err == nil && cSeq != nil { correspondents, err := cSeq.ListCorrespondents(start, end, limit) - if err == nil { + if err != nil { + client.server.logger.Error("history", "could not list correspondents", err.Error()) + } else { base = correspondents } } diff --git a/irc/mysql/history.go b/irc/mysql/history.go index b77d282b..53e5b7b8 100644 --- a/irc/mysql/history.go +++ b/irc/mysql/history.go @@ -629,40 +629,46 @@ func (mysql *MySQL) AddChannelItem(target string, item history.Item, account str func (mysql *MySQL) insertSequenceEntry(ctx context.Context, target string, messageTime int64, id int64) (err error) { _, err = mysql.insertSequence.ExecContext(ctx, target, messageTime, id) - mysql.logError("could not insert sequence entry", err) + if err != nil { + return fmt.Errorf("could not insert sequence entry: %w", err) + } return } func (mysql *MySQL) insertConversationEntry(ctx context.Context, target, correspondent string, messageTime int64, id int64) (err error) { _, err = mysql.insertConversation.ExecContext(ctx, target, correspondent, messageTime, id) - mysql.logError("could not insert conversations entry", err) + if err != nil { + return fmt.Errorf("could not insert conversations entry: %w", err) + } return } func (mysql *MySQL) insertCorrespondentsEntry(ctx context.Context, target, correspondent string, messageTime int64, historyId int64) (err error) { _, err = mysql.insertCorrespondent.ExecContext(ctx, target, correspondent, messageTime, messageTime) - mysql.logError("could not insert conversations entry", err) + if err != nil { + return fmt.Errorf("could not insert correspondents entry: %w", err) + } return } func (mysql *MySQL) insertBase(ctx context.Context, item history.Item) (id int64, err error) { value, err := marshalItem(&item) - if mysql.logError("could not marshal item", err) { - return + if err != nil { + return 0, fmt.Errorf("could not marshal item: %w", err) } msgidBytes, err := decodeMsgid(item.Message.Msgid) - if mysql.logError("could not decode msgid", err) { - return + if err != nil { + return 0, fmt.Errorf("could not decode msgid: %w", err) } result, err := mysql.insertHistory.ExecContext(ctx, value, msgidBytes) - if mysql.logError("could not insert item", err) { - return + if err != nil { + return 0, fmt.Errorf("could not insert item: %w", err) } id, err = result.LastInsertId() - if mysql.logError("could not insert item", err) { - return + if err != nil { + return 0, fmt.Errorf("could not insert item: %w", err) } return @@ -673,7 +679,9 @@ func (mysql *MySQL) insertAccountMessageEntry(ctx context.Context, id int64, acc return } _, err = mysql.insertAccountMessage.ExecContext(ctx, id, account) - mysql.logError("could not insert account-message entry", err) + if err != nil { + return fmt.Errorf("could not insert account-message entry: %w", err) + } return } @@ -754,7 +762,9 @@ func (mysql *MySQL) DeleteMsgid(msgid, accountName string) (err error) { } err = mysql.deleteHistoryIDs(ctx, []uint64{id}) - mysql.logError("couldn't delete msgid", err) + if err != nil { + return fmt.Errorf("couldn't delete msgid: %w", err) + } return } @@ -837,10 +847,10 @@ func (mysql *MySQL) lookupMsgid(ctx context.Context, msgid string, includeData b } else { err = row.Scan(&nanoSeq, &nanoConv, &id, &data) } - if err != sql.ErrNoRows { - mysql.logError("could not resolve msgid to time", err) - } if err != nil { + if err != sql.ErrNoRows { + err = fmt.Errorf("could not resolve msgid to time: %w", err) + } return } nanotime := extractNanotime(nanoSeq, nanoConv) @@ -863,8 +873,8 @@ func extractNanotime(seq, conv sql.NullInt64) (result int64) { func (mysql *MySQL) selectItems(ctx context.Context, query string, args ...interface{}) (results []history.Item, err error) { rows, err := mysql.db.QueryContext(ctx, query, args...) - if mysql.logError("could not select history items", err) { - return + if err != nil { + return nil, fmt.Errorf("could not select history items: %w", err) } defer rows.Close() @@ -873,12 +883,12 @@ func (mysql *MySQL) selectItems(ctx context.Context, query string, args ...inter var blob []byte var item history.Item err = rows.Scan(&blob) - if mysql.logError("could not scan history item", err) { - return + if err != nil { + return nil, fmt.Errorf("could not scan history item: %w", err) } err = unmarshalItem(blob, &item) - if mysql.logError("could not unmarshal history item", err) { - return + if err != nil { + return nil, fmt.Errorf("could not unmarshal history item: %w", err) } results = append(results, item) } @@ -955,7 +965,7 @@ func (mysql *MySQL) listCorrespondentsInternal(ctx context.Context, target strin rows, err := mysql.db.QueryContext(ctx, query, args...) if err != nil { - return + return nil, fmt.Errorf("could not query correspondents: %w", err) } defer rows.Close() var correspondent string @@ -963,7 +973,7 @@ func (mysql *MySQL) listCorrespondentsInternal(ctx context.Context, target strin for rows.Next() { err = rows.Scan(&correspondent, &nanotime) if err != nil { - return + return nil, fmt.Errorf("could not scan correspondents: %w", err) } results = append(results, history.TargetListing{ CfName: correspondent, @@ -1006,8 +1016,8 @@ func (mysql *MySQL) ListChannels(cfchannels []string) (results []history.TargetL queryBuf.WriteString(") GROUP BY sequence.target;") rows, err := mysql.db.QueryContext(ctx, queryBuf.String(), args...) - if mysql.logError("could not query channel listings", err) { - return + if err != nil { + return nil, fmt.Errorf("could not query channel listings: %w", err) } defer rows.Close() @@ -1015,8 +1025,8 @@ func (mysql *MySQL) ListChannels(cfchannels []string) (results []history.TargetL var nanotime int64 for rows.Next() { err = rows.Scan(&target, &nanotime) - if mysql.logError("could not scan channel listings", err) { - return + if err != nil { + return nil, fmt.Errorf("could not scan channel listings: %w", err) } results = append(results, history.TargetListing{ CfName: target, @@ -1088,7 +1098,9 @@ func (seq *mySQLHistorySequence) ListCorrespondents(start, end history.Selector, endTime := end.Time results, err = seq.mysql.listCorrespondentsInternal(ctx, seq.target, startTime, endTime, seq.cutoff, limit) - seq.mysql.logError("could not read correspondents", err) + if err != nil { + return nil, fmt.Errorf("could not read correspondents: %w", err) + } return } diff --git a/irc/server.go b/irc/server.go index 10942e1d..0e744eb7 100644 --- a/irc/server.go +++ b/irc/server.go @@ -154,7 +154,6 @@ func (server *Server) Shutdown() { sdnotify.Stopping() server.logger.Info("server", "Stopping server") - //TODO(dan): Make sure we disallow new nicks for _, client := range server.clients.AllClients() { client.Notice("Server is shutting down") } @@ -163,10 +162,12 @@ func (server *Server) Shutdown() { server.performAlwaysOnMaintenance(false, true) if err := server.store.Close(); err != nil { - server.logger.Error("shutdown", fmt.Sprintln("Could not close datastore:", err)) + server.logger.Error("shutdown", "Could not close datastore", err.Error()) } - server.historyDB.Close() + if err := server.historyDB.Close(); err != nil { + server.logger.Error("shutdown", "Could not close history database", err.Error()) + } server.logger.Info("server", fmt.Sprintf("%s exiting", Ver)) }