You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "ruthst00 (via GitHub)" <gi...@apache.org> on 2026/10/04 18:42:24 UTC
[PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
ruthst00 opened a new pull request, #16516:
URL: https://github.com/apache/grails-core/pull/16516
## Description
<!-- Describe your change and the problem it solves. Link to the related issue(s) if they exist. -->
**Root Cause:** Several `render()` branches in `ResponseRenderer.groovy` set `webRequest.renderView = false` and wrote content to the response, but never called `response.flushBuffer()`. This left the response uncommitted, causing Spring MVC's `DispatcherServlet` to attempt default view resolution after the action returned, resulting in a `ServletException: Could not resolve view with name '<actionName>'`.
**Fix:** Added `response.flushBuffer()` after content is written in every render branch that sets `renderView = false`:
1. `render(Object)` — after writing `object.inspect()`
2. `render(CharSequence)` — after writing and flushing the writer
3. `render(Map, Writable)` — after `renderWritable()`
4. `render(Map)` with `text:` (Writable) — after `renderWritable()`
5. `render(Map)` with `template:` — after all template rendering paths
6. `render(Map)` with `file:` — after `SpringIOUtils.copy()`
7. `renderMarkupInternal()` — after `renderWritable()` (covers `render(Closure)` and `render(Map, Closure)` markup path)
8. `renderJsonInternal()` — after `jsonBuilder.call()` (covers `render(Map, Closure)` JSON path)
The `render(Map)` with `status:` branch already had `response.flushBuffer()` — this fix brings all other content-writing branches into parity.
**Tests:** Added 10 new Spock feature methods to `ResponseRendererSpec` verifying that `response.isCommitted()` is `true` after each render variant. All 17 tests in the spec pass, the full `grails-controllers` test suite passes, and all violation reports (Checkstyle, CodeNarc, PMD, Repository Conventions) are clean.
Fixes [#15819](https://github.com/apache/grails-core/issues/15819)
Generated with [Claude Sonnet 4.6](https://www.anthropic.com/claude/sonnet) via [Cline API Provider](https://cline.bot/)
## Contributor Checklist
Please review the following checklist before submitting your pull request. Pull requests that do not meet these requirements may be closed without review.
### Issue and Scope
- [X] This PR is linked to an existing issue that has been **acknowledged or approved** by the project team. If no approved issue exists, please give background on why this change is necessary. Tickets are preferred for release change log history.
- [X] This PR addresses the **complete scope** of the linked issue. Partial implementations or unfinished work should not be submitted for review.
- [X] This PR contains a **single, focused change**. Unrelated changes should be submitted as separate pull requests.
- [X] This PR targets the **correct branch** for the type of change:
- **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new features or API changes.
- **Minor release branches** (e.g., `7.1.x`): New features are welcome, but breaking existing APIs must be avoided.
- **Major release branches** (e.g., `8.0.x`): Reserved for major changes. Breaking API changes are permitted.
### Code Quality
- [X] I have **added or updated tests** that cover the changes introduced in this PR. All code contributions are expected to include appropriate test coverage.
- [X] I have verified that all existing tests pass by running `./gradlew build --rerun-tasks`.
- [X] My code follows the project's **code style** guidelines. I have run `./gradlew codeStyle` and resolved any violations. See [Code Style](../CONTRIBUTING.md#code-style) for details.
- [X] This PR does **not** include mass reformatting, style-only changes, or large-scale refactoring unless it was **explicitly approved** in the linked issue. Unsolicited reformatting will not be accepted.
- [X] If generative AI tooling was used in preparing this contribution, a quality model was used to ensure contributions are **consistent with the project's quality standards**.
### Licensing and Attribution
- [X] All contributed code is provided under the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include the appropriate **Apache license header**.
- [X] I have the necessary rights to submit this contribution and confirm it is my own original work (see [Legal Notice](../CONTRIBUTING.md#i-want-to-contribute)).
- [X] If generative AI tooling was used in preparing this contribution, I have followed the [Apache Software Foundation's policy on generative tooling](https://www.apache.org/legal/generative-tooling.html) and have properly attributed its use.
### Documentation
- [ ] If this PR introduces user-facing changes, I have included or updated the relevant documentation.
- [ ] If this PR adds a new feature, I have updated the **What's New** section of the Grails Guide.
- [ ] If this PR introduces breaking changes or changes that require user action during an upgrade, I have updated the **Upgrade Notes** for the corresponding version in the Grails Guide.
- [X] The PR description clearly explains **what** was changed and **why**.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6085710560
@jdaugherty, @jamesfredley, in an attempt to recreate the [issue](https://github.com/apache/grails-core/issues/15819), I created a [new Grails 8 app](https://github.com/ruthst00/15819-response-renderer-overloads) with a controller action that calls `render(template: '_results', model: [...])` on an AJAX endpoint with no GSP layout. Unfortunately I don't observe any exceptions written to browser console or stdout/stderr:
``` java
// grails-app/controllers/com/example/SearchController.groovy
class SearchController {
static allowedMethods = [results: 'GET']
/**
* Returns an HTML fragment for AJAX callers, without a GSP layout.
*/
def results() {
// Grails prefixes template names with "_", so this resolves to __results.gsp.
render(template: '_results', model: [results: ['Grails', 'Groovy', 'Spring Boot']])
}
}
```
Could this be a Grails 7 only issue?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6298802547
@jdaugherty, new functional test added with https://github.com/apache/grails-core/pull/16516/changes/d3a8be33cfb006892f7a3cf26efd1dcd04c736d9. This is my first function test app so please let me know if I missed anything ;o)
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4222936862
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ // All other render() variants (template, text, JSON, file, closure, object) set
+ // webRequest.renderView = false. If that flag is clear the response has already been
+ // handled; returning a ModelAndView here would cause DispatcherServlet to attempt
+ // view resolution and throw "Could not resolve view". (#15819)
+ if (!webRequest.renderView) {
Review Comment:
A redirect, and a response that is already committed, now return no view. An included action whose outer response is already committed is still dropped. That case is on the new check.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
render(view:) now calls render(view:). The new redirect row still sets the redirect attribute itself instead of calling redirect().
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,27 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ if (result instanceof Map) {
+ // All render() variants except render(view:) set webRequest.renderView = false.
+ // Check the raw flag (not the composite isRenderView(), which also returns false
+ // for error status, committed response, or redirect) so that only an explicit
+ // render() call suppresses view resolution. (#15819)
+ //
+ // redirect() sets REDIRECT_ISSUED on the request and a 3xx status without calling
+ // setRenderView(false), so renderViewRequested stays true. Similarly, the response
+ // may already be committed (e.g. the body was written directly) without that flag
+ // being cleared. In both cases there is nothing left for DispatcherServlet to do.
+ if (!webRequest.renderViewRequested
+ || request.getAttribute(GrailsApplicationAttributes.REDIRECT_ISSUED) != null
+ || response.committed) {
Review Comment:
response.committed is already true when a servlet include runs after the outer response has been flushed. The include can still append output, and an included action that returns a Map used to get a view. This check now returns null, so that include writes nothing. Skip the committed check for an include dispatch, and cover an already-committed outer response.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +405,73 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Calls render(text:), which sets renderView=false and writes the body. The adapter must
+ * return null so DispatcherServlet does not attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ render(text: 'hello')
+ null
+ }
+
+ /**
+ * Calls render(text:) and also returns a Map — the exact bug scenario from #15819 where the
+ * adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ render(text: 'hello')
+ [foo: 'bar']
+ }
+
+ /**
+ * Calls redirect() (which sets REDIRECT_ISSUED on the request and a 3xx status without calling
+ * setRenderView(false)) and also returns a Map. The adapter must return null — there is nothing
+ * left for DispatcherServlet to do after a redirect. (#15819)
+ */
+ @Action
+ def redirectWithMap() {
+ request.setAttribute(
Review Comment:
This does not call redirect(). It sets REDIRECT_ISSUED and status 302 itself, so the row stays green even if redirect() stops setting that attribute. Call redirect() and assert the status and Location header, the way renderText() calls render(text:).
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4223554835
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,27 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ if (result instanceof Map) {
+ // All render() variants except render(view:) set webRequest.renderView = false.
+ // Check the raw flag (not the composite isRenderView(), which also returns false
+ // for error status, committed response, or redirect) so that only an explicit
+ // render() call suppresses view resolution. (#15819)
+ //
+ // redirect() sets REDIRECT_ISSUED on the request and a 3xx status without calling
+ // setRenderView(false), so renderViewRequested stays true. Similarly, the response
+ // may already be committed (e.g. the body was written directly) without that flag
+ // being cleared. In both cases there is nothing left for DispatcherServlet to do.
+ if (!webRequest.renderViewRequested
+ || request.getAttribute(GrailsApplicationAttributes.REDIRECT_ISSUED) != null
+ || response.committed) {
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/53679a78227345e58c92d7725dc5897526e90321
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +405,73 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Calls render(text:), which sets renderView=false and writes the body. The adapter must
+ * return null so DispatcherServlet does not attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ render(text: 'hello')
+ null
+ }
+
+ /**
+ * Calls render(text:) and also returns a Map — the exact bug scenario from #15819 where the
+ * adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ render(text: 'hello')
+ [foo: 'bar']
+ }
+
+ /**
+ * Calls redirect() (which sets REDIRECT_ISSUED on the request and a 3xx status without calling
+ * setRenderView(false)) and also returns a Map. The adapter must return null — there is nothing
+ * left for DispatcherServlet to do after a redirect. (#15819)
+ */
+ @Action
+ def redirectWithMap() {
+ request.setAttribute(
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/53679a78227345e58c92d7725dc5897526e90321
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4217515634
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ // All other render() variants (template, text, JSON, file, closure, object) set
+ // webRequest.renderView = false. If that flag is clear the response has already been
+ // handled; returning a ModelAndView here would cause DispatcherServlet to attempt
+ // view resolution and throw "Could not resolve view". (#15819)
+ if (!webRequest.renderView) {
Review Comment:
@jamesfredley, thank you for your feedback. I agree the correct fix is to check the raw `renderView` field (not the composite `isRenderView()`), so I added a method to `GrailsWebRequest` that exposes just the field value without the composite checks. Hopefully this fixes the problem: https://github.com/apache/grails-core/pull/16516/changes/ba4575019ce032554682711fcd2c16c84d52845c
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4217518712
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
Tests now call `render()` directly. Also added a test case where an action sets an error status AND returns a model to prove the guard doesn't swallow error views: https://github.com/apache/grails-core/pull/16516/changes/ba4575019ce032554682711fcd2c16c84d52845c
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-5983669522
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16516?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 59.5405%. Comparing base ([`9a8ceda`](https://app.codecov.io/gh/apache/grails-core/commit/9a8ceda9a4b83185eee1332ac6f7b16a02322be4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0d0ac73`](https://app.codecov.io/gh/apache/grails-core/commit/0d0ac73e33dd1e2241c8ee228a43ce32bc325861?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 32 commits behind head on 8.0.x.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16516?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16516 +/- ##
==================================================
+ Coverage 59.5208% 59.5405% +0.0197%
- Complexity 25014 25019 +5
==================================================
Files 2175 2175
Lines 108636 108637 +1
Branches 19823 19823
==================================================
+ Hits 64661 64683 +22
+ Misses 35020 34999 -21
Partials 8955 8955
```
[see 5 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16516/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4227590271
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
This was fixed in https://github.com/apache/grails-core/pull/16516/changes/53679a78227345e58c92d7725dc5897526e90321
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6346976279
@matrei, thanks very much for the risk assessment. Based on your recommendations, should I:
1. Retarget this entire PR for 9.0.x
2. Split this PR into two PRs, one for 8.0.1 and the other for 9.0.x
My preference would be Option 1 as Option 2 could involve some major surgery.
cc: @jdaugherty
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6410920224
@ruthst00 I'd go with option 2. #15819 is a bug in 8.0.x and the fix for it is small, so I don't think apps on 8.0.x should have to wait for 9.0 to get it. I also don't think the split needs much surgery:
**This PR, against `8.0.x`**
- `GrailsDispatcherServlet` as in the sketch in point 2, which takes care of points 1 and 2.
- `GrailsDispatcherServletSpec` and `GrailsDispatcherServletHiddenMethodSpec` updated to match, with the `Callable` case added.
- In `UrlMappingsHandlerMappingSpec`, the late-filter feature with only the `renderTemplate` row. It should assert that rendering uses the instance in the request attribute, not the filter's.
- The example app with only the `results` rows.
- `UrlMappingsInfoHandlerAdapter`, `GrailsWebRequest` and `render.adoc` can simply be checked out from `8.0.x` again, and the adapter rows come out of `UrlMappingsHandlerMappingSpec`.
**A new PR against `9.0.x`**
- The adapter guard, `isRenderViewRequested()` (`@since 9.0.0`), the adapter rows, the `renderTemplateWithMap` row, the `resultsWithModel` and `textWithModel` rows of the example app, the `render.adoc` paragraph, and a note in `upgrading90x.adoc`.
- The adapter conflicts on `9.0.x`, where #16237 added a branch for `ResponseEntity` and `@ResponseBody` values just before the `Map` branch. The guard still goes in the `Map` branch.
- The late-filter rows need the dispatcher fix, so this PR can go up once the `8.0.x` part has been merged up to `9.0.x`.
If you and @jdaugherty would still rather retarget the whole PR, point 1 has to be fixed there too. On `9.0.x` it also affects Grails actions that return a `StreamingResponseBody`, because #16237 streams those through the same `startCallableProcessing`. Point 2 and the guard would each need an upgrade note, and apps on 8.0.x with a late filter would keep the bug, and the workaround from the issue, until 9.0.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6130952360
@jdaugherty, @jamesfredley, with guidance from the Reporter of the issue (@ivaylodimov-taulia), I was able to reproduce the exception with a simple [Grails 8 app](https://github.com/ruthst00/15819-response-renderer-overloads) containing a Spring `RequestContextFilter` that runs after `GrailsWebRequestFilter`.
With this new information I made the following updates with commit https://github.com/apache/grails-core/pull/16516/changes/8eacc339d6256223ff533ebeed85296f1c2a6ced:
### Changes
- The dispatcher now reuses the active `GrailsWebRequest` when a later `RequestContextFilter` replaces the thread binding.
- Newly created instances are also published as the request attribute, keeping renderer and URL-mapping lookups consistent.
- Added regression coverage for uncommitted template rendering with both null and Map action returns.
- Added lifecycle coverage for completed requests, async/error redispatch, and nested include/forward restoration.
- Updated the rendering documentation. Existing branch fixes were retained.
### Verification
- Confirmed both late-filter regression cases fail with the original dispatcher implementation.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4222532748
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ if (result instanceof Map) {
+ // All render() variants except render(view:) set webRequest.renderView = false.
+ // Check the raw flag (not the composite isRenderView(), which also returns false
+ // for error status, committed response, or redirect) so that only an explicit
+ // render() call suppresses view resolution. (#15819)
+ if (!webRequest.renderViewRequested) {
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/7675410cc74ab8b8fdbddc7dafef56cf5ca6ee83
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4181558474
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -138,6 +138,7 @@ trait ResponseRenderer extends WebAttributes {
try {
response.writer.write(object.inspect())
+ response.flushBuffer()
Review Comment:
Agreed. After a second look, the correct fix for #15819 is **not** in `ResponseRenderer.groovy` at all — it belongs in `UrlMappingsInfoHandlerAdapter.groovy`.
**Root cause:** `UrlMappingsInfoHandlerAdapter.handle()` only checks `webRequest.renderView` in the `result == null` branch. The `result instanceof Map` and `result instanceof ModelAndView` branches return a `ModelAndView` unconditionally, even when `render()` was called and set `renderView = false`. If an action calls `render(template:...)` and then returns a non-null value (or if something sets the `MODEL_AND_VIEW` request attribute), the adapter returns a `ModelAndView` to Spring's `DispatcherServlet`, which then tries to resolve the view and throws `Could not resolve view`.
**The fix** — add a `!webRequest.renderView` guard in `UrlMappingsInfoHandlerAdapter` after the `MODEL_AND_VIEW` attribute check (which is the intentional `render(view:)` path) but before the `result instanceof Map` / `result instanceof ModelAndView` / `result == null` branches:
```groovy
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView // render(view:) — always honour
}
// All other render() variants set renderView=false; don't attempt view resolution
if (!webRequest.renderView) {
return null
}
if (result instanceof Map) { ... return new ModelAndView(...) }
else if (result instanceof ModelAndView) { return result }
else if (result == null) { return new ModelAndView(actionUri) }
```
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6076827327
@jamesfredley addressed two of your test concerns not addressed in previous commits with https://github.com/apache/grails-core/pull/16516/changes/7d6043541ab545187979893464370ac60b9065e9
Also updated PR to match code changes.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4220952546
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -138,6 +138,7 @@ trait ResponseRenderer extends WebAttributes {
try {
response.writer.write(object.inspect())
+ response.flushBuffer()
Review Comment:
Those flushes are no longer the fix. render(text:) clears the render flag, and the adapter returns no view after that call.
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -409,6 +430,7 @@ trait ResponseRenderer extends WebAttributes {
input = IOUtils.openStream(new File(o.toString()))
}
SpringIOUtils.copy(input, response.getOutputStream())
+ response.flushBuffer()
Review Comment:
The extra flush path is gone with the same change. Body-writing render calls clear the flag, and the adapter does not resolve a view afterward.
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ // All other render() variants (template, text, JSON, file, closure, object) set
+ // webRequest.renderView = false. If that flag is clear the response has already been
+ // handled; returning a ModelAndView here would cause DispatcherServlet to attempt
+ // view resolution and throw "Could not resolve view". (#15819)
+ if (!webRequest.renderView) {
Review Comment:
An error status by itself no longer drops the model. A redirect, or a response that is already committed, can still fall through when the action returns a Map. That case is noted on the new check.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
render(text:) now goes through render(). render(view:) still assigns MODEL_AND_VIEW directly, so that part is still open.
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ if (result instanceof Map) {
+ // All render() variants except render(view:) set webRequest.renderView = false.
+ // Check the raw flag (not the composite isRenderView(), which also returns false
+ // for error status, committed response, or redirect) so that only an explicit
+ // render() call suppresses view resolution. (#15819)
+ if (!webRequest.renderViewRequested) {
Review Comment:
This uses only the raw render flag. redirect() sets the redirect attribute and a 3xx status without calling setRenderView(false), and a response can already be committed without that call. An action that then returns a Map used to produce no view, because isRenderView() was false. It now returns a ModelAndView, so DispatcherServlet resolves the default view after the redirect or the committed body. Keep returning the model for an error status when render() was not called, and still return null when the redirect attribute is set or the response is already committed.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +403,50 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Calls render(text:), which sets renderView=false and writes the body. The adapter must
+ * return null so DispatcherServlet does not attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ render(text: 'hello')
+ null
+ }
+
+ /**
+ * Calls render(text:) and also returns a Map — the exact bug scenario from #15819 where the
+ * adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ render(text: 'hello')
+ [foo: 'bar']
+ }
+
+ /**
+ * Simulates render(view: 'myView'): sets MODEL_AND_VIEW on the request but does NOT set
+ * renderView=false. The adapter must return the ModelAndView so DispatcherServlet resolves
+ * the named view. (#15819)
+ */
+ @Action
+ def renderView() {
+ request.setAttribute(
Review Comment:
This action still installs MODEL_AND_VIEW itself and returns null. The test name says render(view:) is used, but render(view: 'myView') never runs, so the assertion passes even if that call stops publishing the attribute. Call render(view: 'myView') here, the way renderText() calls render(text:).
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4222530476
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/7675410cc74ab8b8fdbddc7dafef56cf5ca6ee83
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6082699744
@jdaugherty, the issue is described here: https://github.com/apache/grails-core/issues/15819
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4181579546
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -409,6 +430,7 @@ trait ResponseRenderer extends WebAttributes {
input = IOUtils.openStream(new File(o.toString()))
}
SpringIOUtils.copy(input, response.getOutputStream())
+ response.flushBuffer()
Review Comment:
Agreed. I've backed-out the changes to `ResponseRenderer.groovy` and associated tests as they do not address the exception thrown in [#15819](https://github.com/apache/grails-core/issues/15819). Instead, I've implemented changes to `UrlMappingsInfoHandlerAdapter.groovy` and associated tests.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6082310280
## 🚨 All tests passed but jobs failed 🚨
### Failed Jobs without Test Failures
❌ [CI / Functional Tests \(Java 21, indy\=true, shard 1\)](https://github.com/apache/grails-core/actions/runs/37900897530/job/113847863207?pr=16516)
🏷️ Commit: 7d6043541ab545187979893464370ac60b9065e9
▶️ Tests: 1260 executed
🟡 Checks: 13/80 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
☑️ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4179009844
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -138,6 +138,7 @@ trait ResponseRenderer extends WebAttributes {
try {
response.writer.write(object.inspect())
+ response.flushBuffer()
Review Comment:
`response.flushBuffer()` does not stop the exception in #15819.
Spring Framework 7.0.9 `DispatcherServlet.processDispatchResult` renders whenever the handler returns a non-null, uncleared `ModelAndView`. It does not consult `response.isCommitted()`. `render()` then throws `ServletException: Could not resolve view with name '...'` if that name does not resolve (`DispatcherServlet` around the `if (mv != null && !mv.wasCleared())` check, and the `Could not resolve view with name` throw in `render()`).
On this branch the skip signal is already `GrailsWebRequest.renderView`. `UrlMappingsInfoHandlerAdapter` builds the default action view only when the action result is null and `webRequest.renderView` is true (that property calls `isRenderView()`). These branches already set the flag to false, so a null return does not reach view resolution. The adapter ignores the flag when the action returns a `Map`, and when `GrailsApplicationAttributes.MODEL_AND_VIEW` is already set (`UrlMappingsInfoHandlerAdapter` lines 164-180). `render('ok'); return [foo: 'bar']` still selects the action view after this flush.
Committing here also has a cost. `GrailsExceptionResolver` only forwards to the error handler when the response is still uncommitted, and later status or header changes cannot take effect. `IncludeResponseWrapper.flushBuffer()` is a no-op, so this call does not commit an included response either.
Please add a DispatcherServlet-level regression for the reported failure, and make the handler adapter honor response-rendering suppression before it builds an implicit view. Do not treat `flushBuffer()` as the signal that skips view resolution.
##########
grails-controllers/src/main/groovy/grails/artefact/controller/support/ResponseRenderer.groovy:
##########
@@ -409,6 +430,7 @@ trait ResponseRenderer extends WebAttributes {
input = IOUtils.openStream(new File(o.toString()))
}
SpringIOUtils.copy(input, response.getOutputStream())
+ response.flushBuffer()
Review Comment:
This is the last new flush in `render(Map)`, but the same method still has a content-writing branch that sets `renderView = false` and does not commit.
Immediately below, the `else if (!statusSet)` branch (lines 447-460) does:
```groovy
webRequest.renderView = false
// JSONElement: renderWritable(...) with no flushBuffer() after it
// otherwise:
response.writer.write(argMap.inspect())
```
`render([a: 1])` therefore stays uncommitted, unlike `render(someObject)` at line 141. The existing `RenderMethodTests` already exercise this public Map form, and the new spec does not check it.
If the policy is that every content-writing `renderView = false` branch commits, this branch needs the same `flushBuffer()` and `ControllerExecutionException` wrapping, plus a public-API test for `render([a: 1])` that checks both the body and `isCommitted()`.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-5990024186
## 🔎 No tests executed 🔎
🏷️ Commit: bde8bf0e41fd84e6880784732b25001562a87a56
▶️ Tests: 0 executed
⚪️ Checks: 1/1 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ## Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-5983714994
## 🚨 TestLens detected 1 failed test 🚨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Functional Tests \(Java 25, indy\=false\)](https://github.com/apache/grails-core/actions/runs/37225465288/job/111504889799?pr=16516) > :grails-test-examples-views-functional-tests:integrationTest
| Test | Runs | Flakiness |
|---|---|--:|
| ModelInterceptorIntSpec > TEMPLATE\_MODEL does not leak into the interceptor of a forwarded action | 🚫 ❌ | 0% 🟢 |
🏷️ Commit: 0d0ac73e33dd1e2241c8ee228a43ce32bc325861
▶️ Tests: 81015 executed
🟡 Checks: 62/81 completed
### Test Failures
<details><summary><strong>ModelInterceptorIntSpec > TEMPLATE_MODEL does not leak into the interceptor of a forwarded action</strong> (:grails-test-examples-views-functional-tests:integrationTest in <a href="https://github.com/apache/grails-core/actions/runs/37225465288/job/111504889799?pr=16516">CI / Functional Tests (Java 25, indy=false)</a>)</summary>
```
Condition not satisfied:
modelInterceptor.modelByAction.containsKey('forwardTarget')
| | |
| [:] false
<functional.tests.ModelInterceptor@59fb0a38 latestModel=inaccessible modelByAction=inaccessible grails_artefact_Interceptor__matchers=inaccessible grails_artefact_Interceptor__order=inaccessible grails_web_api_ServletAttributes__applicationContext=inaccessible grails_web_api_ServletAttributes__servletContext=inaccessible grails_web_api_WebAttributes__grailsApplication=inaccessible grails_artefact_controller_support_RequestForwarder__linkGenerator=inaccessible grails_artefact_controller_support_RequestForwarder__urlConverter=inaccessible grails_artefact_controller_support_ResponseRedirector__linkGenerator=inaccessible grails_artefact_controller_support_ResponseRedirector__redirectListeners=inaccessible grails_artefact_controller_support_ResponseRedirector__requestDataValueProcessor=inaccessible grails_artefact_controller_support_ResponseRedirector__useJsessionId=inaccessible grails_artefact_controller_support_ResponseRenderer__actionResultTransformers=inaccessible grails_artefact_c
ontroller_support_ResponseRenderer__grailsLayoutSelector=inaccessible grails_artefact_controller_support_ResponseRenderer__grailsRenderViewMutator=inaccessible grails_artefact_controller_support_ResponseRenderer__mimeUtility=inaccessible grails_artefact_controller_support_ResponseRenderer__pluginManager=inaccessible>
at functional.tests.ModelInterceptorIntSpec.TEMPLATE_MODEL does not leak into the interceptor of a forwarded action(ModelInterceptorIntSpec.groovy:92)
```
</details>
### Rerun Controls
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 ModelInterceptorIntSpec > TEMPLATE\_MODEL does not leak into the interceptor of a forwarded action <!-- uniqueId=[engine:spock]/[spec:functional.tests.ModelInterceptorIntSpec]/[feature:$spock_feature_0_3] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
☑️ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4210585896
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,20 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ // All other render() variants (template, text, JSON, file, closure, object) set
+ // webRequest.renderView = false. If that flag is clear the response has already been
+ // handled; returning a ModelAndView here would cause DispatcherServlet to attempt
+ // view resolution and throw "Could not resolve view". (#15819)
+ if (!webRequest.renderView) {
Review Comment:
`GrailsWebRequest.isRenderView()` is false when `renderView` was cleared, and also when the status is 300 or higher, the response is committed, or a redirect was called. An action that sets `response.status` to 404 or 500 and returns a Map or `ModelAndView` used to render that model. This guard now returns null, so the error page body is dropped.
Limit the guard to an explicit `render()` (the `renderView` field itself). Leave the error-status, committed, and redirect cases on the previous path.
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +377,44 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Simulates render(text: 'hello') or render(template: '_partial'): sets renderView=false,
+ * writes content, returns null. The adapter must return null so DispatcherServlet does not
+ * attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
+ webRequest.renderView = false
+ webRequest.response.writer.write('hello')
+ null
+ }
+
+ /**
+ * Simulates an action that calls render(text:) but also returns a Map — the bug scenario
+ * from #15819 where the adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ def webRequest = RequestContextHolder.currentRequestAttributes()
Review Comment:
These fixtures assign `renderView` and `MODEL_AND_VIEW` directly, so they never call `render()`. Please drive the text, template, and view cases through the public `render(...)` methods, and add a case that sets an error status and returns a model so this guard cannot swallow error views.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4222534382
##########
grails-web-url-mappings/src/test/groovy/org/grails/web/mapping/mvc/UrlMappingsHandlerMappingSpec.groovy:
##########
@@ -319,4 +403,50 @@ class FooController {
def notFound() {
RequestContextHolder.currentRequestAttributes().response.writer << "Not Found"
}
+
+ /**
+ * Calls render(text:), which sets renderView=false and writes the body. The adapter must
+ * return null so DispatcherServlet does not attempt view resolution. (#15819)
+ */
+ @Action
+ def renderText() {
+ render(text: 'hello')
+ null
+ }
+
+ /**
+ * Calls render(text:) and also returns a Map — the exact bug scenario from #15819 where the
+ * adapter previously ignored renderView=false when result instanceof Map.
+ */
+ @Action
+ def renderTextWithMap() {
+ render(text: 'hello')
+ [foo: 'bar']
+ }
+
+ /**
+ * Simulates render(view: 'myView'): sets MODEL_AND_VIEW on the request but does NOT set
+ * renderView=false. The adapter must return the ModelAndView so DispatcherServlet resolves
+ * the named view. (#15819)
+ */
+ @Action
+ def renderView() {
+ request.setAttribute(
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/7675410cc74ab8b8fdbddc7dafef56cf5ca6ee83
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when ResponseRenderer does not commit the HTTP response [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6082144728
@ruthst00 What found this issue? Is there a functional example where this is broken? I'm inclined to suggest we move this out of 8.0 if it's technically an issue, but not reproducible in an end app - due to the risk of accepting the change.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6293977677
@ruthst00 we should add a functional test app under grails-test-examples for this scenario since it was reproduced by an end app. Can you please do that?
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4340302990
##########
grails-doc/src/en/ref/Controllers/render.adoc:
##########
@@ -95,7 +95,10 @@ render(file: new File(absolutePath), inline: true)
A multi-purpose method for rendering responses to the client which is best illustrated with a few examples! Warning - this method does not always support multiple parameters. For example, if you specify both collection and model, the model parameter will be ignored.
-Parameters
+
+Rendering a response body (for example, with `render(template: ...)` or `render(text: ...)`) suppresses the action's implicit view rendering, even if the action subsequently returns a model map. In contrast, `render(view: ...)` explicitly selects a view to render.
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/c4cb607c2bde85b90d50410431d321ec60dbe31b
##########
grails-test-examples/issue-15819/grails-app/controllers/issue15819/SearchController.groovy:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package issue15819
+
+import grails.compiler.GrailsCompileStatic
+
+@GrailsCompileStatic
+class SearchController {
+
+ static allowedMethods = [results: 'GET']
+
+ def results() {
+ // Preserve the reproducer's template name: Grails resolves it to __results.gsp.
+ render(template: '_results', model: [results: ['Grails', 'Groovy', 'Spring Boot']])
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/c4cb607c2bde85b90d50410431d321ec60dbe31b
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "ruthst00 (via GitHub)" <gi...@apache.org>.
ruthst00 commented on code in PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#discussion_r4340300660
##########
grails-web-common/src/main/groovy/org/grails/web/servlet/mvc/GrailsWebRequest.java:
##########
@@ -426,6 +426,27 @@ public void setRenderView(boolean renderView) {
this.renderView = renderView;
}
+ /**
+ * Returns {@code true} if the {@code renderView} flag has not been explicitly cleared by a
+ * {@code render()} call. Unlike {@link #isRenderView()}, this method does <em>not</em> consult
+ * the response status, the committed state, or the redirect flag — it reflects only whether
+ * {@link #setRenderView(boolean) setRenderView(false)} was called (which every {@code render()}
+ * variant except {@code render(view:)} does).
+ *
+ * <p>Use this in the handler adapter to decide whether to attempt view resolution: returning
+ * {@code null} from {@code handle()} is the correct signal to {@code DispatcherServlet} that
+ * the response has already been handled, and that signal must be sent whenever any
+ * {@code render()} variant other than {@code render(view:)} was called — regardless of whether
+ * the response happens to be committed or carries an error status.</p>
+ *
+ * @return {@code true} if no {@code render()} call has cleared the flag; {@code false} if
+ * {@code render()} (other than {@code render(view:)}) has already handled the response
+ * @since 8.0
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/c4cb607c2bde85b90d50410431d321ec60dbe31b
##########
grails-web-url-mappings/src/main/groovy/org/grails/web/mapping/mvc/UrlMappingsInfoHandlerAdapter.groovy:
##########
@@ -161,11 +161,31 @@ class UrlMappingsInfoHandlerAdapter implements HandlerAdapter, ApplicationContex
}
}
+ // render(view:) sets MODEL_AND_VIEW on the request and does not set renderView=false,
+ // so this path is always intentional view resolution — honour it unconditionally.
def modelAndView = request.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW)
if (modelAndView instanceof ModelAndView) {
return (ModelAndView) modelAndView
}
- else if (result instanceof Map) {
+ if (result instanceof Map) {
+ // All render() variants except render(view:) set webRequest.renderView = false.
+ // Check the raw flag (not the composite isRenderView(), which also returns false
+ // for error status, committed response, or redirect) so that only an explicit
+ // render() call suppresses view resolution. (#15819)
+ //
+ // redirect() sets REDIRECT_ISSUED on the request and a 3xx status without calling
+ // setRenderView(false), so renderViewRequested stays true. Similarly, the response
+ // may already be committed (e.g. the body was written directly) without that flag
+ // being cleared. In both cases there is nothing left for DispatcherServlet to do.
+ //
+ // Exception: during a servlet include dispatch the outer response is already
+ // committed, but the include can still append output and must still resolve a
+ // view. Skip the committed check for include dispatches.
+ if (!webRequest.renderViewRequested
Review Comment:
Fixed in https://github.com/apache/grails-core/pull/16516/changes/c4cb607c2bde85b90d50410431d321ec60dbe31b
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix when render(template:) throws "Could not resolve view" [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16516:
URL: https://github.com/apache/grails-core/pull/16516#issuecomment-6328449408
## 🚨 TestLens detected 1 failed test 🚨
### Test Summary
> [!TIP]
> Inspect all details with the [TestLens CLI](https://testlens.app/docs/features/cli): `testlens pr 16516`
#### [Groovy Snapshot Canary Build / Build Grails \(shard 2\)](https://github.com/apache/grails-core/actions/runs/39650414129/job/119488715613?pr=16516) > :grails-data-mongodb-spring-boot:test
| Test | Runs | Flakiness |
|---|---|--:|
| MongoDbGormAutoConfigurationSpec | 🚫 ❌ | 0% 🟢 |
🏷️ Commit: c4cb607c2bde85b90d50410431d321ec60dbe31b
▶️ Tests: 94912 executed
🟡 Checks: 94/97 completed
### Test Failures
<details><summary><strong>MongoDbGormAutoConfigurationSpec</strong> (:grails-data-mongodb-spring-boot:test in <a href="https://github.com/apache/grails-core/actions/runs/39650414129/job/119488715613?pr=16516">Groovy Snapshot Canary Build / Build Grails (shard 2)</a>)</summary>
```
org.testcontainers.containers.ContainerLaunchException: Container startup failed for image mongo:8.0
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:346)
at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:317)
at org.apache.grails.testing.mongo.MongoContainerHolder.startMongoContainer(MongoContainerHolder.groovy:42)
at org.apache.grails.testing.mongo.MongoContainerHolder.getContainer(MongoContainerHolder.groovy:53)
at org.apache.grails.testing.mongo.StartMongoGrailsUnitExtension.visitSpec_closure1(StartMongoGrailsUnitExtension.groovy:78)
at org.spockframework.runtime.extension.MethodInvocation.proceed(MethodInvocation.java:139)
at org.spockframework.runtime.model.MethodInfo.invoke(MethodInfo.java:157)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:88)
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:331)
... 7 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:551)
at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:341)
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
... 8 more
Caused by: java.lang.IllegalStateException: Wait strategy failed. Container exited with code 1
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:520)
... 10 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for log output matching '(?i).*waiting for connections.*'
at org.testcontainers.containers.wait.strategy.LogMessageWaitStrategy.waitUntilReady(LogMessageWaitStrategy.java:47)
at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:52)
at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:904)
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:487)
... 10 more
```
</details>
### Rerun Controls
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
🔲 MongoDbGormAutoConfigurationSpec <!-- uniqueId=[engine:spock]/[spec:org.grails.datastore.gorm.mongodb.boot.autoconfigure.MongoDbGormAutoConfigurationSpec] -->
Reuse successful test results:
🔲 ♻️ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
☑️ **Rerun jobs**
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org