Repository navigation
Merge OnException handlers for the same exception type into one catch clause - #5357
Conversation
… clause Two middleware types that handled the same exception type generated two catch clauses for it, and the handler failed to compile with CS0160. A later handler for a type that is already caught now joins the existing clause, and the variables it creates get names no earlier handler in that clause uses. The handlers run in registration order until one of them ends the request or rethrows.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c878c19c19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both cascaded messages in both_handlers_can_return_a_value can be handled in parallel off the local queue, and concurrent List<T>.Add can drop one.
|
Thanks, merged. Merging into one catch clause is the right call; a startup error would penalize a legitimate setup like |
#5357) Two middleware OnException methods for one exception type now share a catch block in registration order, and an early exit stops the rest. The docs say so, and recommend an explicit try/catch where the order is in doubt, plus checking the generated code with codegen preview/write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DCggXgxnG3EBXoR9rrq37U
Two middleware types that each declare
OnExceptionfor the same exception type made every handler they apply to fail code generation with CS0160, becauseTryCatchFinallyFrameadded a second catch clause for that type. One way to hit it isMapMartenConcurrencyFailuresToConflict()next to an app's ownConcurrencyExceptionmiddleware.TryCatchFinallyFrame.AddCatchBlocknow adds a later handler for a type that is already caught to the existing clause. The variables the handler creates get names that no earlier handler in that clause uses, so two handlers that return values don't declare the same local twice (CS0128).Beforethat stops the chain. If you'd rather have a startup error naming both middlewares, I can switch to that.HandlerContinuationhandlers) fail onmainand pass with the fix. Full CoreTests pass on net10.0.OnExceptionthat returns a tuple still doesn't compile, even on its own. That's A middleware OnException that returns a tuple generates code that doesn't compile #5356.