fix: inherit the sub app's errorHandler in app.Route - #142
Open
poteto0 wants to merge 1 commit into
Open
Conversation
Route() merged only the sub app's routes and middlewares, so an error handler registered on the sub app via OnError was silently dropped and errors from its handlers always hit the parent's handler. Following hono's route(), a sub app's handlers are now wrapped with its own error handler, and only when it actually called OnError. An error returned by that handler still falls through to the parent's handler. To tell "customized" from "default" without comparing func values, errorHandler is left nil until OnError sets it and the default is applied at dispatch time (handleError). Route is also moved out of takibi_native.go / takibi_wasm.go into the shared route.go, where the two copies were identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Code Metrics Report
Code coverage of files in pull request scope (95.0%)
Reported by octocov |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #141
やったこと
Route(basePath, app)はサブアプリのルートとミドルウェアだけを親にコピーしていたため、サブアプリがOnErrorで設定したエラーハンドラは黙って捨てられていた。Hono のroute()に合わせて、サブアプリがOnErrorを呼んでいた場合のみ、サブアプリ由来のハンドラをそのエラーハンドラで包んでから親に登録する。blowErrorHandlerは引き継がない(Routeはサブアプリのtasksをマージしないため対象が存在しない)デフォルト判定のしかた
Go では関数値を
==で比較できないので、errorHandlerを コンストラクタでは nil のままにし、OnErrorが呼ばれたときだけセットするようにした。デフォルトはディスパッチ時にhandleErrorで適用する。sub.errorHandler != nilがそのまま「カスタムされたか」の判定になり、フラグを別に持たずに済む。native と wasm でデフォルトの本文が違う("Internal Server Error"/err.Error())ので、そこは既存挙動のまま各ビルドに残してある。ついでに
Routeの実装をtakibi_native.go/takibi_wasm.goから共有のroute.goに移動(今回の変更を 2 箇所に書かずに済む)route.Middlewareが空のときはrouter.Useを呼ばないようにした(Useは毎回サブツリーを再合成するため、無駄な再合成を省ける)docs
/docsの Sub-Routing ページに「errorHandler は引き継ぐ / Bindings と Blow タスクは引き継がない」を追記ITakibi.Routeの doc comment も同様に更新検討したが今回やっていないこと
サブアプリのミドルウェアが返したエラーは、サブアプリのエラーハンドラを経由せず親に届く。ミドルウェアは親の trie に
Useで登録され、合成時にラップの外側に来るため。単体で動かしたときとマウント後で挙動が変わる点なので気になるなら別途対応したい(エラー境界をミドルウェアとして表現する案がある)が、Issue の方針どおりハンドラのラップに留めた。テスト
just ci(ut / lint / lint-wasm / build-wasm / fmt) 通過。Routeのエラーハンドラ継承についてテーブルドリブンで 3 ケース追加。🤖 Generated with Claude Code