🐷

Goで書かれたプロジェクトの混沌としたエラーハンドリングを改善した話

2024/11/19に公開

こんにちは、バックエンドエンジニアの永田です。今回は、Go で書かれている CastingONE のバックエンドの、エラーハンドリングの改善に取り組んだ話しをします。

非推奨のパッケージを使っていたり、エラーにスタックトレースがついていないことがあるなど、元々混沌とした状況でした。同じような状況を抱えているプロジェクトもあるかと思うので、何らか参考になれば幸いです。

Before

  • エラーにスタックトレースをつけるためのパッケージとして github.com/pkg/errorsgolang.org/x/xerrors の二つが導入されており、混在している。しかも両方現在は非推奨。[1]

  • 標準パッケージのerrors.New()fmt.Errorf()関数を利用している箇所があり、それらを起点とするエラーにスタックトレースがついていないことがある。

After

  • エラーハンドリングを行う際には必ず独自のパッケージ(pkg/errorsのラッパー)を使うようにし、それ以外の方法でエラーハンドリングを行うと github actions で警告が出るようにした。これにより、エラーに必ずスタックトレースがつくようにした。

  • 今後スタックトレースについて乗り換えたいパッケージが見つかったり標準エラーパッケージの拡張が起きたときに、ラッパーだけ修正すれば済むようになった。

改善の手順

  1. x/xerrorsの利用箇所をerrors.New()fmt.Errorf()関数に置換し、プロジェクトの依存関係からx/xerrorsを削除する。

  2. pkg/errorsのラッパーを実装する。

  3. AST を用いてpkg/errorsの関数とerrors.New()fmt.Errorf()をラッパーの関数に置き換えるプログラムを実装し、実行する。

  4. pkg/errorsのインポート及びerrors.New()fmt.Errorf()の利用に警告を出すための github actions を実装し導入する。

1.x/xerrorsの除去

x/xerrorsは、Go 1.13 で標準ライブラリに導入されたエラー処理機能のベースとなったパッケージです。バージョンが進んだ現在も使っているのはおかしいので、利用箇所を標準パッケージのerrors.New()fmt.Errorf()に置換します。

一旦スタックトレースがつかなくなってしまいますが、後の手順によって再度つくことになります。

2.pkg/errorsのラッパーの実装

基本的にはpkg/errorsの関数をラップしているだけです。pkg/errors.Errorf()のみそのままではfmt.Errorf()と互換性がないため、少し工夫しています。

code
package error

import (
	"fmt"
	pkgerrors "github.com/pkg/errors"
)

// msgからトレース付きerrorを生成する
func New(msg string) error {
    return pkgerrors.New(msg)
}

// format文からトレース付きerrorを生成する
func Errorf(format string, args ...interface{}) error {
    // pkgerrors.Errorf()は%wに対応していないため、直接wrapせず以下のようにしてある。
    err := fmt.Errorf(format, args...)
    return pkgerrors.WithStack(err)
}

// errorにトレースをつける
func WithStack(err error) error {
    return pkgerrors.WithStack(err)
}

// errのメッセージをmsgでラップし、トレースも付与する
func Wrap(err error, msg string) error {
    return pkgerrors.Wrap(err, msg)
}

// errのメッセージをフォーマット文でラップし、トレースも付与する
func Wrapf(err error, format string, args ...interface{}) error {
    return pkgerrors.Wrapf(err, format, args...)
}

// errのメッセージをmsgでラップする. トレースは付与しない
func WithMessage(err error, message string) error {
    return pkgerrors.WithMessage(err, message)
}

// errのメッセージをフォーマット文でラップする. トレースは付与しない
func WithMessagef(err error, format string, args ...interface{}) error {
    return pkgerrors.WithMessagef(err, message)
}

3. AST を用いてpkg/errorsの関数とerrors.New()fmt.Errorf()をラッパーの関数に置き換える

置き換えツールのコードは以下になります。

これでは対応しきれないケースがあり手動で置き換えた場所があったり、プロジェクト固有の事情[2]も考慮しているので、あくまで参考程度になります。

code
package main

import (
	"bytes"
	"fmt"
	"go/ast"
	"go/format"
	"go/parser"
	"go/token"
	"log"
	"os"
	"path/filepath"
	"strings"
)

const (
	replacementPkg   = "github.com/CastingONE/castingone/go/pkg/error" // 置き換え後の関数の属するパッケージ
	replacementAlias = "pkgerror"                                      // 固定するエイリアス名
)

func main() {
	rootDir := "../../../../go" // 再帰的に探索するルートディレクトリ

	// 再帰的にディレクトリを歩いてGoファイルを修正する
	err := filepath.WalkDir(rootDir, func(path string, d os.DirEntry, err error) error {
		if err != nil {
			return err
		}

		// Goファイルだけを対象とする
		if !d.IsDir() && strings.HasSuffix(path, ".go") {
			fmt.Printf("Processing file: %s\n", path)
			if err := processFile(path); err != nil {
				return fmt.Errorf("failed to process file %s: %w", path, err)
			}
		}
		return nil
	})

	if err != nil {
		log.Fatalf("Error walking the path: %v\n", err)
	}
	fmt.Println("全てのファイルの処理が完了しました。")
}

// Goファイルを修正する処理
func processFile(filePath string) error {
	fset := token.NewFileSet()

	// ファイルをパース
	file, err := parser.ParseFile(fset, filePath, nil, parser.ParseComments)
	if err != nil {
		return fmt.Errorf("failed to parse file: %w", err)
	}

	// インポートパッケージをマッピング
	importMap := make(map[string]string)
	importExists := false
	var importDecl *ast.GenDecl // インポート文の位置を特定するための変数
	replaceOccurred := false    // 置き換えが行われたかどうかを追跡するフラグ

	for _, decl := range file.Decls {
		if genDecl, ok := decl.(*ast.GenDecl); ok && genDecl.Tok == token.IMPORT {
			importDecl = genDecl
			for _, spec := range genDecl.Specs {
				if importSpec, ok := spec.(*ast.ImportSpec); ok {
					pkgPath := strings.Trim(importSpec.Path.Value, "\"")
					alias := ""
					if importSpec.Name != nil {
						alias = importSpec.Name.Name
					} else {
						// パッケージ名を推定
						parts := strings.Split(pkgPath, "/")
						alias = parts[len(parts)-1]
					}

					importMap[alias] = pkgPath

					// すでに置き換え対象のパッケージがインポートされている場合、エイリアスを"pkgerror"に変更
					if pkgPath == replacementPkg {
						importSpec.Name = ast.NewIdent(replacementAlias)
						importExists = true
					}
				}
			}
		}
	}

	// 関数呼び出しの置き換え
	ast.Inspect(file, func(n ast.Node) bool {
		if callExpr, ok := n.(*ast.CallExpr); ok {
			if selExpr, ok := callExpr.Fun.(*ast.SelectorExpr); ok {
				if ident, ok := selExpr.X.(*ast.Ident); ok {
					pkgPath, exists := importMap[ident.Name]

					// "github.com/pkg/errors"の関数置き換え
					if exists && pkgPath == "github.com/pkg/errors" {
						ident.Name = replacementAlias
						replaceOccurred = true
					}

					// "errors.New()"の置き換え
					if exists && pkgPath == "errors" && selExpr.Sel.Name == "New" {
						ident.Name = replacementAlias
						replaceOccurred = true
					}

					// "fmt.Errorf()"の置き換え
					if exists && pkgPath == "fmt" && selExpr.Sel.Name == "Errorf" {
						ident.Name = replacementAlias
						replaceOccurred = true
					}
				}
			}
		}
		return true
	})

	// 置き換えが行われた場合にのみインポート文を追加
	if replaceOccurred && !importExists {
		newImport := &ast.ImportSpec{
			Name: ast.NewIdent(replacementAlias),
			Path: &ast.BasicLit{
				Value: fmt.Sprintf("\"%s\"", replacementPkg),
			},
		}

		// インポート文がある場合はその位置に追加、ない場合は新しく作成
		if importDecl != nil {
			importDecl.Specs = append(importDecl.Specs, newImport)
		} else {
			// 新しくインポート宣言を作成
			newGenDecl := &ast.GenDecl{
				Tok:   token.IMPORT,
				Specs: []ast.Spec{newImport},
			}
			// 新しいインポートを最初に追加
			file.Decls = append([]ast.Decl{newGenDecl}, file.Decls...)
		}
	}

	// ファイルの内容をバッファに書き込む
	var buf bytes.Buffer
	if err := format.Node(&buf, fset, file); err != nil {
		return fmt.Errorf("failed to format file: %w", err)
	}

	// 置き換え後の内容をファイルに書き戻す
	if err := os.WriteFile(filePath, buf.Bytes(), 0644); err != nil {
		return fmt.Errorf("failed to write file: %w", err)
	}

	return nil
}

4. pkg/errorsのインポート及びerrors.New()fmt.Errorf()の利用に警告を出すための github actions

PR でトリガーするワークフローで、差分のある Go ファイルを抽出し、Go のプログラムを呼び出してファイルの内容をチェックしています。

ルールに違反している場合ワークフローが Fail し、以下のコメントが PR に投稿されます。

pr_comment

code
name: Go Error Handling Check
on:
  pull_request:
    types: [opened, synchronize, reopened]

permissions:
  contents: read
  pull-requests: write

jobs:
  check-go-files:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout code
        uses: actions/checkout@v4

      - name: Set up Go
        uses: actions/setup-go@v5
        with:
          go-version: "1.23"

      - name: Get changed files
        id: changes
        run: |
          git fetch origin ${{ github.event.pull_request.base.sha }} --depth=1
          changed_files=$(git diff --name-only ${{ github.event.pull_request.base.sha }} -- '*.go')

          # $changed_filesに含まれるスラッシュをエスケープする
          changed_files=$(echo $changed_files | sed 's/\//\\\//g')
          echo "changed_files=${changed_files}" >> $GITHUB_ENV

      - name: Run Go program for rule checks
        id: check-rules
        run: |
          go build -o error_handling_check ops/tools/developers-tool/error_handling_check/main.go

          failed_files=""
          for file in $changed_files; do
            # $fileに含まれるエスケープされたスラッシュを元に戻す
            file=$(echo $file | sed 's/\\\//\//g')

            # ルールチェックを行うGoプログラムの呼び出し
            exit_code=0
            ./error_handling_check "$file" || exit_code=$?
            if [ $exit_code -eq 1 ]; then
              if [ "$failed_files" = "" ]; then
                failed_files="$file"
              else
                failed_files="$failed_files $file"
              fi
            fi
          done

          echo "failed_files=$failed_files" >> $GITHUB_OUTPUT
          echo "failed_files=$failed_files" >> $GITHUB_ENV

      - name: Post comment if there are failed files
        if: ${{ steps.check-rules.outputs.failed_files != '' }}
        uses: actions/github-script@v6
        with:
          script: |
            const failedFiles = process.env.failed_files.split(' ');
            const commentBody = `:warning: 以下のファイルに、github.com/pkg/errorsのimport、またはerrors.New()、fmt.Errorf()の呼び出しが含まれています。github.com/CastingONE/castingone/go/pkg/errorを代わりに使いましょう。\n\n${failedFiles.map(f => `- ${f}`).join('\n')}`;
            github.rest.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: commentBody
            })

            core.setFailed("Some files failed the check.");
package main

import (
	"fmt"
	"go/ast"
	"go/parser"
	"go/token"
	"os"
	"strings"
)

const (
	checkFailedExitCode = 1
	otherErrorExitCode  = 2
)

// チェックをスキップしたいファイルやディレクトリのリスト
var excludedFiles = []string{
	"go/pkg/error/error.go",
	"go/pkg/logger/stacktrace.go",
	"go/pkg/test/test.go",
}

var excludedDirs = []string{
	"ops",
	"go/pkg/dbtype/model",
	"go/console/interface/graph/generated",
	"go/console/interface/graph/model",
}

func main() {
	if len(os.Args) < 2 {
		fmt.Println("Usage: go run main.go <file-path>")
		os.Exit(otherErrorExitCode)
	}

	filePath := os.Args[1]
	if shouldSkipCheck(filePath) {
		fmt.Printf("Skipping check for %s\n", filePath)
		os.Exit(0)
	}

	checkFailed, err := checkFileForErrors(filePath)
	if err != nil {
		fmt.Println("Error:", err)
		os.Exit(otherErrorExitCode)
	}

	if checkFailed {
		os.Exit(checkFailedExitCode)
	}

	fmt.Println("No issues found.")
	os.Exit(0)
}

func shouldSkipCheck(filePath string) bool {
	// ファイル名がexcludedFilesリストに含まれている場合
	for _, f := range excludedFiles {
		if filePath == f {
			return true
		}
	}

	// ディレクトリ名がexcludedDirsリストに含まれている場合
	for _, dir := range excludedDirs {
		if strings.HasPrefix(filePath, dir) {
			return true
		}
	}

	return false
}

func checkFileForErrors(filePath string) (bool, error) {
	// Goファイルのソースコードを解析
	fset := token.NewFileSet()
	node, err := parser.ParseFile(fset, filePath, nil, parser.AllErrors)
	if err != nil {
		return false, fmt.Errorf("could not parse file %s: %w", filePath, err)
	}

	var issues []string

	// ASTを走査して指定されたインポートや関数呼び出しを検出
	ast.Inspect(node, func(n ast.Node) bool {
		// インポートをチェック
		if imp, ok := n.(*ast.ImportSpec); ok {
			if imp.Path.Value == `"github.com/pkg/errors"` {
				pos := fset.Position(imp.Pos())
				issues = append(issues, fmt.Sprintf("Line %d: found import of 'github.com/pkg/errors'", pos.Line))
			}
		}

		// 関数呼び出しをチェック
		if call, ok := n.(*ast.CallExpr); ok {
			if fun, ok := call.Fun.(*ast.SelectorExpr); ok {
				// errors.New() のチェック
				if pkg, ok := fun.X.(*ast.Ident); ok && pkg.Name == "errors" && fun.Sel.Name == "New" {
					pos := fset.Position(call.Pos())
					issues = append(issues, fmt.Sprintf("Line %d: found call to 'errors.New()'", pos.Line))
				}
				// fmt.Errorf() のチェック
				if pkg, ok := fun.X.(*ast.Ident); ok && pkg.Name == "fmt" && fun.Sel.Name == "Errorf" {
					pos := fset.Position(call.Pos())
					issues = append(issues, fmt.Sprintf("Line %d: found call to 'fmt.Errorf()'", pos.Line))
				}
			}
		}

		return true
	})

	// 問題が見つかった場合、エラーメッセージを生成
	if len(issues) > 0 {
		fmt.Printf("file %s has the following issues:\n%s\n", filePath, joinIssues(issues))
		return true, nil
	}

	return false, nil
}

// 複数のエラーを一つの文字列に結合
func joinIssues(issues []string) string {
	return " - " + stringJoin(issues, "\n - ")
}

func stringJoin(strs []string, sep string) string {
	result := ""
	for i, str := range strs {
		if i > 0 {
			result += sep
		}
		result += str
	}
	return result
}

学び

  • pkg/errorsについて、何となく使っていた各関数の違いや、標準エラーパッケージと乖離してきてしまっていることが分かりました。(本当に今更なんですが...)

  • 単なる置換では難しい広範囲の修正について、AST を用いたプログラムで修正するという経験ができました。(プログラム自体は Chat GPT にかなり頼りましたが...)

脚注
  1. pkg/errorsx/xerrorsが誕生した背景、なぜ現在は非推奨なのか等々については複雑な歴史があるわけですが、ここでは触れません。 ↩︎

  2. ラッパーはgithub.com/CastingONE/castingone/go/pkg/errorというパッケージに配置しているのですが、パッケージ自体は以前から存在しており、さまざまな場所で参照されていました。そのため、置き換えを行った後にインポート文を追加すべきケースと追加すべきではないケースがありました。 ↩︎

Discussion