[Golang] deferでerrorを参照しようとしてハマった

概要

関数が返したerrorをdeferを使ってキャプチャしようとしたらハマったのでメモ。

起こったこと

関数でエラーが発生したらそれをログに残そうとしていたとき、以下のようなコードを書いてしまった。

func Execute() error {
	var err error
	defer func() { SendErrIfExists(err) }()

	err = DoSomething()
	if !errors.Is(err, ErrCanIgnore) {
		return err
	}

	return nil // ここに来たらSendErrIfExistsの引数はnilになって欲しい
}

このコードでDoSomethingがErrCanIgnoreを返すとエラーログにはしっかりとErrCanIgnoreが記録されてしまった。原因は以下の2点。

deferで実行される関数がerr変数を含むクロージャとなっている
クロージャが参照しているerr変数に入った値がErrCanIgnoreのまま

修正するとすれば以下のようになる。

errをnilで上書きする

func Execute() error {
	var err error
	defer func() { SendErrIfExists(err) }()

	err = DoSomething()
	if !errors.Is(err, ErrCanIgnore) {
		return err
	}

	err = nil
	return err
}

最終行の return err は return nil のままでも良いが、こちらのほうがerrの中身に意識がいくため意図に気づきやすいのではないだろうか。

別名の変数を使う

func Execute() error {
	var errLogged error
	defer func() { SendErrIfExists(errLogged) }()

	err := DoSomething()
	if !errors.Is(err, ErrCanIgnore) {
		errLogged = err
		return err
	}

	return nil
}

errとの混同は起きないが、単純に代入し忘れるのであんまりおすすめできない。

まとめ

deferで実行する関数は別に返り値を参照するわけではないのだが見落としがちなエラーだった。この手のロギングは定型句として扱いがちなのでレビューのときは疑ってかかるようにしたい。 deferでerr変数を参照する場合はreturnで必ずerr変数を返すようにしておくのが手堅いので、機械的に保護するならこれをチェックするような静的解析器があると良さそう。

2020-12-05