From 5c33640aea871737795f816e648f4000e9661245 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Thu, 16 Feb 2023 13:22:00 +0100 Subject: [PATCH] consistently use log.Check for logging errors that "should not happen", don't influence application flow sooner or later, someone will notice one of these messages, which will lead us to a bug. --- ctl.go | 34 +++++++++++++------------ dsn/dsn.go | 14 ++++++++--- export.go | 7 +++++- http/account.go | 34 ++++++++++++++++--------- http/admin.go | 13 ++++++---- http/import.go | 44 ++++++++++++++++---------------- http/mtasts.go | 2 +- imapclient/parse.go | 2 +- imapserver/server.go | 31 +++++++++++++---------- import.go | 39 ++++++++++++++--------------- junk.go | 58 +++++++++++++++++++++++++++++++++++-------- junk/filter.go | 39 ++++++++++++++++++----------- junk/parse.go | 5 +++- main.go | 16 +++++++++--- mox-/admin.go | 30 +++++++++++----------- mox-/config.go | 10 ++++++-- moxio/syncdir.go | 7 +++--- mtastsdb/db.go | 3 ++- queue/dsn.go | 21 +++++++++------- queue/queue.go | 18 ++++++++------ serve.go | 22 ++++++++-------- smtpserver/analyze.go | 5 ++-- smtpserver/dsn.go | 13 +++++----- smtpserver/server.go | 57 +++++++++++++++++++++++------------------- store/account.go | 13 +++++----- store/export.go | 35 +++++++++++++++----------- store/import.go | 24 ++++++++++-------- store/tmp.go | 3 ++- store/train.go | 10 ++++++-- tlsrptdb/db.go | 3 ++- 30 files changed, 366 insertions(+), 246 deletions(-) diff --git a/ctl.go b/ctl.go index 50e509d..d29b53e 100644 --- a/ctl.go +++ b/ctl.go @@ -371,10 +371,10 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu ctl.xcheck(err, "creating temporary message file") defer func() { if msgFile != nil { - if err := os.Remove(msgFile.Name()); err != nil { - log.Errorx("removing temporary message file", err, mlog.Field("path", msgFile.Name())) - } - msgFile.Close() + err := os.Remove(msgFile.Name()) + log.Check(err, "removing temporary message file", mlog.Field("path", msgFile.Name())) + err = msgFile.Close() + log.Check(err, "closing temporary message file") } }() mw := &message.Writer{Writer: msgFile} @@ -400,7 +400,8 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu log.Info("message delivered through ctl", mlog.Field("to", to)) }) - msgFile.Close() + err = msgFile.Close() + log.Check(err, "closing delivered message file") msgFile = nil err = a.Close() ctl.xcheck(err, "closing account") @@ -421,7 +422,8 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu ctl.xcheck(err, "open account") defer func() { if acc != nil { - acc.Close() + err := acc.Close() + log.Check(err, "closing account after setting password") } }() @@ -501,7 +503,10 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu } mr, err := queue.OpenMessage(id) ctl.xcheck(err, "opening message") - defer mr.Close() + defer func() { + err := mr.Close() + log.Check(err, "closing message from queue") + }() ctl.xwriteok() ctl.xstreamfrom(mr) @@ -652,12 +657,10 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu basePath := mox.DataDirPath("accounts") dbPath := filepath.Join(basePath, acc.Name, "junkfilter.db") bloomPath := filepath.Join(basePath, acc.Name, "junkfilter.bloom") - if err := os.Remove(dbPath); err != nil { - log.Errorx("removing old junkfilter database file", err, mlog.Field("path", dbPath)) - } - if err := os.Remove(bloomPath); err != nil { - log.Errorx("removing old junkfilter bloom filter file", err, mlog.Field("path", bloomPath)) - } + err := os.Remove(dbPath) + log.Check(err, "removing old junkfilter database file", mlog.Field("path", dbPath)) + err = os.Remove(bloomPath) + log.Check(err, "removing old junkfilter bloom filter file", mlog.Field("path", bloomPath)) // Open junk filter, this creates new files. jf, _, err := acc.OpenJunkFilter(ctl.log) @@ -666,9 +669,8 @@ func servectlcmd(ctx context.Context, log *mlog.Log, ctl *ctl, xcmd *string, shu if jf == nil { return } - if err := jf.Close(); err != nil { - log.Errorx("closing junk filter during cleanup", err) - } + err := jf.Close() + log.Check(err, "closing junk filter during cleanup") }() // Read through messages with junk or nonjunk flag set, and train them. diff --git a/dsn/dsn.go b/dsn/dsn.go index 8d6bc5d..d7d0f47 100644 --- a/dsn/dsn.go +++ b/dsn/dsn.go @@ -151,7 +151,7 @@ func (m *Message) Compose(log *mlog.Log, smtputf8 bool) ([]byte, error) { } line := func(w io.Writer) { - w.Write([]byte("\r\n")) + _, _ = w.Write([]byte("\r\n")) } // Outer message headers. @@ -179,7 +179,9 @@ func (m *Message) Compose(log *mlog.Log, smtputf8 bool) ([]byte, error) { if err != nil { return nil, err } - msgp.Write([]byte(strings.ReplaceAll(m.TextBody, "\n", "\r\n"))) + if _, err := msgp.Write([]byte(strings.ReplaceAll(m.TextBody, "\n", "\r\n"))); err != nil { + return nil, err + } // Machine-parsable message. ../rfc/3464:455 statusHdr := textproto.MIMEHeader{} @@ -329,10 +331,14 @@ func (m *Message) Compose(log *mlog.Log, smtputf8 bool) ([]byte, error) { n = 78 } line, data = data[:n], data[n:] - origp.Write([]byte(line + "\r\n")) + if _, err := origp.Write([]byte(line + "\r\n")); err != nil { + return nil, err + } } } else { - origp.Write(headers) + if _, err := origp.Write(headers); err != nil { + return nil, err + } } } diff --git a/export.go b/export.go index d0f196e..978797c 100644 --- a/export.go +++ b/export.go @@ -1,6 +1,7 @@ package main import ( + "log" "path/filepath" "time" @@ -57,7 +58,11 @@ func xcmdExport(mbox bool, args []string, c *cmd) { dbpath := filepath.Join(accountDir, "index.db") db, err := bstore.Open(dbpath, &bstore.Options{Timeout: 5 * time.Second, Perm: 0660}, store.Message{}, store.Recipient{}, store.Mailbox{}) xcheckf(err, "open database %q", dbpath) - defer db.Close() + defer func() { + if err := db.Close(); err != nil { + log.Printf("closing db after export: %v", err) + } + }() a := store.DirArchiver{Dir: dst} err = store.ExportMessages(mlog.New("export"), db, accountDir, a, !mbox, mailbox) diff --git a/http/account.go b/http/account.go index ad96158..5437b50 100644 --- a/http/account.go +++ b/http/account.go @@ -94,7 +94,8 @@ func checkAccountAuth(ctx context.Context, log *mlog.Log, w http.ResponseWriter, } else { authResult = "ok" accName := acc.Name - acc.Close() + err := acc.Close() + log.Check(err, "closing account") return accName } // note: browsers don't display the realm to prevent users getting confused by malicious realm messages. @@ -183,9 +184,9 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { f, err := os.Open("http/account.html") if err == nil { defer f.Close() - io.Copy(w, f) + _, _ = io.Copy(w, f) } else { - w.Write(accountHTML) + _, _ = w.Write(accountHTML) } case "/mail-export-maildir.tgz", "/mail-export-maildir.zip", "/mail-export-mbox.tgz", "/mail-export-mbox.zip": @@ -198,7 +199,10 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { http.Error(w, "500 - internal server error", http.StatusInternalServerError) return } - defer acc.Close() + defer func() { + err := acc.Close() + log.Check(err, "closing account") + }() var archiver store.Archiver if tgz { @@ -207,7 +211,7 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { gzw := gzip.NewWriter(w) defer func() { - gzw.Close() + _ = gzw.Close() }() archiver = store.TarArchiver{Writer: tar.NewWriter(gzw)} } else { @@ -215,9 +219,8 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { archiver = store.ZipArchiver{Writer: zip.NewWriter(w)} } defer func() { - if err := archiver.Close(); err != nil { - log.Errorx("exporting mail close", err) - } + err := archiver.Close() + log.Check(err, "exporting mail close") }() if err := store.ExportMessages(log, acc.DB, acc.Dir, archiver, maildir, ""); err != nil { log.Errorx("exporting mail", err) @@ -238,7 +241,10 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { } return } - defer f.Close() + defer func() { + err := f.Close() + log.Check(err, "closing form file") + }() skipMailboxPrefix := r.FormValue("skipMailboxPrefix") tmpf, err := os.CreateTemp("", "mox-import") if err != nil { @@ -247,7 +253,8 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { } defer func() { if tmpf != nil { - tmpf.Close() + err := tmpf.Close() + log.Check(err, "closing uploaded file") } }() if err := os.Remove(tmpf.Name()); err != nil { @@ -269,7 +276,7 @@ func accountHandle(w http.ResponseWriter, r *http.Request) { tmpf = nil // importStart is now responsible for closing. w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]string{"ImportToken": token}) + _ = json.NewEncoder(w).Encode(map[string]string{"ImportToken": token}) default: if strings.HasPrefix(r.URL.Path, "/api/") { @@ -294,7 +301,10 @@ func (Account) SetPassword(ctx context.Context, password string) { accountName := ctx.Value(authCtxKey).(string) acc, err := store.OpenAccount(accountName) xcheckf(ctx, err, "open account") - defer acc.Close() + defer func() { + err := acc.Close() + xlog.Check(err, "closing account") + }() err = acc.SetPassword(password) xcheckf(ctx, err, "setting password") } diff --git a/http/admin.go b/http/admin.go index 9e3a67f..0d9f12b 100644 --- a/http/admin.go +++ b/http/admin.go @@ -189,9 +189,9 @@ func adminHandle(w http.ResponseWriter, r *http.Request) { f, err := os.Open("http/admin.html") if err == nil { defer f.Close() - io.Copy(w, f) + _, _ = io.Copy(w, f) } else { - w.Write(adminHTML) + _, _ = w.Write(adminHTML) } return } @@ -549,8 +549,8 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, dialer *net.Dialer, end := time.Now().Add(10 * time.Second) cctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - conn.SetReadDeadline(end) - conn.SetWriteDeadline(end) + err = conn.SetDeadline(end) + xlog.WithContext(ctx).Check(err, "setting deadline") br := bufio.NewReader(conn) _, err = br.ReadString('\n') @@ -1447,7 +1447,10 @@ func (Admin) SetPassword(ctx context.Context, accountName, password string) { } acc, err := store.OpenAccount(accountName) xcheckf(ctx, err, "open account") - defer acc.Close() + defer func() { + err := acc.Close() + xlog.Check(err, "closing account") + }() err = acc.SetPassword(password) xcheckf(ctx, err, "setting password") } diff --git a/http/import.go b/http/import.go index b57e449..0de4b88 100644 --- a/http/import.go +++ b/http/import.go @@ -198,7 +198,8 @@ type importAborted struct{} func importStart(log *mlog.Log, accName string, f *os.File, skipMailboxPrefix string) (string, error) { defer func() { if f != nil { - f.Close() + err := f.Close() + log.Check(err, "closing uploaded file") } }() @@ -254,7 +255,8 @@ func importStart(log *mlog.Log, accName string, f *os.File, skipMailboxPrefix st tx, err := acc.DB.Begin(true) if err != nil { acc.Unlock() - acc.Close() + xerr := acc.Close() + log.Check(xerr, "closing account") return "", fmt.Errorf("start transaction: %v", err) } @@ -313,25 +315,22 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store } defer func() { - if err := f.Close(); err != nil { - log.Errorx("closing uploaded messages file", err) - } + err := f.Close() + log.Check(err, "closing uploaded messages file") + for _, id := range deliveredIDs { p := acc.MessagePath(id) - if err := os.Remove(p); err != nil { - log.Errorx("closing message file after import error", err, mlog.Field("path", p)) - } + err := os.Remove(p) + log.Check(err, "closing message file after import error", mlog.Field("path", p)) } if tx != nil { - if err := tx.Rollback(); err != nil { - log.Errorx("rolling back transaction", err) - } + err := tx.Rollback() + log.Check(err, "rolling back transaction") } if acc != nil { acc.Unlock() - if err := acc.Close(); err != nil { - log.Errorx("closing account", err) - } + err := acc.Close() + log.Check(err, "closing account") } x := recover() @@ -395,7 +394,10 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store problemf("opening message again for training junk filter: %v (continuing)", err) return } - defer f.Close() + defer func() { + err := f.Close() + log.Check(err, "closing file after training junkfilter") + }() p, err := m.LoadPart(f) if err != nil { problemf("loading parsed message again for training junk filter: %v (continuing)", err) @@ -714,7 +716,8 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store continue } importFile(f.Name, zf) - zf.Close() + err = zf.Close() + log.Check(err, "closing file from zip") } } else { for { @@ -749,8 +752,7 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store deliveredIDs = nil if jf != nil { - err := jf.Close() - if err != nil { + if err := jf.Close(); err != nil { problemf("saving changes of training junk filter: %v (continuing)", err) log.Errorx("saving changes of training junk filter", err) } @@ -761,10 +763,8 @@ func importMessages(ctx context.Context, log *mlog.Log, token string, acc *store defer comm.Unregister() comm.Broadcast(changes) acc.Unlock() - if err := acc.Close(); err != nil { - log.Errorx("closing account after import", err) - // Continue - } + err = acc.Close() + log.Check(err, "closing account after import") acc = nil sendEvent("done", importDone{}) diff --git a/http/mtasts.go b/http/mtasts.go index 740b66e..3c40c94 100644 --- a/http/mtasts.go +++ b/http/mtasts.go @@ -60,5 +60,5 @@ func mtastsPolicyHandle(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Content-Type", "text/plain") w.Header().Set("Cache-Control", "no-cache, max-age=0") - w.Write([]byte(policy.String())) + _, _ = w.Write([]byte(policy.String())) } diff --git a/imapclient/parse.go b/imapclient/parse.go index fea71c8..7bdb30c 100644 --- a/imapclient/parse.go +++ b/imapclient/parse.go @@ -72,7 +72,7 @@ func (c *Conn) peek(exp byte) bool { func (c *Conn) take(exp byte) bool { if c.peek(exp) { - c.readbyte() + _, _ = c.readbyte() return true } return false diff --git a/imapserver/server.go b/imapserver/server.go index 2c91433..2582f0b 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -401,9 +401,8 @@ func (c *conn) Write(buf []byte) (int, error) { var n int for len(buf) > 0 { - if err := c.conn.SetWriteDeadline(time.Now().Add(30 * time.Second)); err != nil { - c.log.Errorx("setting write deadline", err) - } + err := c.conn.SetWriteDeadline(time.Now().Add(30 * time.Second)) + c.log.Check(err, "setting write deadline") nn, err := c.conn.Write(buf[:chunk]) if err != nil { @@ -442,7 +441,8 @@ func (c *conn) readline0() (string, error) { if c.state == stateNotAuthenticated { d = 30 * time.Second } - c.conn.SetReadDeadline(time.Now().Add(d)) + err := c.conn.SetReadDeadline(time.Now().Add(d)) + c.log.Check(err, "setting read deadline") line, err := bufpool.Readline(c.br) if err != nil && errors.Is(err, moxio.ErrLineTooLong) { @@ -477,7 +477,8 @@ func (c *conn) readline(readCmd bool) string { } if err != nil { if readCmd && errors.Is(err, os.ErrDeadlineExceeded) { - c.conn.SetWriteDeadline(time.Now().Add(10 * time.Second)) + err := c.conn.SetWriteDeadline(time.Now().Add(10 * time.Second)) + c.log.Check(err, "setting write deadline") c.writelinef("* BYE inactive") } if !errors.Is(err, errIO) && !errors.Is(err, errProtocol) { @@ -496,7 +497,8 @@ func (c *conn) readline(readCmd bool) string { if c.state == stateNotAuthenticated { wd = 30 * time.Second } - c.conn.SetWriteDeadline(time.Now().Add(wd)) + err = c.conn.SetWriteDeadline(time.Now().Add(wd)) + c.log.Check(err, "setting write deadline") return line } @@ -740,7 +742,8 @@ func (c *conn) command() { c.log.Info("imap syntax error", mlog.Field("lastline", c.lastLine)) fatal := strings.HasSuffix(c.lastLine, "+}") if fatal { - c.conn.SetWriteDeadline(time.Now().Add(5 * time.Second)) + err := c.conn.SetWriteDeadline(time.Now().Add(5 * time.Second)) + c.log.Check(err, "setting write deadline") } c.bwriteresultf("%s BAD %s unrecognized syntax/command: %v", tag, cmd, err) if fatal { @@ -1145,7 +1148,8 @@ func (c *conn) applyChanges(changes []store.Change, initial bool) { return } - c.conn.SetWriteDeadline(time.Now().Add(5 * time.Minute)) + err := c.conn.SetWriteDeadline(time.Now().Add(5 * time.Minute)) + c.log.Check(err, "setting write deadline") c.log.Debug("applying changes", mlog.Field("changes", changes)) @@ -1970,9 +1974,8 @@ func (c *conn) cmdDelete(tag, cmd string, p *parser) { for _, m := range remove { p := c.account.MessagePath(m.ID) - if err := os.Remove(p); err != nil { - c.log.Infox("removing message file for mailbox delete", err, mlog.Field("path", p)) - } + err := os.Remove(p) + c.log.Check(err, "removing message file for mailbox delete", mlog.Field("path", p)) } c.ok(tag, cmd) @@ -2534,7 +2537,8 @@ func (c *conn) cmdAppend(tag, cmd string, p *parser) { c.broadcast([]store.Change{store.ChangeAddUID{MailboxID: mb.ID, UID: msg.UID, Flags: msg.Flags}}) }) - msgFile.Close() + err = msgFile.Close() + c.log.Check(err, "closing appended file") msgFile = nil if c.mailboxID == mb.ID { @@ -2581,7 +2585,8 @@ wait: // Reset the write deadline. In case of little activity, with a command timeout of // 30 minutes, we have likely passed it. - c.conn.SetWriteDeadline(time.Now().Add(5 * time.Minute)) + err := c.conn.SetWriteDeadline(time.Now().Add(5 * time.Minute)) + c.log.Check(err, "setting write deadline") if strings.ToUpper(line) != "DONE" { // We just close the connection because our protocols are out of sync. diff --git a/import.go b/import.go index 31c8b35..260550a 100644 --- a/import.go +++ b/import.go @@ -135,19 +135,16 @@ func importctl(ctl *ctl, mbox bool) { defer func() { if mboxf != nil { - if err := mboxf.Close(); err != nil { - ctl.log.Infox("closing mbox file after import", err) - } + err := mboxf.Close() + ctl.log.Check(err, "closing mbox file after import") } if mdnewf != nil { - if err := mdnewf.Close(); err != nil { - ctl.log.Infox("closing maildir new after import", err) - } + err := mdnewf.Close() + ctl.log.Check(err, "closing maildir new after import") } if mdcurf != nil { - if err := mdcurf.Close(); err != nil { - ctl.log.Infox("closing maildir cur after import", err) - } + err := mdcurf.Close() + ctl.log.Check(err, "closing maildir cur after import") } }() @@ -157,9 +154,8 @@ func importctl(ctl *ctl, mbox bool) { ctl.xcheck(err, "opening account") defer func() { if a != nil { - if err := a.Close(); err != nil { - ctl.log.Errorx("closing account after import", err) - } + err := a.Close() + ctl.log.Check(err, "closing account after import") } }() @@ -185,7 +181,8 @@ func importctl(ctl *ctl, mbox bool) { ctl.xcheck(err, "begin transaction") defer func() { if tx != nil { - tx.Rollback() + err := tx.Rollback() + ctl.log.Check(err, "rolling back transaction") } }() @@ -208,9 +205,8 @@ func importctl(ctl *ctl, mbox bool) { for _, id := range deliveredIDs { p := a.MessagePath(id) - if err := os.Remove(p); err != nil { - ctl.log.Errorx("closing message file after import error", err, mlog.Field("path", p)) - } + err := os.Remove(p) + ctl.log.Check(err, "closing message file after import error", mlog.Field("path", p)) } ctl.xerror(fmt.Sprintf("%v", x)) @@ -256,10 +252,10 @@ func importctl(ctl *ctl, mbox bool) { if msgf == nil { return } - if err := os.Remove(msgf.Name()); err != nil { - ctl.log.Errorx("removing temporary message after failing to import", err) - } - msgf.Close() + err := os.Remove(msgf.Name()) + ctl.log.Check(err, "removing temporary message after failing to import") + err = msgf.Close() + ctl.log.Check(err, "closing temporary message after failing to import") }() // Parse message and store parsed information for later fast retrieval. @@ -295,7 +291,8 @@ func importctl(ctl *ctl, mbox bool) { m.MailboxID = mb.ID m.MailboxOrigID = mb.ID xdeliver(m, msgf) - msgf.Close() + err = msgf.Close() + ctl.log.Check(err, "closing message after delivery") msgf = nil n++ diff --git a/junk.go b/junk.go index 05d2bc0..c8274b7 100644 --- a/junk.go +++ b/junk.go @@ -48,7 +48,11 @@ func (a junkArgs) Memprofile() { f, err := os.Create(a.memprofile) xcheckf(err, "creating memory profile") - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing memory profile: %v", err) + } + }() runtime.GC() // get up-to-date statistics err = pprof.WriteHeapProfile(f) xcheckf(err, "writing memory profile") @@ -67,7 +71,9 @@ func (a junkArgs) Profile() func() { xcheckf(err, "start CPU profile") return func() { pprof.StopCPUProfile() - f.Close() + if err := f.Close(); err != nil { + log.Printf("closing cpu profile: %v", err) + } a.Memprofile() } } @@ -129,7 +135,11 @@ func cmdJunkTrain(c *cmd) { a.SetLogLevel() f := must(junk.NewFilter(mlog.New("junktrain"), a.params, a.databasePath, a.bloomfilterPath)) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing junk filter: %v", err) + } + }() hamFiles := listDir(args[0]) spamFiles := listDir(args[1]) @@ -155,7 +165,11 @@ func cmdJunkCheck(c *cmd) { a.SetLogLevel() f := must(junk.OpenFilter(mlog.New("junkcheck"), a.params, a.databasePath, a.bloomfilterPath, false)) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing junk filter: %v", err) + } + }() prob, _, _, _, err := f.ClassifyMessagePath(args[0]) xcheckf(err, "testing mail") @@ -176,7 +190,11 @@ func cmdJunkTest(c *cmd) { a.SetLogLevel() f := must(junk.OpenFilter(mlog.New("junktest"), a.params, a.databasePath, a.bloomfilterPath, false)) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing junk filter: %v", err) + } + }() testDir := func(dir string, ham bool) (int, int) { ok, bad := 0, 0 @@ -229,7 +247,11 @@ messages are shuffled, with optional random seed.` a.SetLogLevel() f := must(junk.NewFilter(mlog.New("junkanalyze"), a.params, a.databasePath, a.bloomfilterPath)) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing junk filter: %v", err) + } + }() hamDir := args[0] spamDir := args[1] @@ -317,7 +339,11 @@ func cmdJunkPlay(c *cmd) { a.SetLogLevel() f := must(junk.NewFilter(mlog.New("junkplay"), a.params, a.databasePath, a.bloomfilterPath)) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Printf("closing junk filter: %v", err) + } + }() // We'll go through all emails to find their dates. type msg struct { @@ -339,15 +365,21 @@ func cmdJunkPlay(c *cmd) { p, err := message.EnsurePart(mf, fi.Size()) if err != nil { nbad++ - mf.Close() + if err := mf.Close(); err != nil { + log.Printf("closing message file: %v", err) + } continue } if p.Envelope.Date.IsZero() { nnodate++ - mf.Close() + if err := mf.Close(); err != nil { + log.Printf("closing message file: %v", err) + } continue } - mf.Close() + if err := mf.Close(); err != nil { + log.Printf("closing message file: %v", err) + } msgs = append(msgs, msg{dir, name, ham, sent, p.Envelope.Date}) if sent { nsent++ @@ -403,7 +435,11 @@ func cmdJunkPlay(c *cmd) { } else { mf, err := os.Open(path) xcheckf(err, "open %q", path) - defer mf.Close() + defer func() { + if err := mf.Close(); err != nil { + log.Printf("closing message file: %v", err) + } + }() fi, err := mf.Stat() xcheckf(err, "stat %q", path) p, err := message.EnsurePart(mf, fi.Size()) diff --git a/junk/filter.go b/junk/filter.go index e19f7e1..359548e 100644 --- a/junk/filter.go +++ b/junk/filter.go @@ -145,7 +145,8 @@ func OpenFilter(log *mlog.Log, params Params, dbPath, bloomPath string, loadBloo return err }) if err != nil { - f.Close() + cerr := f.Close() + log.Check(cerr, "closing filter after error") return nil, fmt.Errorf("looking up ham/spam message count: %s", err) } return f, nil @@ -172,16 +173,21 @@ func NewFilter(log *mlog.Log, params Params, dbPath, bloomPath string) (*Filter, return nil, fmt.Errorf("creating bloom file: %w", err) } if err := bf.Truncate(4 * 1024 * 1024); err != nil { - bf.Close() - os.Remove(bloomPath) + xerr := bf.Close() + log.Check(xerr, "closing bloom filter file after truncate error") + xerr = os.Remove(bloomPath) + log.Check(xerr, "removing bloom filter file after truncate error") return nil, fmt.Errorf("making empty bloom filter: %s", err) } - bf.Close() + err = bf.Close() + log.Check(err, "closing bloomfilter file") - db, err := newDB(dbPath) + db, err := newDB(log, dbPath) if err != nil { - os.Remove(bloomPath) - os.Remove(dbPath) + xerr := os.Remove(bloomPath) + log.Check(xerr, "removing bloom filter file after db init error") + xerr = os.Remove(dbPath) + log.Check(xerr, "removing database file after db init error") return nil, fmt.Errorf("open database: %s", err) } @@ -210,17 +216,14 @@ func openBloom(path string) (*Bloom, error) { return NewBloom(buf, bloomK) } -func newDB(path string) (db *bstore.DB, rerr error) { +func newDB(log *mlog.Log, path string) (db *bstore.DB, rerr error) { // Remove any existing files. os.Remove(path) defer func() { if rerr != nil { - if db != nil { - db.Close() - } - db = nil - os.Remove(path) + err := os.Remove(path) + log.Check(err, "removing db file after init error") } }() @@ -271,7 +274,10 @@ func (f *Filter) Save() error { if err := f.db.HintAppend(true, wordscore{}); err != nil { f.log.Errorx("hint appendonly", err) } else { - defer f.db.HintAppend(false, wordscore{}) + defer func() { + err := f.db.HintAppend(false, wordscore{}) + f.log.Check(err, "restoring append hint") + }() } } err := f.db.Write(func(tx *bstore.Tx) error { @@ -480,7 +486,10 @@ func (f *Filter) ClassifyMessagePath(path string) (probability float64, words ma if err != nil { return 0, nil, 0, 0, err } - defer mf.Close() + defer func() { + err := mf.Close() + f.log.Check(err, "closing file after classify") + }() fi, err := mf.Stat() if err != nil { return 0, nil, 0, 0, err diff --git a/junk/parse.go b/junk/parse.go index 521508a..f21631a 100644 --- a/junk/parse.go +++ b/junk/parse.go @@ -23,7 +23,10 @@ func (f *Filter) tokenizeMail(path string) (bool, map[string]struct{}, error) { if err != nil { return false, nil, err } - defer mf.Close() + defer func() { + err := mf.Close() + f.log.Check(err, "closing message file") + }() fi, err := mf.Stat() if err != nil { return false, nil, err diff --git a/main.go b/main.go index 8b269d8..b663e9a 100644 --- a/main.go +++ b/main.go @@ -1689,7 +1689,11 @@ func cmdEnsureParsed(c *cmd) { mustLoadConfig() a, err := store.OpenAccount(args[0]) xcheckf(err, "open account") - defer a.Close() + defer func() { + if err := a.Close(); err != nil { + log.Printf("closing account: %v", err) + } + }() n := 0 err = a.DB.Write(func(tx *bstore.Tx) error { @@ -1734,7 +1738,11 @@ func cmdBumpUIDValidity(c *cmd) { mustLoadConfig() a, err := store.OpenAccount(args[0]) xcheckf(err, "open account") - defer a.Close() + defer func() { + if err := a.Close(); err != nil { + log.Printf("closing account: %v", err) + } + }() var uidvalidity uint32 err = a.DB.Write(func(tx *bstore.Tx) error { @@ -1951,7 +1959,9 @@ binary should be setgid that group: xcheckf(err, "creating temp file for storing message after failed delivery") defer func() { if f != nil { - os.Remove(f.Name()) + if err := os.Remove(f.Name()); err != nil { + log.Printf("removing temp file after failure storing failed delivery: %v", err) + } } }() _, err = f.Write([]byte(msg)) diff --git a/mox-/admin.go b/mox-/admin.go index 6ec1af4..b8de874 100644 --- a/mox-/admin.go +++ b/mox-/admin.go @@ -148,9 +148,8 @@ func MakeDomainConfig(ctx context.Context, domain, hostname dns.Domain, accountN var paths []string defer func() { for _, p := range paths { - if err := os.Remove(p); err != nil { - log.Errorx("removing path for domain config", err, mlog.Field("path", p)) - } + err := os.Remove(p) + log.Check(err, "removing path for domain config", mlog.Field("path", p)) } }() @@ -163,8 +162,10 @@ func MakeDomainConfig(ctx context.Context, domain, hostname dns.Domain, accountN } defer func() { if f != nil { - os.Remove(path) - f.Close() + err := os.Remove(path) + log.Check(err, "removing file after error") + err = f.Close() + log.Check(err, "closing file after error") } }() if _, err := f.Write(data); err != nil { @@ -298,9 +299,8 @@ func DomainAdd(ctx context.Context, domain dns.Domain, accountName string, local } defer func() { for _, f := range cleanupFiles { - if err := os.Remove(f); err != nil { - log.Errorx("cleaning up file after error", err, mlog.Field("path", f)) - } + err := os.Remove(f) + log.Check(err, "cleaning up file after error", mlog.Field("path", f)) } }() @@ -316,7 +316,7 @@ func DomainAdd(ctx context.Context, domain dns.Domain, accountName string, local nc.Domains[domain.Name()] = confDomain - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("domain added", mlog.Field("domain", domain)) @@ -355,7 +355,7 @@ func DomainRemove(ctx context.Context, domain dns.Domain) (rerr error) { } } - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } @@ -548,7 +548,7 @@ func AccountAdd(ctx context.Context, account, address string) (rerr error) { } nc.Accounts[account] = MakeAccountConfig(addr) - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("account added", mlog.Field("account", account), mlog.Field("address", addr)) @@ -582,7 +582,7 @@ func AccountRemove(ctx context.Context, account string) (rerr error) { } } - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("account removed", mlog.Field("account", account)) @@ -642,7 +642,7 @@ func AddressAdd(ctx context.Context, address, account string) (rerr error) { a.Destinations = nd nc.Accounts[account] = a - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("address added", mlog.Field("address", addr), mlog.Field("account", account)) @@ -699,7 +699,7 @@ func AddressRemove(ctx context.Context, address string) (rerr error) { } nc.Accounts[ad.Account] = na - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("address removed", mlog.Field("address", addr), mlog.Field("account", ad.Account)) @@ -744,7 +744,7 @@ func DestinationSave(ctx context.Context, account, destName string, newDest conf nacc.Destinations = nd nc.Accounts[account] = nacc - if err := writeDynamic(ctx, nc); err != nil { + if err := writeDynamic(ctx, log, nc); err != nil { return fmt.Errorf("writing domains.conf: %v", err) } log.Info("destination saved", mlog.Field("account", account), mlog.Field("destname", destName)) diff --git a/mox-/config.go b/mox-/config.go index e7c8ba4..1f15d9c 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -232,7 +232,7 @@ func (c *Config) allowACMEHosts() { // todo future: write config parsing & writing code that can read a config and remembers the exact tokens including newlines and comments, and can write back a modified file. the goal is to be able to write a config file automatically (after changing fields through the ui), but not loose comments and whitespace, to still get useful diffs for storing the config in a version control system. // must be called with lock held. -func writeDynamic(ctx context.Context, c config.Dynamic) error { +func writeDynamic(ctx context.Context, log *mlog.Log, c config.Dynamic) error { accDests, errs := prepareDynamicConfig(ctx, ConfigDynamicPath, Conf.Static, &c) if len(errs) > 0 { return errs[0] @@ -249,7 +249,8 @@ func writeDynamic(ctx context.Context, c config.Dynamic) error { } defer func() { if f != nil { - f.Close() + err := f.Close() + log.Check(err, "closing file after error") } }() buf := b.Bytes() @@ -271,6 +272,11 @@ func writeDynamic(ctx context.Context, c config.Dynamic) error { return fmt.Errorf("stat after writing domains.conf: %v", err) } + if err := f.Close(); err != nil { + return fmt.Errorf("close written domains.conf: %v", err) + } + f = nil + Conf.dynamicMtime = fi.ModTime() Conf.DynamicLastCheck = time.Now() Conf.Dynamic = c diff --git a/moxio/syncdir.go b/moxio/syncdir.go index a490d6d..33075e2 100644 --- a/moxio/syncdir.go +++ b/moxio/syncdir.go @@ -11,7 +11,8 @@ func SyncDir(dir string) error { if err != nil { return fmt.Errorf("open directory: %v", err) } - xerr := d.Sync() - d.Close() - return xerr + err = d.Sync() + xerr := d.Close() + xlog.Check(xerr, "closing directory after sync") + return err } diff --git a/mtastsdb/db.go b/mtastsdb/db.go index fc211dc..bc4575b 100644 --- a/mtastsdb/db.go +++ b/mtastsdb/db.go @@ -99,7 +99,8 @@ func Close() { mutex.Lock() defer mutex.Unlock() if mtastsDB != nil { - mtastsDB.Close() + err := mtastsDB.Close() + xlog.Check(err, "closing database") mtastsDB = nil } } diff --git a/queue/dsn.go b/queue/dsn.go index 4f6430e..7bd5215 100644 --- a/queue/dsn.go +++ b/queue/dsn.go @@ -71,7 +71,10 @@ func queueDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg stri return } msgr := store.FileMsgReader(m.MsgPrefix, msgf) - defer msgr.Close() + defer func() { + err := msgr.Close() + log.Check(err, "closing message reader after queuing dsn") + }() headers, err := message.ReadHeaders(bufio.NewReader(msgr)) if err != nil { qlog("reading headers of queued message", err) @@ -140,9 +143,8 @@ func queueDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg stri mailbox = mox.Conf.Static.Postmaster.Mailbox } defer func() { - if err := acc.Close(); err != nil { - log.Errorx("queue dsn: closing account", err, mlog.Field("sender", m.Sender().XString(m.SMTPUTF8)), mlog.Field("kind", kind)) - } + err := acc.Close() + log.Check(err, "queue dsn: closing account", mlog.Field("sender", m.Sender().XString(m.SMTPUTF8)), mlog.Field("kind", kind)) }() msgFile, err := store.CreateMessageTemp("queue-dsn") @@ -152,10 +154,10 @@ func queueDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg stri } defer func() { if msgFile != nil { - if err := os.Remove(msgFile.Name()); err != nil { - log.Errorx("removing message file", err, mlog.Field("path", msgFile.Name())) - } - msgFile.Close() + err := os.Remove(msgFile.Name()) + log.Check(err, "removing message file", mlog.Field("path", msgFile.Name())) + err = msgFile.Close() + log.Check(err, "closing message file") } }() @@ -176,6 +178,7 @@ func queueDSN(log *mlog.Log, m Msg, remoteMTA dsn.NameIP, secodeOpt, errmsg stri return } }) - msgFile.Close() + err = msgFile.Close() + log.Check(err, "closing dsn file") msgFile = nil } diff --git a/queue/queue.go b/queue/queue.go index 61e0a1a..ca429a1 100644 --- a/queue/queue.go +++ b/queue/queue.go @@ -131,9 +131,8 @@ func Init() error { // Shutdown closes the queue database. The delivery process isn't stopped. For tests only. func Shutdown() { - if err := queueDB.Close(); err != nil { - xlog.Errorx("closing queue db", err) - } + err := queueDB.Close() + xlog.Check(err, "closing queue db") queueDB = nil } @@ -202,9 +201,8 @@ func Add(log *mlog.Log, senderAccount string, mailFrom, rcptTo smtp.Path, has8bi dst := mox.DataDirPath(filepath.Join("queue", store.MessagePath(qm.ID))) defer func() { if dst != "" { - if err := os.Remove(dst); err != nil { - log.Infox("removing destination message file for queue", err, mlog.Field("path", dst)) - } + err := os.Remove(dst) + log.Check(err, "removing destination message file for queue", mlog.Field("path", dst)) } }() dstDir := filepath.Dir(dst) @@ -243,7 +241,8 @@ func writeFile(dst string, r io.Reader) error { } defer func() { if df != nil { - df.Close() + err := df.Close() + xlog.Check(err, "closing file after failed write") } }() if _, err := io.Copy(df, r); err != nil { @@ -713,7 +712,10 @@ func deliverHost(log *mlog.Log, resolver dns.Resolver, cid int64, host dns.IPDom return false, false, "", nil, fmt.Sprintf("open message file: %s", err), false } msgr := store.FileMsgReader(m.MsgPrefix, f) - defer msgr.Close() + defer func() { + err := msgr.Close() + log.Check(err, "closing message after delivery attempt") + }() cidctx := context.WithValue(mox.Context, mlog.CidKey, cid) ctx, cancel := context.WithTimeout(cidctx, 30*time.Second) diff --git a/serve.go b/serve.go index ad79b21..b73a3bb 100644 --- a/serve.go +++ b/serve.go @@ -152,9 +152,8 @@ requested, other TLS certificates are requested on demand. if _, err := fmt.Fprint(f, "ok\n"); err != nil { log.Infox("writing ok to restart ctl socket", err) } - if err := f.Close(); err != nil { - log.Errorx("closing restart ctl socket", err) - } + err = f.Close() + log.Check(err, "closing restart ctl socket") } log.Print("starting up", mlog.Field("version", moxvar.Version)) @@ -186,9 +185,8 @@ requested, other TLS certificates are requested on demand. log.Print("shutting down with pending sockets") } } - if err := os.Remove(mox.DataDirPath("ctl")); err != nil { - log.Errorx("removing ctl unix domain socket during shutdown", err) - } + err := os.Remove(mox.DataDirPath("ctl")) + log.Check(err, "removing ctl unix domain socket during shutdown") } if err := moxio.CheckUmask(); err != nil { @@ -246,7 +244,10 @@ requested, other TLS certificates are requested on demand. log.Infox("open account for postmaster changelog delivery", err) return next } - defer a.Close() + defer func() { + err := a.Close() + log.Check(err, "closing account") + }() f, err := store.CreateMessageTemp("changelog") if err != nil { log.Infox("making temporary message file for changelog delivery", err) @@ -261,9 +262,8 @@ requested, other TLS certificates are requested on demand. m.Size = int64(n) if err := a.DeliverMailbox(log, mox.Conf.Static.Postmaster.Mailbox, m, f, true); err != nil { log.Infox("changelog delivery", err) - if err := os.Remove(f.Name()); err != nil { - log.Infox("removing temporary changelog message after delivery failure", err) - } + err := os.Remove(f.Name()) + log.Check(err, "removing temporary changelog message after delivery failure") } log.Info("delivered changelog", mlog.Field("current", current), mlog.Field("lastknown", lastknown), mlog.Field("latest", latest)) if err := mox.StoreLastKnown(latest); err != nil { @@ -313,7 +313,7 @@ requested, other TLS certificates are requested on demand. go monitorDNSBL(log) ctlpath := mox.DataDirPath("ctl") - os.Remove(ctlpath) + _ = os.Remove(ctlpath) ctl, err := net.Listen("unix", ctlpath) if err != nil { log.Fatalx("listen on ctl unix domain socket", err) diff --git a/smtpserver/analyze.go b/smtpserver/analyze.go index b6d7cbd..5f5a79d 100644 --- a/smtpserver/analyze.go +++ b/smtpserver/analyze.go @@ -243,9 +243,8 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive f, jf, err := d.acc.OpenJunkFilter(log) if err == nil { defer func() { - if err := f.Close(); err != nil { - log.Errorx("closing junkfilter", err) - } + err := f.Close() + log.Check(err, "closing junkfilter") }() contentProb, _, _, _, err := f.ClassifyMessageReader(store.FileMsgReader(d.m.MsgPrefix, d.dataFile), d.m.Size) if err != nil { diff --git a/smtpserver/dsn.go b/smtpserver/dsn.go index b8cdc60..050065f 100644 --- a/smtpserver/dsn.go +++ b/smtpserver/dsn.go @@ -30,10 +30,10 @@ func queueDSN(c *conn, rcptTo smtp.Path, m dsn.Message) error { } defer func() { if f != nil { - if err := os.Remove(f.Name()); err != nil { - c.log.Errorx("removing temporary dsn message file", err) - } - f.Close() + err := os.Remove(f.Name()) + c.log.Check(err, "removing temporary dsn message file") + err = f.Close() + c.log.Check(err, "closing temporary dsn message file") } }() if _, err := f.Write([]byte(buf)); err != nil { @@ -48,9 +48,8 @@ func queueDSN(c *conn, rcptTo smtp.Path, m dsn.Message) error { if err := queue.Add(c.log, "", smtp.Path{}, rcptTo, has8bit, smtputf8, int64(len(buf)), nil, f, bufUTF8, true); err != nil { return err } - if err := f.Close(); err != nil { - c.log.Errorx("closing dsn file", err) - } + err = f.Close() + c.log.Check(err, "closing dsn file") f = nil return nil } diff --git a/smtpserver/server.go b/smtpserver/server.go index daafb09..9a89272 100644 --- a/smtpserver/server.go +++ b/smtpserver/server.go @@ -309,7 +309,8 @@ func (c *conn) reset() { c.hello = dns.IPDomain{} c.username = "" if c.account != nil { - c.account.Close() + err := c.account.Close() + c.log.Check(err, "closing account") } c.account = nil c.rset() @@ -544,12 +545,11 @@ func serve(listenerName string, cid int64, hostname dns.Domain, tlsConfig *tls.C defer func() { c.origConn.Close() // Close actual TCP socket, regardless of TLS on top. - c.conn.Close() // Will try to write alert notification to already closed socket, returning error quickly. + c.conn.Close() // If TLS, will try to write alert notification to already closed socket, returning error quickly. if c.account != nil { - if err := c.account.Close(); err != nil { - c.log.Infox("smtp: account close", err) - } + err := c.account.Close() + c.log.Check(err, "closing account") c.account = nil } @@ -997,9 +997,7 @@ func (c *conn) cmdAuth(p *parser) { defer func() { if acc != nil { err := acc.Close() - if err != nil { - c.log.Errorx("closing account", err) - } + c.log.Check(err, "closing account") } }() var ipadhash, opadhash hash.Hash @@ -1069,9 +1067,7 @@ func (c *conn) cmdAuth(p *parser) { defer func() { if acc != nil { err := acc.Close() - if err != nil { - c.log.Errorx("closing account", err) - } + c.log.Check(err, "closing account") } }() if ss.Authorization != "" && ss.Authorization != ss.Authentication { @@ -1430,10 +1426,10 @@ func (c *conn) cmdData(p *parser) { } defer func() { if dataFile != nil { - if err := os.Remove(dataFile.Name()); err != nil { - c.log.Infox("removing temporary message file", err, mlog.Field("path", dataFile.Name())) - } - dataFile.Close() + err := os.Remove(dataFile.Name()) + c.log.Check(err, "removing temporary message file", mlog.Field("path", dataFile.Name())) + err = dataFile.Close() + c.log.Check(err, "removing temporary message file") } }() msgWriter := &message.Writer{Writer: dataFile} @@ -1673,7 +1669,8 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr metricSubmission.WithLabelValues("ok").Inc() c.log.Info("message queued for delivery", mlog.Field("mailfrom", *c.mailFrom), mlog.Field("rcptto", rcptAcc.rcptTo), mlog.Field("smtputf8", c.smtputf8), mlog.Field("msgsize", msgSize)) } - dataFile.Close() + err = dataFile.Close() + c.log.Check(err, "closing file after submission") *pdataFile = nil c.transactionGood++ @@ -1725,7 +1722,11 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW var dkimErr error go func() { defer func() { - recover() // Should not happen, but don't take program down if it does. + x := recover() // Should not happen, but don't take program down if it does. + if x != nil { + c.log.Error("dkim verify panic", mlog.Field("err", x)) + debug.PrintStack() + } }() defer wg.Done() // We always evaluate all signatures. We want to build up reputation for each @@ -1756,7 +1757,11 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW wg.Add(1) go func() { defer func() { - recover() // Should not happen, but don't take program down if it does. + x := recover() // Should not happen, but don't take program down if it does. + if x != nil { + c.log.Error("dkim verify panic", mlog.Field("err", x)) + debug.PrintStack() + } }() defer wg.Done() spfctx, spfcancel := context.WithTimeout(ctx, time.Minute) @@ -2015,9 +2020,8 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW } defer func() { if acc != nil { - if err := acc.Close(); err != nil { - log.Errorx("closing account after delivery", err) - } + err := acc.Close() + log.Check(err, "closing account after delivery") } }() @@ -2238,9 +2242,8 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW } }) - if err := acc.Close(); err != nil { - log.Infox("closing account after delivering", err) - } + err = acc.Close() + log.Check(err, "closing account after delivering") acc = nil } @@ -2325,8 +2328,10 @@ func (c *conn) deliver(ctx context.Context, recvHdrFor func(string) string, msgW } } - os.Remove(dataFile.Name()) - dataFile.Close() + err = os.Remove(dataFile.Name()) + c.log.Check(err, "removing file after delivery") + err = dataFile.Close() + c.log.Check(err, "closing data file after delivery") *pdataFile = nil c.transactionGood++ diff --git a/store/account.go b/store/account.go index dbac5c4..588f16d 100644 --- a/store/account.go +++ b/store/account.go @@ -1055,9 +1055,8 @@ func (a *Account) TidyRejectsMailbox(log *mlog.Log, rejectsMailbox string) (hasS defer func() { for _, m := range remove { p := a.MessagePath(m.ID) - if err := os.Remove(p); err != nil { - log.Errorx("removing rejects message file", err, mlog.Field("path", p)) - } + err := os.Remove(p) + log.Check(err, "removing rejects message file", mlog.Field("path", p)) } }() @@ -1153,9 +1152,8 @@ func (a *Account) RejectsRemove(log *mlog.Log, rejectsMailbox, messageID string) defer func() { for _, m := range remove { p := a.MessagePath(m.ID) - if err := os.Remove(p); err != nil { - log.Errorx("removing rejects message file", err, mlog.Field("path", p)) - } + err := os.Remove(p) + log.Check(err, "removing rejects message file", mlog.Field("path", p)) } }() @@ -1219,7 +1217,8 @@ func OpenEmailAuth(email string, password string) (acc *Account, rerr error) { defer func() { if rerr != nil && acc != nil { - acc.Close() + err := acc.Close() + xlog.Check(err, "closing account after open auth failure") acc = nil } }() diff --git a/store/export.go b/store/export.go index 384e80b..0220dd1 100644 --- a/store/export.go +++ b/store/export.go @@ -114,7 +114,8 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar } defer func() { if tx != nil { - tx.Rollback() + err := tx.Rollback() + log.Check(err, "transaction rollback after export error") } }() @@ -207,7 +208,8 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar var mboxwriter *bufio.Writer defer func() { if mboxtmp != nil { - mboxtmp.Close() + err := mboxtmp.Close() + log.Check(err, "closing mbox temp file") } }() @@ -245,7 +247,8 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar return fmt.Errorf("adding dovecot-keywords: %v", err) } if _, err := w.Write(b.Bytes()); err != nil { - w.Close() + xerr := w.Close() + log.Check(xerr, "closing dovecot-keywords file after closing") return fmt.Errorf("writing dovecot-keywords: %v", err) } maildirFlags = map[string]int{} @@ -272,16 +275,15 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar return fmt.Errorf("add mbox to archive: %v", err) } if _, err := io.Copy(w, mboxtmp); err != nil { - w.Close() + xerr := w.Close() + log.Check(xerr, "closing mbox message file after error") return fmt.Errorf("copying temp mbox file to archive: %v", err) } if err := w.Close(); err != nil { return fmt.Errorf("closing message file: %v", err) } - if err := mboxtmp.Close(); err != nil { - log.Errorx("closing temporary mbox file", err) - // Continue, not fatal. - } + err = mboxtmp.Close() + log.Check(err, "closing temporary mbox file") mboxwriter = nil mboxtmp = nil return nil @@ -293,13 +295,16 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar if m.Size == int64(len(m.MsgPrefix)) { mr = io.NopCloser(bytes.NewReader(m.MsgPrefix)) } else { - mpf, err := os.Open(mp) + mf, err := os.Open(mp) if err != nil { errors += fmt.Sprintf("open message file for id %d, path %s: %v (message skipped)\n", m.ID, mp, err) return nil } - defer mpf.Close() - st, err := mpf.Stat() + defer func() { + err := mf.Close() + log.Check(err, "closing message file after export") + }() + st, err := mf.Stat() if err != nil { errors += fmt.Sprintf("stat message file for id %d, path %s: %v (message skipped)\n", m.ID, mp, err) return nil @@ -308,7 +313,7 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar if size != m.Size { errors += fmt.Sprintf("message size mismatch for message id %d, database has %d, size is %d+%d=%d, using calculated size\n", m.ID, m.Size, len(m.MsgPrefix), st.Size(), size) } - mr = FileMsgReader(m.MsgPrefix, mpf) + mr = FileMsgReader(m.MsgPrefix, mf) } if maildir { @@ -386,7 +391,8 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar return fmt.Errorf("adding message to archive: %v", err) } if _, err := io.Copy(w, &dst); err != nil { - w.Close() + xerr := w.Close() + log.Check(xerr, "closing message") return fmt.Errorf("copying message to archive: %v", err) } return w.Close() @@ -479,8 +485,9 @@ func ExportMessages(log *mlog.Log, db *bstore.DB, accountDir string, archiver Ar return err } if _, err := w.Write([]byte(errors)); err != nil { - w.Close() log.Errorx("writing errors.txt to archive", err) + xerr := w.Close() + log.Check(xerr, "closing errors.txt after error") return err } if err := w.Close(); err != nil { diff --git a/store/import.go b/store/import.go index 1102719..3fbb0e1 100644 --- a/store/import.go +++ b/store/import.go @@ -82,10 +82,10 @@ func (mr *MboxReader) Next() (*Message, *os.File, string, error) { } defer func() { if f != nil { - f.Close() - if err := os.Remove(f.Name()); err != nil { - mr.log.Errorx("removing temporary message file after mbox read error", err, mlog.Field("path", f.Name())) - } + err := os.Remove(f.Name()) + mr.log.Check(err, "removing temporary message file after mbox read error", mlog.Field("path", f.Name())) + err = f.Close() + mr.log.Check(err, "closing temporary message file after mbox read error") } }() @@ -209,7 +209,8 @@ func NewMaildirReader(createTemp func(pattern string) (*os.File, error), newf, c if err == nil { mr.dovecotKeywords, err = ParseDovecotKeywords(kf, log) log.Check(err, "parsing dovecot keywords file") - kf.Close() + err = kf.Close() + log.Check(err, "closing dovecot-keywords file") } return mr @@ -242,17 +243,20 @@ func (mr *MaildirReader) Next() (*Message, *os.File, string, error) { if err != nil { return nil, nil, p, fmt.Errorf("open message in maildir: %s", err) } - defer sf.Close() + defer func() { + err := sf.Close() + mr.log.Check(err, "closing message file after error") + }() f, err := mr.createTemp("maildirreader") if err != nil { return nil, nil, p, err } defer func() { if f != nil { - f.Close() - if err := os.Remove(f.Name()); err != nil { - mr.log.Errorx("removing temporary message file after maildir read error", err, mlog.Field("path", f.Name())) - } + err := os.Remove(f.Name()) + mr.log.Check(err, "removing temporary message file after maildir read error", mlog.Field("path", f.Name())) + err = f.Close() + mr.log.Check(err, "closing temporary message file after maildir read error") } }() diff --git a/store/tmp.go b/store/tmp.go index 8baa84b..89a8288 100644 --- a/store/tmp.go +++ b/store/tmp.go @@ -19,7 +19,8 @@ func CreateMessageTemp(pattern string) (*os.File, error) { } err = f.Chmod(0660) if err != nil { - f.Close() + xerr := f.Close() + xlog.Check(xerr, "closing temp message file after chmod error") return nil, err } return f, err diff --git a/store/train.go b/store/train.go index 0e0a3b0..218c276 100644 --- a/store/train.go +++ b/store/train.go @@ -95,7 +95,10 @@ func (a *Account) RetrainMessage(log *mlog.Log, tx *bstore.Tx, jf *junk.Filter, log.Debug("updating junk filter", mlog.Field("untrain", untrain), mlog.Field("untrainJunk", untrainJunk), mlog.Field("train", train), mlog.Field("trainJunk", trainJunk)) mr := a.MessageReader(*m) - defer mr.Close() + defer func() { + err := mr.Close() + log.Check(err, "closing message reader after retraining") + }() p, err := m.LoadPart(mr) if err != nil { @@ -137,7 +140,10 @@ func (a *Account) TrainMessage(log *mlog.Log, jf *junk.Filter, m Message) (bool, } mr := a.MessageReader(m) - defer mr.Close() + defer func() { + err := mr.Close() + log.Check(err, "closing message after training") + }() p, err := m.LoadPart(mr) if err != nil { diff --git a/tlsrptdb/db.go b/tlsrptdb/db.go index a90e8eb..b281d9a 100644 --- a/tlsrptdb/db.go +++ b/tlsrptdb/db.go @@ -87,7 +87,8 @@ func Close() { mutex.Lock() defer mutex.Unlock() if tlsrptDB != nil { - tlsrptDB.Close() + err := tlsrptDB.Close() + xlog.Check(err, "closing database") tlsrptDB = nil } }