現在参加しているプロジェクトで、どういう風にエラー処理をするべきか迷った箇所があるので、選択した設計方針について書いてみたい。
エラー処理を書く対象のメソッドの仕様
該当するコードは、Log Analytics のログを検索して取得するメソッドです。内部でREST-API をコールしています。
public async Task<ActivityWindowStatusLog[]> SearchAsync(string query,int top, DateTime start, DateTime end)
{
var httpClient = new HttpClient();
httpClient.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", await GetAccessToken());
var content = new StringContent(GetQueryString(query, top, start, end), Encoding.UTF8, "application/json");
var response = await httpClient.PostAsync(GetSearchUri(), content);
var body = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<Rootobject>(body);
:
このコードは、単に REST-API に検索キーを渡して検索するだけなのですが、問題は、この検索用 REST-API の仕様でした。例えば、query に対して、TYPE=sometype というのを指定して検索するときに、sometypeが無ければ、検索０件となりそうなものですが、エラーになります。ところが、このエラーは、リクエスト自体が失敗するのではなく、このREST-API の戻り値の中の、metadata 情報のところに、error とこっそり帰ってきます。ちなみに、原因は、このメタ情報ではわからず、同じクエリで、Log Analytics を検索すればわかるという感じです。
result.__metadata.resultType == "error"
ちょうどこんな感じで判定できます。さて、このエラーハンドリングをどう実装しようか？というところです。なぜ、TYPE がマッチしないだけでエラーになるかというと、このTYPEはログの種類で、固定で決まっているものと、自分がカスタムのログを投げるときに名前を付けてログを投げますが、その値がここにくる TYPE です。つまりプログラムを書く側は明確にこの値はわかっているはずなのです。しかし、HTTPのリクエスト自体がこけてるわけではないというなんとも微妙な感じになっています。
実装方針の検討
この実装方針には、３つの方針が考えられます。
- エラーとせず０件で返す
- 例外を投げる
- REST-API と同じインターフェイスにする。返却オブジェクトを作って、そこに、
resultTypeを持たせる。
結局師匠とディスカッションして2. にしたのですが、その検討の過程をお話しします。
1. エラーとせず０件とする
これは検索メソッドなのだから、TYPE が単にヒットしないだけと考えると、別に０件が返ってきても不思議はありません。ただ、これはライブラリなので、自分がライブラリを使う側だとすると、クエリーがなんとなくヒットせず、エラーにもならないと相当ハマると思われます。そこで最初に考えたアイデアが、クエリーは０件にするけど、ワーニングメッセージとして、分かるようにするのはどうだろうと考えました。
例えばこんな実装。
if (resultObject.__metadata.resultType == "error")
{
Console.WriteLine($"Warning: returns error for your query: \"{query}\". To solve this problem, please search by the same query on your Log Analytics");
}
メッセージは長いですが、クエリーと、それを、Log Analytics で検索してねというメッセージを持たせるという感じです。ちなみに、このライブラリは、Azure Functions から使う、ヘルパなのですが、Warning メッセージを出力するためには、Azure Functions 側からログオブジェクトを引数にもらう必要があります。その所で、なんとなくこの方針に対して「不吉なにおい」を感じました。また、いくらワーニングを出したところで、開発者は気づかないかもしれません。エラーにならないので。師匠が言った「ということは、エラーは通知されないの？」が決め手となってやっぱ例外で実装しようと思い立ちました。
2. 例外を投げる
「例外を投げる」方針に若干抵抗があったのは、「これは本当に例外か？、通常の起こりうるケースではないのか？」という点です。ほかにもパフォーマンスという話もあります。パフォーマンスに関しては、１秒に１００回以上例外がスローされるなら、検討するという方針があるので、それを考えるとここでは問題ではありませんが、「これは例外か？」ということです。
「例外か否か」
最終的に思い至ったのは、TYPE=sometypeのsometype は本来は、ログを送った人なら当然知っている値で、これを指定しないと、関係ないログまで拾ってしまいます。TYPEの種類はだから、そうやってカスタムで指定されたものと、プリセットのものなので、本来正しくあるべき。だからたぶんLogAnalytics 側でerrorになるのでしょう。そう考えると、TYPE=のクエリを間違えて書いていた場合は、プログラマは気づいてほしい。それを正しく記述すると、このエラーは起きない。ということを考えると、これは「例外」であると考えることができます。Validation をすればいいのではとも思ったのですが、クライアント側では判別できない性質ですので、例外に決定しました。
既存の例外か、カスタム例外か？
.NETのクラスライブラリ設計 開発チーム直伝の設計原則,コーディング標準,パターンを読んでいると方針がのっていました。
DO NOT 使用法エラーを伝えるために、例外型を作成してはいけません。ArugumentNullException, ArgumentException, InvalidOperationException, NotSupportedException 等のいづれかを使う
カスタム例外を使う場合としては
他の既存の例外とは異なる方法でプログラム的に処理することが可能であるエラー状態があるならば、カスタム例外を作成およびスローすることを検討する。
とあります。今回ので言うと、InvalidOperationError が近い気がしますが、InvalidOperation にしてしまうのは若干今回の例は違う気がしました。ですので、カスタム例外を作成してみました。
/// <summary>
/// The exception that is thrown when the query is invalid
/// </summary>
public class InvalidQueryException : Exception
{
/// <summary>
/// Constructs a new <see cref="InvalidQueryException"/>.
/// </summary>
public InvalidQueryException()
{
}
/// <summary>
/// Constructs a new <see cref="InvalidQueryException"/> with the given message.
/// </summary>
/// <param name="message">The exception message.</param>
public InvalidQueryException(string message): base(message)
{
}
/// <summary>
/// Constructs a new <see cref="InvalidQueryException"/> with the given message and inner exception.
/// </summary>
/// <param name="message">The exception message.</param>
/// <param name="inner">The inner exception.</param>
public InvalidQueryException(string message, Exception inner): base(message, inner)
{
}
}
例外のメッセージを設計する
例外のメッセージにもガイドがあります。
例外をスローするときには、開発者を対象とした十分いで意味のあるメッセージテキストを提供します。
例外メッセージが文法的にただしいことを確認します。
メッセージテキストの各文にピリオドがあることを確認します。
例外メッセージでは、疑問文(?)や感嘆符(!)をさけます。
またこんなことも。
適切なアクセス許可を要求することなく、例外メッセージでセキュリティ上重要な情報を開示してはいけません
なるほど。というわけでこんな感じに。
if (result.__metadata.resultType == "error")
{
throw new InvalidQueryException($"Invalid Query: \"{query}\". Try execute the same query on the LogSearch of your LogAnalytics ");
}
文法的に正しいかは知らんけど、とりあえず、Grammary はただしいといってるので、後は同僚の Peter の Pull Request レビューに期待しよう。
3. REST-API と同じインターフェイスにする。返却オブジェクトを作って、そこに、resultTypeを持たせる。
これは却下。なぜなら、上記の本に
エラーコードを返してはいけません。
また、
例外をスローすることによって実行エラーを報告します。
戻り値にラップしても、戻り値を返す方針にはかわらないので、これは悪い設計です。何故悪いかというと、戻り値は、しばしば無視されるからです。
おわりに
自分はこのように考えてエラーハンドリングを実装しましたが、もっと良い意見があるお方は是非コメントお待ちしております。
リソース
この本最高！