podhmo's diary

どちらか一方に統一されていた場合には困らないが、混在している環境での動作のメモ。特に、トップレベルでerrors.Cause()を使っている処理が存在していると危険かもしれないという話。

実験

テキトーに以下のようなコードを書いてみる。直接エラーを返すもの、pkg/errorsでwrapするもの、Errorfでwrapするものについて調べる感じのコード。

package main

import (
    "fmt"
    "log"

    "github.com/pkg/errors"
)

var ErrFoo = fmt.Errorf("FOO")

func main() {
    if err := run(); err != nil {
        log.Fatalf("!! %+v", err)
    }
}

func run() error {
    fmt.Println("foo is Foo?", errors.Is(foo(), ErrFoo))                       // true
    fmt.Println("withWrap-foo is Foo?", errors.Is(withWrap(), ErrFoo))         // true
    fmt.Println("withWrap-foo cause Foo?", errors.Cause(withWrap()) == ErrFoo) // true
    fmt.Println("errorf-foo is Foo?", errors.Is(withErrorf(), ErrFoo))         // true
    fmt.Println("errorf-foo cause Foo?", errors.Cause(withErrorf()) == ErrFoo) // false
    return nil
}

func foo() error {
    return ErrFoo
}

func withWrap() error {
    if err := foo(); err != nil {
        return errors.Wrap(err, "with pkg.errors.Wrap")
    }
    return nil
}

func withErrorf() error {
    if err := foo(); err != nil {
        return fmt.Errorf("with errorf: %w", err)
    }
    return nil
}

実行結果は以下。Causeの方はfalseになる。

foo is Foo? true
withWrap-foo is Foo? true
withWrap-foo cause Foo? true
errorf-foo is Foo? true
errorf-foo cause Foo? false

原因

fmt.Errorf()でwrapされた関数は Cause() error を実装していないので。

READEMEにも書かれている通りeerrors.Cause()は以下のようなインターフェイスでwrapされていることを前提にしている。

Using errors.Wrap constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to reverse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by errors.Cause.

type causer interface {
        Cause() error
}

というわけでmainに近いところで以下のようなコードがあると危険かも。

switch err := errors.Cause(err).(type) {
case *MyError:
        // handle specifically
default:
        // unknown error
}

代わりに errors.Is() や errors.As() を使うと良い。

ちなみに、asにgo vetを効かせたかったら真面目にlinterを調整するか、errors.As() を使うように心がけると良い。

goのerrors.As()でpanicしちゃう話 - podhmo's diary

gist

何かのファクトリーを統一的に扱いたいみたいなことを考えたときにそれへの対応を考えることがある。 goの場合はそれぞれの状況で自分の手でコードを書いてつなげなくてはいけない。ファクトリーに限らないがこの種のバリエーション自体に言及しているのはwireやgo-aws-sdkのlambdaの部分なんかがそう。

それぞれのものが全部同じというわけではないのだけれど、今回は func(...)(object, func(), error) のような関数についてだけ考える(利用例)。

errorを含んだ場合

まず、errorを含んだ場合について考えてみる。例えば、DBというstructのようなものがあったとする¹。これを返すOpen()のようなファクトリー関数があることにする。そしてerrorを持たない場合は以下のような感じで使われる。

db := Open(...)
return Use(db, ...)

これがerrorを返すように変更されると、errorハンドリングが必要になる。

db, err := Open(...)
if err != nil {
    return err // Useの戻り値がerrorの場合
}
return Use(db, ...)

単体で考えるとこれだけ。

cleanupを含んだ場合

同様にcleanupを含んだ場合は以下のようになる。特にDBがDBオブジェクトとしてではなく、セッションオブジェクトのようなものになったり、トランザクション（をラッピングしたもの）として扱いたくなった場合にこのような変更が起きる。

db, cleanup := Open(...)
defer cleanup()
return Use(db, ...)

errorとcleanupを同時に返すような場合

さて、ここが本題。errorとcleanupの両方に対応するときにどういう形のコードで組み合わせるべきかで少し悩んだ²。

最初の方法はerrorを先にハンドリングする方法。

db, cleanup, err := Open(...)
if err != nil {
    return err
}
defer cleanup()
return Use(db, ...)

完全に同じではないが、net/http.Responseなどを扱っているときのClose()の扱いがこの形。エラーの時にはClose()の呼び出しを気にしなくて良い。

ここでの悩みどころは、取り扱い方を統一しようとしたときに本当にcleanupを呼ばずに済ませて良いのだろうか？ということ。

次の方法はcleanupを先にハンドリングする方法。

db, cleanup, err := Open(...)
defer cleanup()
if err != nil {
    return err
}
return Use(db, ...)

先のdeferを呼ぶことにすれば、必ずcleanupが呼ばれることが保証される。とはいえ、こちらは逆に本当に常にcleanupが呼ばれるのが正しいのだろうか？という疑問がすぐに出る。いや、もっと単純に、戻り値がnilだった時にすぐにpanicするのは危険では？という気持ちになってくる。

あるいはケースバイケースで都度呼び分けてというのは最悪で、消費者に常に負担を強いることになる³。

ちょっと改良してnilチェックを加える。これが実は良い形と個人的には思っている。

db, cleanup, err := Open(...)
if cleanup != nil {
    defer cleanup()
}    
if err != nil {
    return err
}
return Use(db, ...)

deferは関数スコープ⁴なのでifを書いても困らない。エラーに関しても提供者側が気を遣うことでユーザー側が気にしなくても意思表明ができる。具体的には以下のような形。

たとえエラーであってもcleanupを呼んでほしいファクトリー関数においてはcleanupをしっかり返してあげれば良い。

return object, cleanup, err

逆に、エラーの時にcleanupをスキップしてほしいファクトリー関数においてはnilを返してあげれば良い。

return object, nil, err

この場合はcleanupがスキップされる。io.CloserのClose()もこのスタイルで書いてあげれば暫定的には対応できる⁵。

rollback?

ところで、もう少し考えることがある。このような場合はどうだろう。

トランザクションを作る
処理が成功したらトランザクションをcommitしたい
処理が失敗したらトランザクションをrollbackしたい

直接手書きするとこういうことになる。

isSuccess := true
defer func(){
    if isSuccess {
        commit()
        return
    }
    rollback()
}()
if err := Use(db, ...); err != nil {
    isSuccess = false
    return err
}
return nil

このハンドリングもいい感じに扱うことができないだろうか？つまるところファクトリー関数の中でいい感じに扱うことはできないだろうか？というのがちょっとした追加の悩みどころ。

これは暫定的な意見だけれど、context.Contextのようにsync.Onceで囲んだメソッドを用意してそこで制御してあげると良いのかもしれない。

type DB interface { // 別にinterfaceにする必要はないかも？今回は説明のため
    ...
    Failure(err error) error
}

type db struct {
    ...
    transaction *Transaction
    once sync.Once
    err error
}

func (db *db) Failure(err error){
  db.once.Do(func(){
      db.err = err
  })
  return err
}

func Open(...) (DB, func(), error) {
    t, err := newTransaction()
    db := &db{transaction: t}
    if err != nil {
        return db, nil, err
    }
    cleanup := func(){
        if db.err != nil {
            return db.transaction.Rollback(err)
        }
        return db.transaction.Commit()
    }
    return db, cleanup, err    
}

こうしてあげるとUseの中でFailure()を呼ぶ対応する必要があるが、何とかなる。

db, cleanup, err := Open(...)
if cleanup != nil {
    defer cleanup()
}    
if err != nil {
    return err
}
return Use(db, ...)

// func Use(db DB, ...) error{
// ...
// if err := cont(...); err != nil {
//    return db.Failure(err)
// }

別の方法としてはcallbackに対してerrorbackを返すようにするというものだけれど、これはもう使い手側が厳しくなってくると思う。

暫定的な意見の理由としては、Failure()を呼ぶのがだるくない？呼び忘れしそうじゃない？というのと本当に最初のエラーを捕まえるので良いの？というところ。

さいごに

ところで、こういう変更が一番めんどくさい（当初の値で良いのでmainで一回作成して渡しておけば良いよねというのがウソになり）。そして、この種の部分が一番先に見越して対応した気になっているとだるくなるだけの部分な気がしている（全部のコンポーネントをセッションとみなす対応はだるい）。

単純に言えば誰かがいい感じに代わりにやってほしい。

interfaceかもしれない。その辺については言及していない。↩
ちなみにどういう形式で返すべきかということには悩みはない。errorが再右端というのはlintを有効にしていればわかる。通常戻り値は最左端になるので、(object, func(), error) 以外考える必要はない。↩
まぁそれでも良いという意思決定が随所にみられるのがgoという気もするが。正確に言えばその部分の自由度は最大にしておきたい的な意味合い。↩
この言葉の定義が正しいかちょっと怪しい。言いたいのはブロックスコープではないのでifで囲っても特に困らないみたいなこと。↩
Close()の失敗の対応がちょっと面倒。ログに出力するだけにして良いのなら。↩