grails-core icon indicating copy to clipboard operation
grails-core copied to clipboard

Regression in grails-views 7.0.0-M3 when forwarding from controller

Open davidkron opened this issue 9 months ago • 6 comments

I am currently experimenting with migrating an existing grails application to version 7.0.0-M3. There are some problems related to forwarding requests to other actions/controllers, which result in an error like this:

jakarta.servlet.ServletException: Could not resolve view with name 'route' in servlet with name 'dispatcherServletRegistration'
	at org.springframework.web.servlet.DispatcherServlet.render(DispatcherServlet.java:1410)
	at org.springframework.web.servlet.DispatcherServlet.processDispatchResult(DispatcherServlet.java:1167)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1105)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:978)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:903)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:564)

In this case the action route is called and it is correct that there doesn't exist a route.gsp for this action, because it is simply a dispatcher, that does forward to a different action.

Unfortunately I can't reproduce the error using a simple grails application, so the problem seems to be some special constellation in our application, which is pretty complex.

After some debugging, the problem seems to be that after the forward dispatch (handling of the forwarded request inclusive rendering seems to work as expected), the result handling of the original action will still try to render the view, even though a rendering was already done by the forward. At that point in time, the response is still in a non-committed state, which seems to be the criteria to trigger a render: https://github.com/apache/grails-core/blob/c4012113edc8bc7ad86e2158f7942790c56d68c2/grails-web-common/src/main/groovy/org/grails/web/servlet/mvc/GrailsWebRequest.java#L371

I managed to track down the settings of the commited state of the response to catalina's ApplicationDispatcher, where the closing of the response's PrintWriter will commit the response.

PrintWriter writer = response.getWriter();
writer.close();

In version 7.0.0-M3 of grails, the response's print writer seems to be a RoutablePrintWriter from Sitemesh, which doesn't commit anything and seems to be the source of my error. In the previous Grails version, the print writer is a CoyoteWriter, which seems to work as expected.

davidkron avatar Apr 16 '25 12:04 davidkron

Is this issue new as of the M3 of 7.x or new for you as you tried the M3 build of 7.x. ie did this work on M2 originally or 6.x?

I assume this is an issue with the upgrade from 6.x to 7.x?

jdaugherty avatar Apr 16 '25 20:04 jdaugherty

The issue appeared from a 6.x upgrade to 7.x. I did not use the previous milestones releases yet and went straight to M3.

davidkron avatar Apr 16 '25 21:04 davidkron

The issue appeared from a 6.x upgrade to 7.x. I did not use the previous milestones releases yet and went straight to M3.

Thank you for confirming. We know that there are several issues related to the sitemesh3 upgrade and I suspect this is another example.

jdaugherty avatar Apr 16 '25 21:04 jdaugherty

As part of the upgrade to Spring 3.5, we found several forward / error handling related issues. These issues are now fixed in 7.0.0-M4. The vote is currently underway, can you please take the time to retest this once all of 7.0.0-M4 is released? I suspect this may be fixed.

jdaugherty avatar Jun 06 '25 03:06 jdaugherty

I did test this today with the current 7.0.0-M4 version, but it still results in the same error. In the meantime, I managed to work around the issue with the following:

Controller code snippet before Grails 7:

forward action: 'enhancedRouting'
return

Workaround without forwarding:

def model = enhancedRouting(cmd)
render view: 'enhancedRouting', model: model
return

davidkron avatar Jun 19 '25 14:06 davidkron

I managed to create a reproducer: https://github.com/davidkron/grails-issue-14193

Opening http://localhost:8080/greeting/hello will produce the error.

davidkron avatar Jun 21 '25 19:06 davidkron

Thank you for the reproducer. This is on our list to take a look before the next milestone / RC.

jdaugherty avatar Jun 23 '25 19:06 jdaugherty

Further information on this ticket, the GrailsLayoutView from Grails 6.x is what used to cause the response to be committed after the call to forward was processed this code triggered on the layout view:

    @Override
        protected void renderTemplate(Map<String, Object> model, GrailsWebRequest webRequest, HttpServletRequest request,
                HttpServletResponse response) throws Exception {

            boolean isCommitted = response.isCommitted() && (response instanceof OutputAwareHttpServletResponse) && !((OutputAwareHttpServletResponse) response).isWriterAvailable();
            if( !isCommitted ) {

                Content content = obtainContent(model, webRequest, request, response);
                if (content != null) {

                    beforeDecorating(content, model, webRequest, request, response);
                    switch (request.getDispatcherType()) {
                        case INCLUDE:
                            break;
                        case ASYNC:
                        case ERROR:
                        case FORWARD:
                        case REQUEST:
                            if(LOG.isDebugEnabled()) {
                                LOG.debug("Finding layout for request and content" );
                            }
                            SpringMVCViewDecorator decorator = (SpringMVCViewDecorator) groovyPageLayoutFinder.findLayout(request, content);
                            if (decorator != null) {
                                if(LOG.isDebugEnabled()) {
                                    LOG.debug("Found layout. Rendering content for layout and model {}", decorator.getPage(), model);
                                }

                                decorator.render(content, model, request, response, webRequest.getServletContext());
                                return;
                            }
                            break;
                    }
                    PrintWriter writer = response.getWriter();
                    if(LOG.isDebugEnabled()) {
                        LOG.debug("Layout not applicable to response, writing original content");
                    }
                    content.writeOriginal(writer);
                    if (!response.isCommitted()) {
                        writer.flush();
                    }
                }
            }


        }

The writer.flush() call from processing the forward's layout would cause the response to be committed, so further calls in the action would be detected as not needing rendered. I'm assuming Sitemesh 3 now handles rendering the layout here: https://github.com/sitemesh/sitemesh3/blob/49c02e7246359cd2b4952d3f319f1a638430a477/sitemesh/src/main/java/org/sitemesh/webapp/SiteMeshFilter.java#L113

I'm not as familiar with the sitemesh 3 code but I assume it's being rendered in the filter I mention above - which occurs after the dispatcher, while before it would have occurred inside of the dispatcher. The dispatcher appears to assume that the call to renderView() will commit the result. This seems incompatible with sitemesh's approach.

@codeconsole can you give your thoughts on this one? Can you help clarify where the rendering of sitemesh occurs? Is it actually in the filter? Can this be moved to a "view" so that the dispatcher can work as it did before ?

jdaugherty avatar Jun 29 '25 21:06 jdaugherty

Thank you @davidkron , I've added a test in the branch issue14193 based on your reproducer so we can further troubleshoot this.

jdaugherty avatar Jun 30 '25 02:06 jdaugherty

@jdaugherty Sitemesh 3 is just a lightweight filter. It buffers the response, applies the decorators, then writes the output.

codeconsole avatar Jul 02 '25 18:07 codeconsole

Fixed by https://github.com/apache/grails-core/pull/14875 , we'll readd sitemesh3 in a separate branch and merge when it's ready.

jdaugherty avatar Jul 06 '25 21:07 jdaugherty