Goで無名関数を使ったら思わぬバグに遭遇した話

はじめに

エンジニアのmatsuokaです。スマートショッピングに入社してちょうど一年が経過しました。
普段は弊社の在庫管理プロダクトSmartMatCloud(以下SMC)のバックエンド開発を担当しています。
そんな中、最近goの無名関数でつまづいたので今回はそのことについて書いてみたいと思います。

前置き

SMCではFaxで注文書を送信することができるのですが、注文を行う際はお客様宛てにも注文書の内容をPDFとしてメールに添付した上で送信しています。
Fax発注には注文内容を白紙に印字して送信する他、販売元が予め用意しているフォーマットに商品番号や注文数といった情報を印字して送信を行う方法もあります。
SMCのFax発注サービスについては@yoneyamaさんが以前別の記事にて詳しく解説されているのでこちらも参照下さい。
SmartMatCloudにおけるAPI連携を利用したFax発注の紹介

バックエンド側で何が行われているか簡単に解説しますと、まず注文書となるPDFフォーマットにgopdfで必要な情報を印字します。
そのデータをbyte配列に変換し、外部サービスを利用してFax送信した後、注文書をPDFとしてメールに添付し、gomailでお客様宛てに送信しています。
例えばFEEDデンタル発注でFax送信を行う場合、注文書はこのような感じになります。

バグ発生

上記フォーマットからも分かるように、1枚で注文可能な商品の種類には限りがある為、1枚の最大行数を超える場合は2枚目に移行して複数枚のPDFがメールに添付されるのですが、何故か2枚以上の注文書が添付されたメールにはすべて最終ページの同じ内容のPDFが添付されていました。
Fax送信された内容に問題はなかったので、メール送信の処理を調べてみることにしました。
大分簡略化してはいますが、メールの作成から送信までの処理は以下のようになっていました。

func SendMailWithPdfAttached(pdfBytes [][]byte) {
    // gomail作成＆ヘッダ情報付与
    msg := gomail.NewMessage()
    msg.SetAddressHeader("From", "hoge@smartshopping.co.jp", "スマートショッピング")
    msg.SetHeader("To", "fuga@example.com")
    msg.SetHeader("Subject", "スマートショッピングFax発注")
    msg.SetBody("text/plain", "Fax注文されました。詳細は添付されたPDFをご確認下さい。")

    // byte型スライスからorder_${連番}.pdfという名前でPDFファイルを作成＆添付
    for i, v := range pdfBytes {
        msg.Attach(fmt.Sprintf("order_%d.pdf", i+1), gomail.SetCopyFunc(func(w io.Writer) error {
            _, err := w.Write(v)
            return err
        }))
    }

    d := gomail.NewDialer("smtp.gmail.com", 587, "username", "password")

    // メール送信
    if err := d.DialAndSend(msg); err != nil {
        panic(err)
    }
}

byte配列からPDFファイルを作成、添付する部分に注目します。
gomailにファイルを添付するにはAttachを使用します。
既に用意されているファイルを添付するだけであれば書き方は至ってシンプルです。

msg.Attach("order_1.pdf")

しかしながら、byte配列から動的にファイルを作成する場合はSetCopyFuncを使用する必要があります。

func SetCopyFunc(f func(io.Writer) error) FileSetting

ファイルをbyte配列としてio.Writerに書き込む関数を引き渡すことで、メール送信時にAttachで指定した名前でファイルを作成＆添付してくれます。

ファイル名は問題なく連番になっており、引き渡しているbyte配列にも異常はなかったので何が間違っているのか分からず、頭を悩ませていました。

何故このようなことが起こるのか

結論として、SetCopyFunc内の関数の引き渡し方に問題がありました。
関数に正しくforループでbyte配列が引き渡せていない且つ、関数はメールが送信されるときまで実行されない為、データを書き込むw.Write(v)のvがループする度に上書きされてしまいます。
その結果、最後のPDFが枚数分だけ量産されてしまったという訳です。
コンパイラによってはこのような関数外の変数への参照はエラーを出してくれるようなのですが、Goでは問題なく参照できてしまうようです。
これらを防ぐ為にはクロージャを使って次のように書き変える必要があります。

for i, v := range pdfBytes {
    msg.Attach(fmt.Sprintf("order_%d.pdf", i+1), gomail.SetCopyFunc(func(b []byte) func(w io.Writer) error {
        return func(w io.Writer) error {
            _, err := w.Write(b)
            return err
        }
    }(v)))
}

少し構造が把握しづらいですが、関数を切り出してみると文法的におかしかったことがわかります。

func NewSendMailWithPdfAttached(pdfBytes [][]byte) {
  // 〜省略〜

    // 引き渡す関数を外に切り出しただけでやってることは同じ
    for i, v := range pdfBytes {
        f := Success(v)
        msg.Attach(fmt.Sprintf("order_%d.pdf", i+1), gomail.SetCopyFunc(f))
    }

  // 〜省略〜
}

// 正しいクロージャを切り出したもの
func Success(b []byte) func(w io.Writer) error {
    return func(w io.Writer) error {
        _, err := w.Write(b)
        return err
    }
}

// 最初の誤った関数を切り出したもの
func Failed(w io.Writer) error {
    _, err := w.Write(b) // 文法エラー（書き込むbが参照できない）
    return err
}

終わりに

中には簡単に気付けた方もいらっしゃると思いますが、普段から何となく無名関数やクロージャを書いていた自分にとって今回の事例は良い学びとなりました。
一年通してGolangを書いてきたもののまだまだ知らないことは多く、日々アップデートされる新しい機能にも積極的にキャッチアップしていきたいところです。
今後も開発していく中で落とし穴にハマった経験などあればブログで発信していきたいと思います。
皆様にとって何かのお役に立てれば幸いです。