codeconsole commented on PR #16230: URL: https://github.com/apache/grails-core/pull/16230#issuecomment-5724744040
Addressed the round-2 review in `69b234e3a5`, `683342d225`, `b698b3f540`, and `f3624cb8c3`, following upstream merge `b09d755655`. - **N1:** Replaced the relevant proxy-based closure coercions with real Groovy lambdas. Added checked-exception coverage for direct/timed/chained results, completion and error callbacks, aggregate callbacks, non-CompletableFuture promise bridges, web tasks, and controller error handling. `IOException` now remains the original cause rather than an `UndeclaredThrowableException`. - **N2:** Standalone `WebPromises` now decorates the owned executor, keeping request context bound through CompletableFuture completion and attached `then` callbacks. Third-party providers retain the compatibility lookup-strategy path. Latched tests cover default, virtual-thread, and legacy factories. Also fixed list task creation to use the receiving factory rather than the global factory, preserving standalone web-context propagation. - **N3:** `GrailsExceptionResolver` now resolves `AsyncRequestTimeoutException` as 503, including status-specific URL mappings, without logging an ERROR stack trace. Added resolver coverage and a real HTTP timeout integration test. Documented the container default (normally 30 seconds on Tomcat), configuration, and that response timeout does not cancel the task. - **N4:** Returning a promise after async request completion now throws `IllegalStateException`, consistent with submitting through `WebPromises`. - **N5:** Renamed the static helper to `AsyncRequestSupport`, made the request variable explicitly typed, and cached the parsed timeout per application context using weak keys. - **N6:** Added debug logging for executor selection and a regression test for a mistyped named executor. One clarification: `BeanNotOfRequiredTypeException` extends `BeansException`, not `NoSuchBeanDefinitionException`, so a mistyped `applicationTaskExecutor` already fails startup rather than silently selecting the fallback; the new test pins that behavior. - **N8:** Converted `VirtualThreadPromiseFactory` to a thin CompletableFuture factory over its owned virtual-thread executor. It now shares the same failure/aggregate contracts, composes task decorators, propagates web request context, exposes its executor for asynchronous event delivery, and closes the owned executor on context shutdown. Plugin tests exercise request propagation, custom decoration, virtual-thread event delivery, and lifecycle cleanup. The changed aggregate contracts and distinction from Boot-managed virtual threads are documented. - **N9:** Merged upstream `8.0.x` through `e749cc79a9`, retained both upstream upgrade sections, and moved the async migration section to 66. GitHub currently reports the PR as mergeable. - **N7:** Kept the commits focused and unsquashed, as explicitly requested by the PR author. No AI trailers were added. Verification: Java 21 module suites cover 427 tests (one existing skip); Java 25 async/MVC suites cover 156 (one existing skip, build cache disabled); the Java 21 sample integration suite passes all 18 tests, including the new timeout case. Affected-module style and dependency-version checks pass. These were resource-limited runs, not a repository-wide clean/aggregate sweep. Updated the PR's combined summary and LOC table. Relative to the merged upstream base: 1,804 lines added, 256 removed, net +1,548; 1,018 retained compatibility lines are slated for future removal, leaving a projected net +530 overall, or -560 for production code alone. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
