😡

空のTryCatchを作らないで！(ついでに非同期周り)

2023/03/16に公開

net

tech

初めに

正直、空のTryCatchを使う人は稀だと思いますが、社内のソースコードで乱用しているテロリストがおり、一被害者として注意を呼びかけたいと思います。
また、C#のコードで書いていますが例外を扱うすべての言語に通ずる話だと思います。

問題のコード

問題となったアプリケーションでは、とあるクラスオブジェクトをシリアライズ、デシリアライズ化し、リソースとして管理するための機構がありました。ここではそのクラスを仮に「Dirty」と呼ぶことにします。
Dirtyのコードは大体こんな感じでした。


public class Dirty : ISerializable
{
    public void GetObjectData(SerializationInfo info, StreamingContext context)
    {
        try
        {
            info.AddValue("hoge", "hoge");
            info.AddValue("huga", "huga");
        }
        catch
        {
            // 無視されている
        }
    }
}

このアプリケーションでは、一部のリソースを指定するとアプリケーションが落ちるという不具合があり、原因は上記のシリアライズ時にエラーが吐かれておりながら無視してリソースとして保存していること、
そして、デシリアライズ時にも失敗し特定のプロパティがnullの状態で処理が進んだためです。

シリアライズに失敗していることそのものよりも、そのままアプリケーションが（通常通りに）動き続けていること、そしてデバック実行の際ですらエラーが特定しにくいことが問題です。

特にデバック実行に関して、通常VisualStudioでデバック実行している際はキャッチされない例外が出力された際は処理が止まり、出力先のコードまで遷移してくれます。
しかしながら空のキャッチではそれすらできないため、もはやキャッチしないほうがましだとすら言えます。

考え方

例外というのはその名の通り、イレギュラーなものです。
イメージとしては、その例外が発生した時点でルートが分岐しています。

Rustのパターンマッチによる例外の取り扱いは特に上記のイメージに近いと思います。

fn sample() {
    let result = Ok(());
    match result {
        Ok(_) => {
            // 成功時の処理
        }
        Err(_) => {
            //　失敗時の処理
        }
    }
}

分岐された例外のルートから通常のルートに戻るためには、現在（通常と比べて）矛盾が発生している箇所の整合性をとる必要があります。
Gitでいうマージのようなイメージです。
それが無理な状態ならば、ログだけでも出力しておいて処理の中断か、アプリケーションの終了をするべきでしょう。
どちらにせよ、空のTryCatchは最悪な選択です。

Async voidをキャッチしても意味ない

これも信じられない話かもしれませんが、async voidをキャッチしようとしているコードがありました。
ちなみに先ほどと同じアプリケーションです。

これについては下記ページを参照してください。

private static void Nanicore()
{
    try
    {
        Usodesho();
    }
    catch (Exception e)
    {
        
    }
}

private static async void Usodesho()
{

}

そもそもAsync voidを使うべきではない

このページにも書かれていますが、async voidはそもそも使うべきではないです。
使うとしても、ボタンのクリック時などのイベントハンドラぐらいでしょう。

投げっぱなしにしたいけどコンパイラからの警告が気になるならばTask内で例外を適切に扱いつつ、ContinueWithなどを使いましょう。

private static void Sample()
{
    HogeAsync().ContinueWith(_ => { ; });
}

private static async Task HogeAsync()
{}

拡張メソッドにしてUniTaskのForgetのように呼び出せるようにしてもいいかもしれません。

public static class TaskExtension
{
    public static void Forget(this Task self)
    {
        self.ContinueWith(_ => {; });
    }
}
private static void Sample()
{
    HogeAsync().Forget();
}

最後に

そもそもC#の例外処理はエラーチェックの強制力が弱いのが問題な気がしなくもない...。

初めに

問題のコード

考え方

Async voidをキャッチしても意味ない

そもそもAsync voidを使うべきではない

最後に

Discussion