You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "reiern70 (via GitHub)" <gi...@apache.org> on 2026/10/20 19:09:14 UTC
[PR] Follow a flow exception thrown while the request is mapped [wicket]
reiern70 opened a new pull request, #1696:
URL: https://github.com/apache/wicket/pull/1696
`RequestCycle` follows a `ReplaceHandlerException` (`AbortWithHttpErrorCodeException`, `RestartResponseException`, `RedirectToUrlException` and the like) only while a request handler executes. One thrown while the request is mapped, before there is a handler, goes to the `onException` listeners and the exception mapper like any other exception, and ends on the error page or as a 500.
That is how the 400 for an ajax request without a base url (WICKET-4841) got lost. `ServletWebRequest#getClientUrl()` throws it, but since WICKET-5071 `BookmarkableMapper` and `PageInstanceMapper` call `getClientUrl()` while they map the request, so for an ajax request it is first called during mapping and the client gets a 500. The test added with WICKET-4841 only checks the exception `getClientUrl()` throws, so it kept passing. #1673, which answers a malformed base url the same way, runs into the same problem: with this change it does answer 400.
The change routes an exception thrown by `resolveRequestHandler()` through the same code `execute()` already uses for one thrown by a handler, including dropping the scheduled handler when the exception asks for it.
Tests:
- `RequestCycleListenerTest#replaceExceptionDuringMapping` mirrors the existing replace-during-response cases in `basicOperations`, with and without dropping the scheduled handler.
- `RequestCycleListenerTest#exceptionDuringMappingIsMapped`: any other exception thrown while mapping still goes to the listeners and the exception mapper.
- `ServletWebRequestTest#ajaxRequestWithoutBaseUrlAnswersBadRequest` sends an ajax request without a base url through `WicketTester` and checks the status the client gets. It fails on master.
**Behaviour change.** An application that throws a flow exception from a request mapper, or from anything a mapper calls, now has it followed: a `RestartResponseException` renders its page, a `RedirectToUrlException` redirects, an `AbortWithHttpErrorCodeException` answers its status code. Its `onException` listeners and its exception mapper no longer see these exceptions, and `onRequestHandlerResolved` is called for the replacement handler. Any other exception thrown while mapping is handled as before.
Suggested migration guide wording: *A `ReplaceHandlerException` (`RestartResponseException`, `RedirectToUrlException`, `AbortWithHttpErrorCodeException`, …) thrown while a request is mapped is now followed, as it always was while a request handler executes. Before, it was handed to `IRequestCycleListener#onException` and the `IExceptionMapper` and usually ended on the error page. An application whose mapper throws one and relied on the error page has to throw a different exception instead.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow a flow exception thrown while the request is mapped [wicket]
Posted by "reiern70 (via GitHub)" <gi...@apache.org>.
reiern70 commented on PR #1696:
URL: https://github.com/apache/wicket/pull/1696#issuecomment-6299072549
@papegaaij this changes what an application sees when a flow exception is thrown while a request is mapped, so I'd keep it on master and not cherry-pick it to wicket-10.x. On 10.x an ajax request without a base url, or with a malformed one, would keep answering 500, as it has since 2013. Do you agree, or do you think it is safe enough for 10.x?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow a flow exception thrown while the request is mapped [wicket]
Posted by "papegaaij (via GitHub)" <gi...@apache.org>.
papegaaij commented on PR #1696:
URL: https://github.com/apache/wicket/pull/1696#issuecomment-6309966843
I think it's ok to backport to 10. It's quite a corner case. I do not expect that anyone relies on the old behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Follow a flow exception thrown while the request is mapped [wicket]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1696:
URL: https://github.com/apache/wicket/pull/1696#issuecomment-6299161178
## [Codecov](https://app.codecov.io/gh/apache/wicket/pull/1696?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 62.18%. Comparing base ([`86b17a9`](https://app.codecov.io/gh/apache/wicket/commit/86b17a9fb2bbd0b07d1be4accaff363a6cd76d7a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`09c20ef`](https://app.codecov.io/gh/apache/wicket/commit/09c20efa0c63673687231eae4de011d532b08f44?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1696 +/- ##
=========================================
Coverage 62.17% 62.18%
- Complexity 11307 11312 +5
=========================================
Files 1253 1253
Lines 48455 48459 +4
Branches 6796 6796
=========================================
+ Hits 30125 30132 +7
Misses 15633 15633
+ Partials 2697 2694 -3
```
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@wicket.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org